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.

Sign up for GitHub

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

Jump to bottom

gh-101979: fix a bug that parentheses in metavar argument of add_argument() were dropped #102318

Merged
merged 8 commits into from
Mar 5, 2023
Merged

gh-101979: fix a bug that parentheses in metavar argument of add_argument() were dropped #102318

merged 8 commits into from
Mar 5, 2023

Conversation

yeojin-dev
Copy link
Contributor

@yeojin-dev yeojin-dev commented Feb 28, 2023 •
edited by bedevere-bot
Loading

Remove a regular expression that removes parentheses. Instead, add parentheses only if you use two or more non required and non suppressed actions.

@bedevere-bot bedevere-bot mentioned this pull request Feb 28, 2023
Closed
@corona10 corona10 self-assigned this Feb 28, 2023
@corona10 corona10 self-requested a review February 28, 2023 11:12
@corona10 corona10 assigned JelleZijlstra and unassigned corona10 Mar 1, 2023
@corona10
Copy link
Member

corona10 commented Mar 1, 2023

@JelleZijlstra
Can you please take a look? I think that I am not argparse expert to review this PR.
@yeojin-dev is one of our internal sprint members :)

corona10
corona10 reviewed Mar 1, 2023
View reviewed changes
Lib/argparse.py Outdated Show resolved Hide resolved
corona10
corona10 reviewed Mar 1, 2023
View reviewed changes
Lib/argparse.py Outdated Show resolved Hide resolved
JelleZijlstra
JelleZijlstra reviewed Mar 3, 2023
View reviewed changes
Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

Looks good apart from minor

Lib/test/test_argparse.py
@@ -3764,6 +3764,28 @@ class TestHelpUsage(HelpTestCase):
version = ''


class TestHelpUsageWithParentheses(HelpTestCase):
Copy link
Member

Choose a reason for hiding this comment

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

I don't see any tests that actually add parentheses for a group with more than one action. Could you add one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea. But I think the testcase you want already exists using the code below. Please review it.

cpython/Lib/test/test_argparse.py

Line 2777 in 66aa78c

usage: PROG [-h] (--bar BAR | --baz [BAZ])

Copy link
Member

Choose a reason for hiding this comment

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

You're right, thanks! There are a lot of tests, I must have glanced over that one.

@yeojin-dev @JelleZijlstra
Update Misc/NEWS.d/next/Library/2023-02-28-09-52-25.gh-issue-101979.o…
b05fb16 
…r3hXV.rst

Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@yeojin-dev
Copy link
Contributor Author

I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

: please review the changes made to this pull request.

@JelleZijlstra JelleZijlstra added needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes labels Mar 5, 2023
@JelleZijlstra JelleZijlstra merged commit 9a478be into python:main Mar 5, 2023
@miss-islington
Copy link
Contributor

Thanks @yeojin-dev for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@bedevere-bot
Copy link

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

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Mar 5, 2023
@bedevere-bot
Copy link

GH-102439 is a backport of this pull request to the 3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 5, 2023
@yeojin-dev @miss-islington
pythongh-101979: argparse: fix a bug where parentheses in metavar arg…
97fde49 
…ument of add_argument() were dropped (pythonGH-102318)

(cherry picked from commit 9a478be)

Co-authored-by: Yeojin Kim <yeojin.dev@gmail.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Mar 5, 2023
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Mar 5, 2023
@yeojin-dev @miss-islington
pythongh-101979: argparse: fix a bug where parentheses in metavar arg…
20dfd9a 
…ument of add_argument() were dropped (pythonGH-102318)

(cherry picked from commit 9a478be)

Co-authored-by: Yeojin Kim <yeojin.dev@gmail.com>
miss-islington added a commit that referenced this pull request Mar 5, 2023
@miss-islington @yeojin-dev
gh-101979: argparse: fix a bug where parentheses in metavar argument …
2a062f2 
…of add_argument() were dropped (GH-102318)

(cherry picked from commit 9a478be)

Co-authored-by: Yeojin Kim <yeojin.dev@gmail.com>
miss-islington added a commit that referenced this pull request Mar 5, 2023
@miss-islington @yeojin-dev
gh-101979: argparse: fix a bug where parentheses in metavar argument …
e748f9e 
…of add_argument() were dropped (GH-102318)

(cherry picked from commit 9a478be)

