GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||
GeezerVison is showing github.com — GH-139809: Fix argparse subcommand prog not respecting color environment variables by savannahostrowski · Pull Request #139818 · python/cpython Conversation
|
| 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.
The reason will be displayed to describe this comment to others. Learn more.
Just to make sure this removes to the coloring independent if coloring is on or not, yes?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Well, _decolor strips ANSI codes when coloring is enabled. When coloring is disabled, it's a no-op since no ANSI codes should exist.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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).
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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 😆
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM. 👍
There are helpers for temporary monkey-patching in test.support and unittest.mock. You can use them if you prefer.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Thanks @savannahostrowski for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
…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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.