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

argparse.ArgumentParser.add_mutually_exclusive_group : metavar create parenthesis undefined behavior #89743

Closed
AbcSxyZ mannequin opened this issue Oct 22, 2021 · 5 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@AbcSxyZ
Copy link
Mannequin

AbcSxyZ mannequin commented Oct 22, 2021

BPO 45580
Nosy @lysnikolaou, @pablogsal, @sobolevn, @AbcSxyZ

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2021-10-22.19:31:30.976>
labels = ['type-bug', 'library', '3.9', '3.10', '3.11']
title = 'argparse.ArgumentParser.add_mutually_exclusive_group : metavar create parenthesis undefined behavior'
updated_at = <Date 2021-10-25.16:32:34.545>
user = 'https://github.com/AbcSxyZ'

bugs.python.org fields:

activity = <Date 2021-10-25.16:32:34.545>
actor = 'paul.j3'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-10-22.19:31:30.976>
creator = 'AbcSxyZ'
dependencies = []
files = []
hgrepos = []
issue_num = 45580
keywords = []
message_count = 4.0
messages = ['404815', '404891', '404893', '404981']
nosy_count = 5.0
nosy_names = ['paul.j3', 'lys.nikolaou', 'pablogsal', 'sobolevn', 'AbcSxyZ']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue45580'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

@AbcSxyZ
Copy link
Mannequin Author

AbcSxyZ mannequin commented Oct 22, 2021

Hi,

I'm getting a kind of undefined behavior where parenthesis seem handled in a strange way. On display, it has a conflict between parenthesis of the option, and nested parenthesis within a metavar.

## Reproduction script

import argparse

def main():
    parser = argparse.ArgumentParser()

    group = parser.add_mutually_exclusive_group(required=True)

    group.add_argument("-p", "--path", metavar="/var/www/html", 
            help="DocumentRoot path")
    group.add_argument("-r", "--reverse", metavar="http)s(://Host:Port",
            help="Reverse proxy address")

    parser.add_argument("--last-args")
    return parser.parse_args()

main()

## Output of help menu

usage: crash.py [-h] (-p /var/www/html | -r http)s://Host:Port [--last-args LAST_ARGS]

## Expected behavior

usage: crash.py [-h] (-p /var/www/html | -r http)s(://Host:Port) [--last-args LAST_ARGS]

@AbcSxyZ AbcSxyZ mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.9 only security fixes labels Oct 22, 2021
@sobolevn
Copy link
Member

I confirm this happens on all recent Python versions.

The source of this problem is that argparse uses regex module to replace some substrings. Direct link: https://github.com/python/cpython/blame/8ce20bbdd6d2b1277a5e74154fcdcef2cb0fee49/Lib/argparse.py#L487

Quick debug showed that without this line these tests fail:

======================================================================
FAIL: test_help_when_required (test.test_argparse.TestMutuallyExclusiveFirstSuppressed)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython/Lib/test/test_argparse.py", line 2649, in test_help_when_required
    self.assertEqual(format_help(), textwrap.dedent(help))
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 'usage: PROG [-h] (-y)\n\noptions:\n  -h, --help  show this[42 chars]lp\n' != 'usage: PROG [-h] -y\n\noptions:\n  -h, --help  show this h[40 chars]lp\n'
- usage: PROG [-h] (-y)
?                  -  -
+ usage: PROG [-h] -y
  
  options:
    -h, --help  show this help message and exit
    -y          y help


======================================================================
FAIL: test_usage_when_required (test.test_argparse.TestMutuallyExclusiveFirstSuppressed)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython/Lib/test/test_argparse.py", line 2639, in test_usage_when_required
    self.assertEqual(format_usage(), textwrap.dedent(expected_usage))
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 'usage: PROG [-h] (-y)\n' != 'usage: PROG [-h] -y\n'
- usage: PROG [-h] (-y)
?                  -  -
+ usage: PROG [-h] -y


======================================================================
FAIL: test_help_when_required (test.test_argparse.TestMutuallyExclusiveFirstSuppressedParent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython/Lib/test/test_argparse.py", line 2649, in test_help_when_required
    self.assertEqual(format_help(), textwrap.dedent(help))
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 'usage: PROG [-h] (-y)\n\noptions:\n  -h, --help  show this[42 chars]lp\n' != 'usage: PROG [-h] -y\n\noptions:\n  -h, --help  show this h[40 chars]lp\n'
- usage: PROG [-h] (-y)
?                  -  -
+ usage: PROG [-h] -y
  
  options:
    -h, --help  show this help message and exit
    -y          y help


======================================================================
FAIL: test_usage_when_required (test.test_argparse.TestMutuallyExclusiveFirstSuppressedParent)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython/Lib/test/test_argparse.py", line 2639, in test_usage_when_required
    self.assertEqual(format_usage(), textwrap.dedent(expected_usage))
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 'usage: PROG [-h] (-y)\n' != 'usage: PROG [-h] -y\n'
- usage: PROG [-h] (-y)
?                  -  -
+ usage: PROG [-h] -y


----------------------------------------------------------------------
Ran 1672 tests in 23.258s

FAILED (failures=4)
test test_argparse failed
test_argparse failed (4 failures)

== Tests result: FAILURE ==

1 test failed:
    test_argparse

Total duration: 25.6 sec
Tests result: FAILURE

@sobolevn
Copy link
Member

Maybe instead we can show users something like:

usage: ex.py [-h] (-p '/var/www/html' | -r 'http)s(://Host:Port') [--last-args LAST_ARGS]

?

@sobolevn sobolevn added stdlib Python modules in the Lib dir 3.10 only security fixes 3.11 only security fixes type-bug An unexpected behavior, bug, or error and removed interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Oct 23, 2021
@paulj3
Copy link
Mannequin

paulj3 mannequin commented Oct 25, 2021

The usage formatting is fragile, with many associated bug reports. Until someone does a major rewrite, it is best to avoid special characters, especially () and [] in the dest or metavar.

Usage uses () to encolde mutually_exclusive_groups and [] to mark non-required arguments. Don't confuse your users (or argparse) with other uses of these characters.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
hamdanal added a commit to hamdanal/cpython that referenced this issue 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
encukou pushed a commit that referenced this issue 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
@encukou
Copy link
Member

encukou commented May 7, 2024

This was fixed in #102318.

@encukou encukou closed this as completed May 7, 2024
@github-project-automation github-project-automation bot moved this from Bugs to Doc issues in Argparse issues May 7, 2024
SonicField pushed a commit to SonicField/cpython that referenced this issue 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
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
Status: Doc issues
Development

No branches or pull requests

2 participants