unittest: no color if stdout is not a tty #7424
Conversation
|
The only thing I'd be concerned about here is if Travis/AppVeyor/etc will stop getting colours because of it. Would be nice to test that. |
|
It is a better default to only output ansi color sequences when output on a tty. In this case, I guess the question is how we can specify the color output in spite of the default. Perhaps we can check the value of |
|
The value should have 3 states always / auto / never to be more flexible than a boolean. |
|
I'm playing with custom compiler's colored output (nimcolors.cfg). colors.compiler:on # can be on, off, auto
colors.compiler.truecolor:on # can be on, off, auto
colors.compiler.error.style = ...
colors.compiler.error.fg = ...
colors.compiler.error.bg = ...
colors.compiler.hint.style = ...
colors.compiler.hint.fg = ...
colors.compiler.hint.bg = ...
colors.compiler.normal.style = ...
colors.compiler.normal.fg = ...
colors.compiler.normal.bg = ...
colors.compiler.warning.style = ...
colors.compiler.warning.fg = ...
colors.compiler.warning.bg = ...Later this configuration file can be extended for other tools. |
We accept a new environment variable, NIMTEST_COLOR, which override the effect of NIMTEST_NO_COLOR. The environment variable, NIMTEST_COLOR, can be 'never' or 'always', which set the color output to false or true, respectively.
jxy
force-pushed
the
unittest_notty_nocolor
branch
from
7bc01ab to
9df2ebf
Compare
|
Now there is the I actually prefer As to |
| colorOutput = false | ||
| elif colorEnv == "always": | ||
| colorOutput = true | ||
| elif existsEnv("NIMTEST_NO_COLOR"): |
There was a problem hiding this comment.
Because NIMTEST_COLOR should override NIMTEST_NO_COLOR, only if NIMTEST_COLOR is not defined in the env, we consider NIMTEST_NO_COLOR.
| @@ -185,7 +192,15 @@ proc defaultConsoleFormatter*(): ConsoleOutputFormatter = | |||
| # Reading settings | |||
| # On a terminal this branch is executed | |||
| var envOutLvl = os.getEnv("NIMTEST_OUTPUT_LVL").string | |||
| var colorOutput = not existsEnv("NIMTEST_NO_COLOR") | |||
| var colorOutput = isatty(stdout) | |||
| if existsEnv("NIMTEST_COLOR"): | |||
There was a problem hiding this comment.
This existsEnv check is not required, getenv returns "" if it doesn't exist. And yes, I do like that because it simplifies the logic.
There was a problem hiding this comment.
My original thought was that, if NIMTEST_COLOR is defined, even if not what we recognize (neither never nor always), we would ignore NIMTEST_NO_COLOR.
Perhaps you have a better idea of introducing a new env variable and deprecating the old?
|
Meh, whatever. |
No description provided.