Proper interpretation of NO_COLOR environment variable

[ptmcg]

What's the problem this feature will solve?

tox interpretation of NO_COLOR uses StrConvert.to_bool() to look for specific string values (case insensitive match):

• 1, on, yes, true to "enable" NO_COLOR (i.e., disable colorizing output)
• 0, off, no, false, or "" to disable NO_COLOR (that is, enable colorizing output)

De facto conventions for NO_COLOR are to accept any non-blank string as an indication to disable colorizing output.

In addition, the StrConvert.to_bool() method raises a TypeError exception if some other value besides those listed above is found in the NO_COLOR environment variable. This will break pipeline builds for an essentially cosmetic reason.

Describe the solution you'd like

StrConvert.to_bool may be used in other places in tox to interpret various settings, so rather than modifying the logic in to_bool, I suggest modifying the code in (parser.py) add_color_flags() to just check for empty vs non-empty string when retrieving NO_COLOR from the environment, with a default of "".

def add_color_flags(parser: ArgumentParser) -> None:
    converter = StrConvert()
    # if converter.to_bool(os.environ.get("NO_COLOR", "")):   <-- CHANGE THIS LINE
    if os.environ.get("NO_COLOR", "") != "":
        color = "no"
    elif converter.to_bool(os.environ.get("FORCE_COLOR", "")):
        color = "yes"
    elif os.environ.get("TERM", "") == "dumb":
        color = "no"
    else:
        color = "yes" if sys.stdout.isatty() else "no"

    parser.add_argument(
        "--colored",
        default=color,
        choices=["yes", "no"],
        help="should output be enriched with colors, default is yes unless TERM=dumb or NO_COLOR is defined.",
    )

(Python does not expressly require the comparison against "", but doing so makes the purpose more explicit.)

Now pipeline scripts using NO_COLOR for other utilities that follow the "empty is off, non-empty is on" convention can also use tox without having to risk getting pipeline-breaking exceptions due to the value constraints in to_bool.

Alternative Solutions

Changing our current pipeline code to use NO_COLOR with one of the tox-compatible true values to disable colorizing, or leaving empty to enable colorizing is a workaround. But it is inconsistent with other conventions if one of the tox-compatible false values is set to enable colorizing, which by convention should disable it.

Additional context

None.

[jimrthy]

It seems worth mentioning that this behavior is new in 4.0. So there probably aren't a lot of existing scripts that depend on it.

[gaborbernat]

Put in a PR with your change proposal.