gh-62090: Simplify argparse usage formatting

[hamdanal]

Rationale

argparse performs a complex formatting of the usage for argument grouping and for line wrapping to fit the terminal width. This formatting has been a constant source of bugs for at least 10 years (see linked issues below) where defensive assertion errors are triggered or brackets and paranthesis are not properly handeled.

Problem

The current implementation of argparse usage formatting relies on regular expressions to group arguments usage only to separate them again later with another set of regular expressions. This is a complex and error prone approach that caused all the issues linked below. Special casing certain argument formats has not solved the problem. The following are some of the most common issues:

• empty metavar
• mutually exclusive groups with SUPPRESSed arguments
• metavars with whitespace
• metavars with brackets or paranthesis

Solution

The following two comments summarize the solution:

• argparse wrapping fails with metavar="" (no metavar) #82091 (comment)
• argparse assertion failure with multiline metavars #77048 (comment)

Mainly, the solution is to rewrite the usage formatting to avoid the group-then-separate approach. Instead, the usage parts are kept separate and only joined together at the end. This allows for a much simpler implementation that is easier to understand and maintain. It avoids the regular expressions approach and fixes the corresponding issues.

This closes the following issues:

• argparse: mutually exclusive groups full of help-suppressed args can cause AssertionErrors #62090
• argparse usage should preserve () in metavars such as range(20) #62549
• argparse assertion failure with multiline metavars #77048
• argparse wrapping fails with metavar="" (no metavar) #82091
• argparse.ArgumentParser.add_mutually_exclusive_group : metavar create parenthesis undefined behavior #89743
• argparse sometimes tracebacks when all options in a mutually exclusive group are suppressed #96310
• mutually exclusive group w/ all parameters argparse.SUPPRESS'd causes an assert #98666

These PRs become obsolete:

• bpo-37910: argparse usage wrapping should allow whitespace differences caused by metavar #15372
• gh-96310: Fix a traceback in argparse when all options in a mutually exclusive group are suppressed #96311

• Issue: argparse: mutually exclusive groups full of help-suppressed args can cause AssertionErrors #62090

[cpython-cla-bot]

All commit authors signed the Contributor License Agreement.

[hamdanal]

@rhettinger @hpaulj I’d like to get your opinion on this change if you don’t mind.

P.S: the failure on Windows doesn’t seem to be related.

[hpaulj]

I agree that the existing usage formatter is too brittle. 9 years ago I tried to write an alternative as part of a project to generalize the concept of mutually-exclusive-groups, allowing other relations (and/or/any etc), and nesting. https://bugs.python.org/issue11588. I can't offhand find my best usage patch, but agree with the idea of building usage groups, and combining them at the end.

But currently I'm not as involved with github and argparse. I'm commenting on some issues, but have not contributed (or tested) a patch (in the old style) or github push in a long time.

[encukou]

A simplification sounds good to me. I'll start reading up on argparse so I can review this in detail.

[encukou]

This does not handle nested exclusive groups correctly. Nested exclusive groups are deprecated, but still supported for now. For example:

import argparse
parser = argparse.ArgumentParser()
g=parser.add_mutually_exclusive_group()
g.add_argument('--spam')
gg=g.add_mutually_exclusive_group()
gg.add_argument('--hax')
gg.add_argument('--hex')
g.add_argument('--eggs')
parser.add_argument('--num')
print(parser.format_usage())

Without this change, the output is not ideal, but technically correct:

/tmp/repro.py:5: DeprecationWarning: Nesting mutually exclusive groups is deprecated.
  gg=g.add_mutually_exclusive_group()
usage: repro.py [-h] [--spam SPAM | [--hax HAX | --hex HEX |] --eggs EGGS] [--num NUM]

But with this change, the argument added at the end (--num) joins the group:

/tmp/repro.py:5: DeprecationWarning: Nesting mutually exclusive groups is deprecated.
  gg=g.add_mutually_exclusive_group()
usage: repro.py [-h] [--spam SPAM | [--hax HAX | --hex HEX] | --eggs EGGS | [--num NUM]]

One fix would be to pad the new sequence with None to keep the original length when replacing parts[start:end] at the end. That way the meaning of indices wouldn't change

[hpaulj]

We need to be very careful when nesting groups. What's the intent, and what's the actual parsing action? Carefully consider all the discussion dealing with the deprecation. It isn't just because the usage formatting is broken.

…

On Mon, Apr 15, 2024, 7:31 AM Petr Viktorin ***@***.***> wrote: This does not handle nested exclusive groups correctly. Nested exclusive groups are deprecated, but still supported for now. For example: import argparse parser = argparse.ArgumentParser() g=parser.add_mutually_exclusive_group() g.add_argument('--spam') gg=g.add_mutually_exclusive_group() gg.add_argument('--hax') gg.add_argument('--hex') g.add_argument('--eggs') parser.add_argument('--num') print(parser.format_usage()) Without this change, the output is not ideal, but technically correct: /tmp/repro.py:5: DeprecationWarning: Nesting mutually exclusive groups is deprecated. gg=g.add_mutually_exclusive_group() usage: repro.py [-h] [--spam SPAM | [--hax HAX | --hex HEX |] --eggs EGGS] [--num NUM] But with this change, the argument added at the end (--num) joins the group: /tmp/repro.py:5: DeprecationWarning: Nesting mutually exclusive groups is deprecated. gg=g.add_mutually_exclusive_group() usage: repro.py [-h] [--spam SPAM | [--hax HAX | --hex HEX] | --eggs EGGS | [--num NUM]] One fix would be to pad the new sequence with None to keep the original length when replacing parts[start:end] at the end. That way the meaning of indices wouldn't change — Reply to this email directly, view it on GitHub <#105039 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAITB6ES7PAKCXDOQTHIGH3Y5PQDPAVCNFSM6AAAAAAYRZ6D3CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJXGAYDINRSHE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[encukou]

I'm aware that nested groups are broken and deprecated. But, their behaviour should not be made worse.

This PR, as it is, breaks unrelated usage formatting in the presence of nested groups: the --num above is not part of any group, yet it's formatted as if it was.
(Otherwise, in all cases I could think of so far, the output is the same as before, or better -- in particular it avoids an extra | at the end of a group.)

[hamdanal]

Thank you both for the valuable feedback, I've addressed the case of nested groups and tested several variations with/without suppressed actions locally and they all produce the correct usage text now.

[encukou]

Thank you for the fix!