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-unsubscr...@maven.apache.org

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

Reply via email to