support NO_COLOR env var for disabling ANSI

[johnbchron]

Objective

Allows disabling ANSI codes in log outputs according to https://no-color.org/. Fixes #11351

Solution

Instead of using the default tracing subscriber formatting layer, we check if the the env variable NO_COLOR equals "1", and if so, set .with_ansi(false).

---

Changelog

Allows setting the NO_COLOR environment variable to disable ANSI codes in log output.

Migration Guide

N/A (non-breaking, new feature)

[dmlary]

Should this be needed with the upstream NO_COLOR support in tracing? tokio-rs/tracing#2647

At the lowest level in the tracing crate, the default subscriber checks for this environment variable. Is there something between bevy and the subscriber calling with_ansi(true)?

[JMS55]

If the env variable is present, color should not be used regardless of its value.

[dmlary]

I initially thought the same thing. But if you carefully read the nocolor website, command line flags are supposed to override the environment variable. Because of this, the upstream change allows with_ansi() to override the environment variable, otherwise it wouldn’t be possible for command-line flags to override the environment variable.

Longer explanation in this comment in the upstream issue for the PR tokio-rs/tracing#2388 (comment)

[dmlary]

Updated: Ignore this comment; I found the reference. Leaving this incorrect info for posterity.

Bah, they literally just updated the nocolor website a week ago, and the ~~text about flag overriding has been removed. ~~

If you want to push for NO_COLOR to always override commandline flags, I’d open a new PR upstream to fix the behavior there. The reason can be that the NO_COLOR guidelines changed.

[dmlary]

I found it in the code block on the no-color website. It’s the comment at the bottom of their example code that says flags can override environment.

int
main(int argc, char *argv[])
{
	char *no_color = getenv("NO_COLOR");
	bool color = true;

	if (no_color != NULL && no_color[0] != '\0')
		color = false;

	/* do getopt(3) and/or config-file parsing to possibly turn color back on */
	...
}

[mockersf]

from my understanding the check should be on "not empty". if it's set but empty, keep the colours, if it's set and not empty, then remove the colours

[BD103]

Should this be needed with the upstream NO_COLOR support in tracing? tokio-rs/tracing#2647

At the lowest level in the tracing crate, the default subscriber checks for this environment variable. Is there something between bevy and the subscriber calling with_ansi(true)?

This was added to tracing-subscriber 0.3.18. Perhaps this needs a version bump?

[mockersf]

This was added to tracing-subscriber 0.3.18.

I just checked and this already works with Bevy

[johnbchron]

Yeah I didn't even think to check whether this was already implemented with tracing. Thanks!