[GitHub] [maven] michael-o commented on a diff in pull request #869: [MNG-7338] Automatically activate batch-mode and make output quiet when running in CI.

["michael-o (via GitHub)" <gi...@apache.org>]

michael-o commented on code in PR #869:
URL: https://github.com/apache/maven/pull/869#discussion_r1148871056


##########
maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java:
##########
@@ -173,7 +177,17 @@ public CLIManager() {
                 .build());
         options.addOption(Option.builder(Character.toString(BATCH_MODE))
                 .longOpt("batch-mode")
-                .desc("Run in non-interactive (batch) mode (disables output color)")
+                .desc(
+                        "Run in non-interactive (batch) mode (disables output color) - alias for --non-interactive (kept for backwards compatability)")

Review Comment:
   I think this is not just color. There more cases. Basically everything where standard streams aren't connected to a tty. Also blocking for input, like Maven release. Since one cannot cover all cases, I would remove the parentheses.



##########
maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java:
##########
@@ -38,6 +38,10 @@ public class CLIManager {
 
     public static final char BATCH_MODE = 'B';
 
+    public static final String NON_INTERACTIVE = "ni";
+
+    public static final String FORCE_INTERACTIVE = "fi";

Review Comment:
   I wonder whether such important flags should really have short options, especially `force`. Keeping `-B` as short hand is fine.



##########
maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -444,10 +447,10 @@ private CommandLine cliMerge(CommandLine mavenConfig, CommandLine mavenCli) {
      */
     void logging(CliRequest cliRequest) {
         // LOG LEVEL
-        cliRequest.verbose = cliRequest.commandLine.hasOption(CLIManager.VERBOSE)
-                || cliRequest.commandLine.hasOption(CLIManager.DEBUG);
-        cliRequest.quiet = !cliRequest.verbose && cliRequest.commandLine.hasOption(CLIManager.QUIET);
-        cliRequest.showErrors = cliRequest.verbose || cliRequest.commandLine.hasOption(CLIManager.ERRORS);
+        CommandLine commandLine = cliRequest.commandLine;
+        cliRequest.verbose = commandLine.hasOption(CLIManager.VERBOSE) || commandLine.hasOption(CLIManager.DEBUG);

Review Comment:
   This is a nice improvement. Can we move it to a separate PR and apply it basically immediately to 3.9.x and master?



##########
maven-embedder/src/main/java/org/apache/maven/cli/CLIManager.java:
##########
@@ -173,7 +177,17 @@ public CLIManager() {
                 .build());
         options.addOption(Option.builder(Character.toString(BATCH_MODE))
                 .longOpt("batch-mode")
-                .desc("Run in non-interactive (batch) mode (disables output color)")
+                .desc(
+                        "Run in non-interactive (batch) mode (disables output color) - alias for --non-interactive (kept for backwards compatability)")
+                .build());
+        options.addOption(Option.builder(NON_INTERACTIVE)
+                .longOpt("non-interactive")
+                .desc("Run in non-interactive (batch) mode (disables output color) - alias for --batch-mode")
+                .build());
+        options.addOption(Option.builder(FORCE_INTERACTIVE)
+                .longOpt("force-interactive")
+                .desc(
+                        "Run in interactive mode - even when the environment variable CI is set to true and --non-interactive or --batch-mode are set")

Review Comment:
   Here we need to check that Jansi and friends get this flag properly since Jansi uses `isatty(2)` to test that.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org