Co-authored-by: Yeojin Kim <yeojin.dev@gmail.com>
@yeojin-dev yeojin-dev deleted the gh-101979 branch March 5, 2023 22:40
hugovk pushed a commit to hugovk/cpython that referenced this pull request Mar 6, 2023
@yeojin-dev @hugovk
pythongh-101979: argparse: fix a bug where parentheses in metavar arg…
0e240a6 
…ument of add_argument() were dropped (python#102318)
carljm added a commit to carljm/cpython that referenced this pull request Mar 6, 2023
@carljm
Merge branch 'main' into nodeaddict
33dda2a 
* main: (21 commits)
  pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives in sub interpreters module (python#102472)
  pythongh-95672: Fix versionadded indentation of get_pagesize in test.rst (pythongh-102455)
  pythongh-102416: Do not memoize incorrectly loop rules in the parser (python#102467)
  pythonGH-101362: Optimise PurePath(PurePath(...)) (pythonGH-101667)
  pythonGH-101362: Check pathlib.Path flavour compatibility at import time (pythonGH-101664)
  pythonGH-101362: Call join() only when >1 argument supplied to pathlib.PurePath() (python#101665)
  pythongh-102444: Fix minor bugs in `test_typing` highlighted by pyflakes (python#102445)
  pythonGH-102341: Improve the test function for pow (python#102342)
  Fix unused classes in a typing test (pythonGH-102437)
  pythongh-101979: argparse: fix a bug where parentheses in metavar argument of add_argument() were dropped (python#102318)
  pythongh-102356: Add thrashcan macros to filter object dealloc (python#102426)
  Move around example in to_bytes() to avoid confusion (python#101595)
  pythonGH-97546: fix flaky asyncio `test_wait_for_race_condition` test (python#102421)
  pythongh-96821: Add config option `--with-strict-overflow` (python#96823)
  pythongh-101992: update pstlib module documentation (python#102133)
  pythongh-63301: Set exit code when tabnanny CLI exits on error (python#7699)
  pythongh-101863: Fix wrong comments in EUC-KR codec (pythongh-102417)
  pythongh-102302 Micro-optimize `inspect.Parameter.__hash__` (python#102303)
  pythongh-102179: Fix `os.dup2` error reporting for negative fds (python#102180)
  pythongh-101892: Fix `SystemError` when a callable iterator call exhausts the iterator (python#101896)
  ...
carljm added a commit to carljm/cpython that referenced this pull request Mar 7, 2023
@carljm
Merge branch 'main' into inlinecomp2
ecb313c 
* main: (37 commits)
  pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives in sub interpreters module (python#102472)
  pythongh-95672: Fix versionadded indentation of get_pagesize in test.rst (pythongh-102455)
  pythongh-102416: Do not memoize incorrectly loop rules in the parser (python#102467)
  pythonGH-101362: Optimise PurePath(PurePath(...)) (pythonGH-101667)
  pythonGH-101362: Check pathlib.Path flavour compatibility at import time (pythonGH-101664)
  pythonGH-101362: Call join() only when >1 argument supplied to pathlib.PurePath() (python#101665)
  pythongh-102444: Fix minor bugs in `test_typing` highlighted by pyflakes (python#102445)
  pythonGH-102341: Improve the test function for pow (python#102342)
  Fix unused classes in a typing test (pythonGH-102437)
  pythongh-101979: argparse: fix a bug where parentheses in metavar argument of add_argument() were dropped (python#102318)
  pythongh-102356: Add thrashcan macros to filter object dealloc (python#102426)
  Move around example in to_bytes() to avoid confusion (python#101595)
  pythonGH-97546: fix flaky asyncio `test_wait_for_race_condition` test (python#102421)
  pythongh-96821: Add config option `--with-strict-overflow` (python#96823)
  pythongh-101992: update pstlib module documentation (python#102133)
  pythongh-63301: Set exit code when tabnanny CLI exits on error (python#7699)
  pythongh-101863: Fix wrong comments in EUC-KR codec (pythongh-102417)
  pythongh-102302 Micro-optimize `inspect.Parameter.__hash__` (python#102303)
  pythongh-102179: Fix `os.dup2` error reporting for negative fds (python#102180)
  pythongh-101892: Fix `SystemError` when a callable iterator call exhausts the iterator (python#101896)
  ...
This was referenced May 7, 2024
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corona10 corona10 corona10 left review comments

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes

Assignees

@JelleZijlstra JelleZijlstra

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yeojin-dev @corona10 @bedevere-bot @miss-islington @JelleZijlstra