Py tooling: fix Windows terminal color output and handle color flags in compare.py (fixes #642) - #2297
Py tooling: fix Windows terminal color output and handle color flags in compare.py (fixes #642)#2297jdymitarai wants to merge 2 commits into
Conversation
…in compare.py (fixes google#642) * Enable virtual terminal processing on Windows console handles when supported so ANSI escape codes are rendered natively instead of printing literal '^[[92m' sequences. * Fall back to disabled colors when stdout is not a TTY or when the standard NO_COLOR environment variable is present. * Support --color and --no-color on the root parser and correctly extract --color, --no-color, and --benchmark_color=false|true|auto if provided after subcommands. * Prevent false warnings in check_inputs when --benchmark_color is provided with JSON inputs. * Fix 'import util' in tools/gbench/report.py test to 'from gbench import util'. * Add unit tests covering color flag parsing, option extraction, and NO_COLOR detection.
8799b00 to
80784e6
Compare
|
|
||
| parser.add_argument( | ||
| color_group = parser.add_mutually_exclusive_group() | ||
| color_group.add_argument( |
There was a problem hiding this comment.
isn't the opposite of --no-color just --color? why do we need both flags?
|
Previously, \compare.py\ only had --no-color\ (with \default=True), which meant color could only be turned off, and could never be explicitly forced on (e.g., when piping output into \less -R\ or in CI runners where stdout is not detected as an interactive TTY). We added --color\ to allow explicitly forcing color output, while retaining --no-color\ for 100% backwards compatibility with existing user scripts and CLI habits. When neither flag is specified (\default=None), \compare.py\ auto-detects whether stdout is an interactive TTY and whether VT processing is supported (and honors the standard \NO_COLOR\ environment variable). |
that explains what it does, but not why. why do we need to force color on? |
|
(This also probably needs LLM disclosure notice.) |
3 participants
Summary
This PR fixes issue #642 where
compare.pyproduced literal ANSI escape characters (^[[92m,^[[0m, etc.) on Windows consoles, and ensures that color settings (--color,--no-color,--benchmark_color=...) are properly recognized regardless of argument position.Root Cause
\033[...]) requireENABLE_VIRTUAL_TERMINAL_PROCESSING(0x0004) to be set on the console output buffer. Without this flag,cmd.exeand standard console hosts print literal escape sequences.compare.pypreviously defaultedcolor=Trueunconditionally, even when stdout was not a TTY (such as redirected to a file or piped to another tool), or when the standardNO_COLORenvironment variable was set.compare.pycollect trailing options intobenchmark_options(nargs=argparse.REMAINDER), placing--no-coloror--benchmark_color=falseafter the subcommand or arguments caused the flag to be captured as benchmark runner options instead of settingargs.color, while triggering a spurious warning when inputs were JSON files.tools/gbench/report.pyline 1492,import utilcausedModuleNotFoundErrorwhen run viapython -m unittest.Changes
ENABLE_VIRTUAL_TERMINAL_PROCESSINGon Windows console handles when available, and disable color formatting if VT mode is not supported by the console.sys.stdout.isatty()) and respectNO_COLORstandard (https://no-color.org/).--colorand--no-coloron root parser, and extract--color,--no-color, and--benchmark_color=false|true|autofrom trailingbenchmark_options.check_inputs.from gbench import utilinreport.py, and add unit tests incompare.pycovering color flag parsing, option resolution, andNO_COLORhandling.AUTHORSandCONTRIBUTORS.Verification
tools/pass:python -m unittest discover -s tools -p "*.py"