Implement NO_COLOR for terminal output #412
Implement NO_COLOR for terminal output #412FlorianLatapie wants to merge 3 commits intos0md3v:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for disabling colored terminal output via the NO_COLOR environment variable (per no-color.org) and a new --no-color CLI flag.
Changes:
- Introduces a
--no-colorargparse flag in the main entrypoint. - Updates color initialization logic to disable ANSI colors when
NO_COLORis present or--no-coloris passed.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| xsstrike.py | Adds a new --no-color CLI option to control colored output. |
| core/colors.py | Disables color output when NO_COLOR or --no-color is detected. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| parser.add_argument('--no-color', help='disable colored output', dest='no_color', | ||
| action='store_true') | ||
| args = parser.parse_args() |
There was a problem hiding this comment.
--no-color is added to argparse, but args.no_color is never read anywhere in this entrypoint. Right now color disabling relies on core/colors.py inspecting sys.argv at import time, which makes the CLI flag handling split across modules and easy to break during refactors. Consider either (a) handling --no-color entirely in xsstrike.py (e.g., pre-scan sys.argv and set NO_COLOR/a shared config flag before importing core.colors), or (b) parsing args before importing/using core.colors, so the flag is applied in one place.
| choices=core.log.log_config.keys(), default=None) | ||
| parser.add_argument('--log-file', help='Name of the file to log', dest='log_file', | ||
| default=core.log.log_file) | ||
| parser.add_argument('--no-color', help='disable colored output', dest='no_color', |
There was a problem hiding this comment.
There is trailing whitespace at the end of this line (after the comma). This tends to create noisy diffs and can fail linting in some environments; please trim it.
| parser.add_argument('--no-color', help='disable colored output', dest='no_color', | |
| parser.add_argument('--no-color', help='disable colored output', dest='no_color', |
| if ('NO_COLOR' in os.environ) or ('--no-color' in sys.argv): | ||
| colors = False | ||
|
|
||
| else : |
There was a problem hiding this comment.
PEP 8 formatting: else : should be else: (no space before the colon) to match standard Python style and reduce formatter/linter churn.
| else : | |
| else: |
| os.system('') # Enables the ANSI | ||
|
|
||
| # Check for NO_COLOR environment variable, see https://no-color.org | ||
| if ('NO_COLOR' in os.environ) or ('--no-color' in sys.argv): |
There was a problem hiding this comment.
Having core/colors.py inspect sys.argv couples this core module to a specific CLI option and makes color behavior depend on import-time process arguments (which can be surprising when the module is reused). If possible, keep core/colors.py driven by environment/config only (e.g., honor NO_COLOR here) and let the entrypoint translate --no-color into that config before importing/using core.colors.
| if ('NO_COLOR' in os.environ) or ('--no-color' in sys.argv): | |
| if 'NO_COLOR' in os.environ: |
See https://no-color.org
What does it implement/fix? Explain your changes.
Where has this been tested?
Python Version: python3.11
Operating System: Windows 11, Debian GNU/Linux 12 (bookworm) on Windows 10 x86_64 (WSL)
Does this close any currently open issues?
no
Does this add any new dependency?
no
Does this add any new command line switch/option?
yes :
--no-coloror use env variableNO_COLOR="1"Any other comments you would like to make?
Some Questions