GH-139809: Fix argparse subcommand prog not respecting color environment variables #139818
Conversation
Lib/argparse.py
Outdated
| groups = self._mutually_exclusive_groups | ||
| formatter.add_usage(None, positionals, groups, '') | ||
| kwargs['prog'] = formatter.format_help().strip() | ||
| kwargs['prog'] = formatter._decolor(formatter.format_help().strip()) |
There was a problem hiding this comment.
Just to make sure this removes to the coloring independent if coloring is on or not, yes?
There was a problem hiding this comment.
Well, _decolor strips ANSI codes when coloring is enabled. When coloring is disabled, it's a no-op since no ANSI codes should exist.
There was a problem hiding this comment.
That's not going to be enough. I mentioned before, but prog is a public metadata on the parser that contains the name of the application. We should not color it until it's about to be printed later on, otherwise will not fix my issue here https://github.com/tox-dev/sphinx-argparse-cli/blob/main/src/sphinx_argparse_cli/_logic.py#L340-L342. Thanks!
There was a problem hiding this comment.
Hmm, I see your point. I think another approach here could be to create a separate formatter explicitly set with color=False, so that prog wouldn't get colored in the first place (rather than having to decolor it).
There was a problem hiding this comment.
Unrelated but I should mention it here that I was surprised that setting color to False for the formatter (see here https://github.com/tox-dev/sphinx-argparse-cli/blob/main/src/sphinx_argparse_cli/_logic.py#L402-L404) doesn't actually disables color printing, had to set NO_COLOR during format_usage to not color that output 🤔 Seems the color argument of the parser is instead used 🤔 not sure if that's a bug or a feature, but suprised me.
By the way I love the colors in the CLI ❤️ (definitely the best feature of 3.14) , just they are not needed when you're writing a sphinx plugin to render it for other outputs 😆
There was a problem hiding this comment.
Hmm, this seems like a bug. HelpFormatter accepts color, but it is then seemingly overridden in _get_formatter. This should probably be respected, or we should remove it to avoid confusion.
|
All commit authors signed the Contributor License Agreement. |
|
Thanks @savannahostrowski for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
…vironment variables (pythonGH-139818) (cherry picked from commit 9fc4366) Co-authored-by: Savannah Ostrowski <[email protected]>
|
GH-139866 is a backport of this pull request to the 3.14 branch. |
| formatter = self._get_formatter() | ||
| # Create formatter without color to avoid storing ANSI codes in prog | ||
| formatter = self.formatter_class(prog=self.prog) | ||
| formatter._set_color(False) |
There was a problem hiding this comment.
Just double checking, sub-commands still get correctly colorized later on, yeah? Before they were just double colored?
There was a problem hiding this comment.
Yeah, that's right.
Uh oh!
There was an error while loading. Please reload this page.