gh-141939: Add colors to interpolated values in argparse

[alexprengere]

• Issue: Add colours to argparse interpolated values in --help #141939

[mpkocher]

Consider adding a test or extending an existing test.

[alexprengere]

Of course 😉 (this is a draft PR).
I will extend the color test in test_argparse, I wanted to be sure whether to color all interpolated values or just default.
Also I am unsure whether to re-use theme.default_value or introduce new values in _colorize.py like #141680 did.
I will clean this up tomorrow and then mark it as ready for review.

[alexprengere]

I updated the code and tests 😉

I had to change a bit the logic introduced in #141680 as introducing a new color for interpolated values, that would be different from the default_value color, is a bit complex to handle.

For ArgumentDefaultsHelpFormatter, as _get_help_string returns the %(default)s already "colored", updating _expand_help to "color" interpolated values would essentially color the default values twice, and no longer respect the default_value color.
So I removed the coloring of %(default)s in _get_help_string, and now it happens in _expand_help, as the default value will be colored, along all interpolated values, using a sinple interpolated_value color.

I hope this is clear, I can amend this if needed.

[alexprengere]

For the record the PR could be slightly improved with the formatting of choices, by coloring each individual values before joining them with ', '.
I tried it but encountered bug in an unrelated part (filed in #142035):

The issue is that the use of textwrap.wrap happens after the ANSI codes have been inserted, causing the wrapping to be broken in some cases.

[savannahostrowski]

A couple of comments but I spent some time playing around with this and I think it looks pretty good. I think that we're early enough in the release cycle that we can see what, if any, feedback we get on the use of YELLOW for default, type and interpolated values. Some folks may find it overly busy but personally, I think having some colour treatment here does help with the overall readability.

FWIW, I also compared this to rich-argparse and click. Neither seem to do this type of interpolation in help text. Not necessarily a point for or against doing this but just wanted to note it!

[bedevere-app]

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.

[alexprengere]

Thanks a lot for the review @savannahostrowski!
I have made the requested changes; please review again.

For the record, I was willing to also change:

# currentparams[choices] =', '.join(map(str, params['choices'])) # possibledef_color(s): returnf"{t.interpolated_value}{s}{t.reset}"params['choices'] =', '.join(map(_color, params['choices']))

But this introduced more ANSI escape codes, and I kept hitting the wrapping bug as described in #142035:

So I left that part alone for now, but I can revisit if we ever find a solution.

[bedevere-app]

Thanks for making the requested changes!

@savannahostrowski: please review the changes made to this pull request.

[savannahostrowski]

Thanks for this! I'll let Hugo also take a look before merging but this LGTM!

[mpkocher]

@alexprengere A lot of attention to detail on this issue and it shows. Nice job.

# currentparams[choices] =', '.join(map(str, params['choices'])) # possibledef_color(s): returnf"{t.interpolated_value}{s}{t.reset}"params['choices'] =', '.join(map(_color, params['choices']))

As a separate issue, when colorize is made public, I believe it would be useful to improve the ergonomics and add a sugar layer so that AsciiColors.Green("car") (or something similar) would translate to {AsciiColors.Green.value}car{AsciiColors.reset}. This current model of {something}value{t.reset} is a bit chatty with always calling .reset. We probably want to avoid people defining their own _color util func in their project/lib.

[hugovk]

Thanks! Please could you fix the merge conflict?

[alexprengere]

It shoud be fine now 👍

[savannahostrowski]

Thanks again for working on this @alexprengere! Appreciate it!

[oyvindronningstad]

The above change looks to be giving me issues at this line.

An MVE looks like this:

my_parser.add_argument( "--foo", type=int, default=1234, help=f"""0x%(default)x""", ) 

The above works in 3.14.2, but not in 3.15 alpha3. I apologize if my code is doing something incorrect.

The error I get is

Traceback (most recent call last): File "[...]/3.15.0a3/lib/python3.15/argparse.py", line 1793, in _check_help formatter._expand_help(action) ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^ File "[...]/3.15.0a3/lib/python3.15/argparse.py", line 696, in _expand_help return help_string % params ~~~~~~~~~~~~^~~~~~~~ TypeError: %x format: an integer is required, not str 

[alexprengere]

IMO your code is correct. Here is the basic issue:

# before 3.15a3>>>"""0x%(default)x"""%{'default': 1234} '0x4d2'# after 3.15a3>>>"""0x%(default)x"""%{'default': f"\x1b[1;31m1234"} Traceback (mostrecentcalllast): File"<python-input-14>", line1, in<module>"""0x%(default)x"""%{'default': f"\x1b[1;31m1234"} ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~TypeError: %xformat: anintegerisrequired, notstr

The value to interpolate is converted to a (colored) string first, then passed to the % formatting, where it fails due to a type mismatch (with x it expects an int).

If we want to keep this feature, I think the coloring needs to happen in help_string, so we do not fail on such cases, as we preserve the types of interpolated values. This is a bit more complex, but was already done for ArgumentDefaultsHelpFormatter in #141680 for _get_help_string.

[alexprengere]

For the record I created #142980 to track this.