Support FORCE_COLOR and NO_COLOR

[hashar]

When running on CI the process stdout is not a tty and coloring is disabled unless one uses --format colored.

Add support for forcing colored output via the environment variable FORCE_COLOR and add NO_COLOR (which takes precedence).

The spec from https://force-color.org/ states an environment variable is taken in account when:

• it is present
• AND NOT an empty string

Command-line software which outputs colored text should check for a FORCE_COLOR environment variable. When this variable is present and not an empty string (regardless of its value), it should force theaddition of ANSI color.

[adrienverge]

Hello Antoine,

Thanks for this proposal. NO_COLOR and FORCE_COLOR seem pretty standard, so it looks like a good idea 👍

Please see my comments in the code, and the tests must be reworked:

• Do no test the internal function cli.supports_color(), instead, test the real yamllint program. See inside in CommandLineTestCase, there are already test_run_piped_output_nocolor(), test_run_format_colored(), test_run_format_colored_warning(). Please add the new tests there.
• Also test NO_COLOR and FORCE_COLOR in conjunction with --format standard and --format colored (the command line argument must win).
• No need for comments # Assertion helpers and # Tests: the readers will infer that 😉

[adrienverge]

This looks like an error, right?

From https://force-color.org/, if FORCE_COLOR is empty, it should be ignored:

When this variable is present and not an empty string, it should force the addition of ANSI color.

[hashar]

It is definitely an error which has two causes:

• I did not read the spec, despite it being written in bold, it is again a case of Read The Fine Manual. Thank you for having quoted it.

• no_color[0] != '\0', I have misread that as the first character is zero and my implementation does not check that. I took a few courses at C language 30 years ago went for pascal and assembly instead and never looked back. That \0 is NULL or the end of a string eg it is an empty string :-]

I'll fix it later today. thank you.

[hashar]

I have changed the logic to check the absence of the env variable and it NOT being an empty string:

    if (no_color is not None and no_color != ''):
        return False
    if (force_color is not None and force_color != ''):
        return True

[hashar]

TODO address the items mentioned in the cover message which I have missed:

• Do no test the internal function cli.supports_color(), instead, test the real yamllint program. See inside in CommandLineTestCase, there are already test_run_piped_output_nocolor(), test_run_format_colored(), test_run_format_colored_warning(). Please add the new tests there.
• Also test NO_COLOR and FORCE_COLOR in conjunction with --format standard and --format colored (the command line argument must win).
• No need for comments # Assertion helpers and # Tests: the readers will infer that 😉

That will be for after the lunch break :]

[hashar]

[x] Do no test the internal function cli.supports_color(), instead, test the real yamllint program. See inside in CommandLineTestCase, there are already test_run_piped_output_nocolor(), test_run_format_colored(), test_run_format_colored_warning(). Please add the new tests there.

I have kept the unit tests covering supports_color() I think there is value in them and I guess they can be extended to cover Windows/ANSICON/TERM/whether stdout is a tty.

[x] Also test NO_COLOR and FORCE_COLOR in conjunction with --format standard and --format colored (the command line argument must win).

I have added test cases for the following combinations:

Format	Env	Colored?
auto	NO_COLOR	🚫
auto	FORCE_COLOR	🆗
standard	NO_COLOR	🚫
standard	FORCE_COLOR	🚫
colored	NO_COLOR	🆗
colored	FORCE_COLOR	🆗

[x] No need for comments # Assertion helpers and # Tests: the readers will infer that 😉

I have removed them :-]