MNG-6220 add color CLI option

[mryan43]

https://issues.apache.org/jira/browse/MNG-6220

[michael-o]

I will comment shortly.

[michael-o]

They are several issues. People are used to GNU and BSD tools like grep. They use values never, always, auto. Having said that I do think we need to do this exactly the same way: grep.c. Option always requires special attention due to isatty in combination with less(1).

[michael-o]

Two issues:

1. Unfortunately, Commons CLI does not offer valid values. If it would, it would fail if the option value is wrong. I would do so too.
2. You blindly return from always, but still forget that Jansi will ignore that because isatty returns false as soon as stdout is passed via pipe.

[mryan43]

Good idea to align the name of choices with grep, I just changed it.

About "isatty", do you think we should set the jansi.force system property when --color is set to "always" ? https://github.com/fusesource/jansi/blob/2616142fda4425d779ac94a3d9bfa76412021b23/jansi/src/main/java/org/fusesource/jansi/AnsiConsole.java

[michael-o]

I think yes, grep doesn't do different. Please try the patch thoroughly!

[mryan43]

I will test with less and jenkins and comment again with results.

[mryan43]

Unfortunately, the system property is read only once in a static init block in the JansiConsole class before the cli main method is invoked, so my change doesn't work.

We have a kind of chicken and egg situation and we need to decide if we prefer

a) not setting up Jansi until arguments/config have been parsed and validated (and call MessageUtils.systemInstall(); later in the loggin() method)

b) forcing jansi until arguments and config have been parsed (by adding System.setProperty( "jansi.force", "true" ); before MessageUtils.systemInstall(); in MavenCli#main and then eventually disable it later in the loggin() method

What do you think ?

[mryan43]

I applied solution B.

I have added an auto-detection method in MessageUtils in my fork of maven-shared-utils (https://github.com/mryan43/maven-shared) based on the corresponding code in jansi.

Unfortunately, upgrading to 3.2.0-SNAPSHOT from maven breaks compilation because of changes done for MSHARED-587 but I'm affraid fixing them would be out of scope of this PR.

[mryan43]

I feel stuck until someone with more experience of the maven code base update the version of maven-shared-utils to 3.2.1-SNAPSHOT on the default branch, anyone up to the task ?

[michael-o]

I don't might to pick this up, but it won't happen before Sep for personal priorities.

[rfscholte]

Here's my proposal: https://git-wip-us.apache.org/repos/asf?p=maven.git;a=commit;h=fd57754218e749305be1dd745fda9407960cf985

[agudian]

@rfscholte: looks good! 👍

[mryan43]

Great thanks @rfscholte !