Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-96310: Fix a traceback in argparse when all options in a mutually exclusive group are suppressed #96311

Conversation

dmach
Copy link
Contributor

@dmach dmach commented Aug 26, 2022

Reproducer depends on terminal size - the traceback occurs when there's
an option long enough so the usage line doesn't fit the terminal width.
Option order is also important for reproducibility.

Excluding empty groups (with all options suppressed) from inserts
fixes the problem.

@bedevere-bot
Copy link

Most changes to Python require a NEWS entry.

Please add it using the blurb_it web app or the blurb command-line tool.

@cpython-cla-bot
Copy link

cpython-cla-bot bot commented Aug 26, 2022

All commit authors signed the Contributor License Agreement.
CLA signed

…ually exclusive group are suppressed

Reproducer depends on terminal size - the traceback occurs when there's
an option long enough so the usage line doesn't fit the terminal width.
Option order is also important for reproducibility.

Excluding empty groups (with all options suppressed) from inserts
fixes the problem.
@dmach dmach force-pushed the fix-argparse-subparser_all_mutually_exclusive_group_members_suppressed branch from 0d6958c to 616c5fe Compare August 26, 2022 13:52
@dmach dmach changed the title gh-96310: Fix a traceback when all options in a mutually exclusive group are suppressed gh-96310: Fix a traceback in argparse when all options in a mutually exclusive group are suppressed Aug 26, 2022
hamdanal added a commit to hamdanal/cpython that referenced this pull request May 28, 2023
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 `SUPPRESS`ed arguments
- metavars with whitespace
- metavars with brackets or paranthesis

Solution
========

The following two comments summarize the solution:
- python#82091 (comment)
- python#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:
- Closes python#62090
- Closes python#62549
- Closes python#77048
- Closes python#82091
- Closes python#89743
- Closes python#96310
- Closes python#98666

These PRs become obsolete:
- Closes python#15372
- Closes python#96311
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@serhiy-storchaka
Copy link
Member

Thank you for your contribution @dmath and sorry for the delay. I resolved the merge conflict and made some simple tweaks.

@serhiy-storchaka serhiy-storchaka merged commit 5f7df88 into python:main Feb 21, 2024
36 checks passed
@miss-islington-app
Copy link

Thanks @dmach for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 21, 2024
…ually exclusive group are suppressed (pythonGH-96311)

Reproducer depends on terminal size - the traceback occurs when there's
an option long enough so the usage line doesn't fit the terminal width.
Option order is also important for reproducibility.

Excluding empty groups (with all options suppressed) from inserts
fixes the problem.
(cherry picked from commit 5f7df88)

Co-authored-by: Daniel Mach <daniel.mach@suse.com>
@bedevere-app
Copy link

bedevere-app bot commented Feb 21, 2024

GH-115767 is a backport of this pull request to the 3.12 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 21, 2024
…ually exclusive group are suppressed (pythonGH-96311)

Reproducer depends on terminal size - the traceback occurs when there's
an option long enough so the usage line doesn't fit the terminal width.
Option order is also important for reproducibility.

Excluding empty groups (with all options suppressed) from inserts
fixes the problem.
(cherry picked from commit 5f7df88)

Co-authored-by: Daniel Mach <daniel.mach@suse.com>
@bedevere-app bedevere-app bot removed the needs backport to 3.12 bug and security fixes label Feb 21, 2024
@bedevere-app
Copy link

bedevere-app bot commented Feb 21, 2024

GH-115768 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Feb 21, 2024
serhiy-storchaka pushed a commit that referenced this pull request Feb 21, 2024
…tually exclusive group are suppressed (GH-96311) (GH-115768)

Reproducer depends on terminal size - the traceback occurs when there's
an option long enough so the usage line doesn't fit the terminal width.
Option order is also important for reproducibility.

Excluding empty groups (with all options suppressed) from inserts
fixes the problem.
(cherry picked from commit 5f7df88)

Co-authored-by: Daniel Mach <daniel.mach@suse.com>
serhiy-storchaka pushed a commit that referenced this pull request Feb 21, 2024
…tually exclusive group are suppressed (GH-96311) (GH-115767)

Reproducer depends on terminal size - the traceback occurs when there's
an option long enough so the usage line doesn't fit the terminal width.
Option order is also important for reproducibility.

Excluding empty groups (with all options suppressed) from inserts
fixes the problem.
(cherry picked from commit 5f7df88)

Co-authored-by: Daniel Mach <daniel.mach@suse.com>
woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
…ually exclusive group are suppressed (pythonGH-96311)

Reproducer depends on terminal size - the traceback occurs when there's
an option long enough so the usage line doesn't fit the terminal width.
Option order is also important for reproducibility.

Excluding empty groups (with all options suppressed) from inserts
fixes the problem.
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
…ually exclusive group are suppressed (pythonGH-96311)

Reproducer depends on terminal size - the traceback occurs when there's
an option long enough so the usage line doesn't fit the terminal width.
Option order is also important for reproducibility.

Excluding empty groups (with all options suppressed) from inserts
fixes the problem.
encukou pushed a commit that referenced this pull request May 7, 2024
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 `SUPPRESS`ed arguments
- metavars with whitespace
- metavars with brackets or paranthesis

Solution
========

The following two comments summarize the solution:
- #82091 (comment)
- #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 GitHub issues:
-  #62090
-  #62549
-  #77048
-  #82091
-  #89743
-  #96310
-  #98666

These PRs become obsolete:
-  #15372
-  #96311
SonicField pushed a commit to SonicField/cpython that referenced this pull request May 8, 2024
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 `SUPPRESS`ed arguments
- metavars with whitespace
- metavars with brackets or paranthesis

Solution
========

The following two comments summarize the solution:
- python#82091 (comment)
- python#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 GitHub issues:
-  python#62090
-  python#62549
-  python#77048
-  python#82091
-  python#89743
-  python#96310
-  python#98666

These PRs become obsolete:
-  python#15372
-  python#96311
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.

None yet

3 participants