Skip to content

Conversation

@frostming
Copy link
Contributor

@bedevere-app
Copy link

bedevere-app bot commented Oct 25, 2025

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@savannahostrowski
Copy link
Member

savannahostrowski commented Oct 27, 2025

Thanks for addressing the comments. After thinking about this more, I'm a bit concerned about API compatibility. Adding the formatter parameter to format_usage() and format_help() changes the public API. While it has a default value, this could still break subclasses that override these methods. I would like to backport a fix for this bug, but maybe @hugovk has thoughts on this change as RM.

Also, we'll need to add some tests here, since this wasn't caught with our existing test suite.

@hugovk
Copy link
Member

hugovk commented Oct 28, 2025

I would like to backport a fix for this bug, but maybe @hugovk has thoughts on this change as RM.

After originally adding colour to argparse (#132323), we did get reports (#133653) that subclassing HelpFormatter was broken:

TypeError: HelpFormatter.__init__() got an unexpected keyword argument 'prefix_chars'

We initially considered this a docs issue, because HelpFormatter isn't fully documented (#133653 (comment)) but then fixed it in another way (#133941).

We could again consider this a docs issue, but I think we need to be careful about backwards compatibility especially after 3.14.0. Could be worth checking things like tox, virtualenv and mypy that were affected last time.

And if we choose not to backport, we can still use NO_COLOR=1 or PYTHON_COLORS=0 when redirecting output:

NO_COLOR=1 ./python.exe -m calendar 2020 01 01 2> err# orPYTHON_COLORS=0 ./python.exe -m calendar 2020 01 01 2> err# thencat err
usage: python.exe -m calendar [-h] [-w WIDTH] [-l LINES] [-s SPACING] [-m MONTHS]
                              [-c CSS] [-L LOCALE] [-e ENCODING] [-t {text,html}]
                              [-f FIRST_WEEKDAY]
                              [year] [month]
python.exe -m calendar: error: unrecognized arguments: 01

@savannahostrowski
Copy link
Member

savannahostrowski commented Oct 30, 2025

@hugovk Oh thanks, I missed that thread...this is the kind of thing I was afraid of. I'm inclined to say 3.15 onward + a docs update to note this limitation/workaround in 3.14.

@savannahostrowski savannahostrowski removed the needs backport to 3.14 bugs and security fixes label Oct 30, 2025
@frostming frostming force-pushed the fix/colorize-argparse branch from fe5c9d7 to 82e00f0 Compare November 12, 2025 02:18
@savannahostrowski
Copy link
Member

Circling back on this (appreciate your patience @frostming!), based on this convo, I think we should just move forward with this for 3.15, no backport since this is an API change. For 3.14, we should open a separate PR for the documentation changes for the workaround.

I'll leave this open for another couple days to see if anyone else wants to chime in. Otherwise, will merge 😄 !

@savannahostrowski
Copy link
Member

@frostming Mind resolving the merge conflicts and then I can get to merging this? 🙂

@frostming frostming force-pushed the fix/colorize-argparse branch from dc33ac5 to 8518870 Compare December 8, 2025 00:30
@frostming
Copy link
Contributor Author

@frostming Mind resolving the merge conflicts and then I can get to merging this? 🙂

It's done. Thank you very much.

@savannahostrowski savannahostrowski enabled auto-merge (squash) December 8, 2025 03:41
@savannahostrowski savannahostrowski merged commit 7099af8 into python:main Dec 8, 2025
46 checks passed
@frostming frostming deleted the fix/colorize-argparse branch December 8, 2025 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Errors raised in argparse are colorized when redirecting stderr to a file

5 participants