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

bpo-26967: fix flag grouping with allow_abbrev=False #14316

Merged
merged 1 commit into from Jul 14, 2019
Merged

bpo-26967: fix flag grouping with allow_abbrev=False #14316

merged 1 commit into from Jul 14, 2019

Conversation

Zac-HD
Copy link
Contributor

@Zac-HD Zac-HD commented Jun 23, 2019

The allow_abbrev option for ArgumentParser is documented and intended to disable support for unique prefixes of --options, which may sometimes be ambiguous due to deferred parsing.

However, the initial implementation also broke parsing of grouped short flags, such as -ab meaning -a -b (or -a=b). Checking the argument for a leading -- before rejecting it fixes this.

This was prompted by pytest-dev/pytest#5469, so a backport to at least 3.8 would be great 😄
And this is my first PR to CPython, so please let me know if I've missed anything!

https://bugs.python.org/issue26967

The `allow_abbrev` option for ArgumentParser is documented and
intended to disable support for unique prefixes of --options,
which may sometimes be ambiguous due to deferred parsing.

However, the initial implementation also broke parsing of
grouped short flags, such as `-ab` meaning `-a -b`.
Checking for a leading `--` fixes this.
@mangrisano
Copy link
Contributor

/cc @bethard

Copy link
Contributor

@mangrisano mangrisano left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot for providing the tests. They are very useful. :)

@Meinersbur
Copy link

Thank you for taking care of the issue.

@@ -182,6 +182,10 @@ ArgumentParser objects
.. versionchanged:: 3.5
*allow_abbrev* parameter was added.

.. versionchanged:: 3.8
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
.. versionchanged:: 3.8
.. versionchanged:: 3.9

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm still hoping that this will be merged into the 3.8 branch before 3.8.0, but I guess we can adjust the number later.

@encukou
Copy link
Member

encukou commented Jul 13, 2019

I talked to a more experienced core dev, and we agreed that this a bugfix appropriate for 3.8.
Sorry for the noise!

@Zac-HD
Copy link
Contributor Author

Zac-HD commented Jul 13, 2019

No worries! I'm just glad I can make a contribution, and getting it into 3.8 is even better 😄

@miss-islington
Copy link
Contributor

Thanks @Zac-HD for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 14, 2019
The `allow_abbrev` option for ArgumentParser is documented and intended to disable support for unique prefixes of --options, which may sometimes be ambiguous due to deferred parsing.

However, the initial implementation also broke parsing of grouped short flags, such as `-ab` meaning `-a -b` (or `-a=b`).  Checking the argument for a leading `--` before rejecting it fixes this.

This was prompted by pytest-dev/pytestGH-5469, so a backport to at least 3.8 would be great 😄
And this is my first PR to CPython, so please let me know if I've missed anything!

https://bugs.python.org/issue26967
(cherry picked from commit dffca9e)

Co-authored-by: Zac Hatfield-Dodds <Zac-HD@users.noreply.github.com>
@bedevere-bot bedevere-bot removed the needs backport to 3.8 only security fixes label Jul 14, 2019
@bedevere-bot
Copy link

GH-14759 is a backport of this pull request to the 3.8 branch.

@Zac-HD Zac-HD deleted the disallow-abbrev-fix branch July 14, 2019 05:37
encukou pushed a commit that referenced this pull request Jul 14, 2019
…4759)

The `allow_abbrev` option for ArgumentParser is documented and intended to disable support for unique prefixes of --options, which may sometimes be ambiguous due to deferred parsing.

However, the initial implementation also broke parsing of grouped short flags, such as `-ab` meaning `-a -b` (or `-a=b`).  Checking the argument for a leading `--` before rejecting it fixes this.

This was prompted by pytest-dev/pytestGH-5469, so a backport to at least 3.8 would be great 😄
And this is my first PR to CPython, so please let me know if I've missed anything!

https://bugs.python.org/issue26967
(cherry picked from commit dffca9e)

Co-authored-by: Zac Hatfield-Dodds <Zac-HD@users.noreply.github.com>
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
The `allow_abbrev` option for ArgumentParser is documented and intended to disable support for unique prefixes of --options, which may sometimes be ambiguous due to deferred parsing.

However, the initial implementation also broke parsing of grouped short flags, such as `-ab` meaning `-a -b` (or `-a=b`).  Checking the argument for a leading `--` before rejecting it fixes this.

This was prompted by pytest-dev/pytest#5469, so a backport to at least 3.8 would be great 😄  
And this is my first PR to CPython, so please let me know if I've missed anything!


https://bugs.python.org/issue26967
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
The `allow_abbrev` option for ArgumentParser is documented and intended to disable support for unique prefixes of --options, which may sometimes be ambiguous due to deferred parsing.

However, the initial implementation also broke parsing of grouped short flags, such as `-ab` meaning `-a -b` (or `-a=b`).  Checking the argument for a leading `--` before rejecting it fixes this.

This was prompted by pytest-dev/pytest#5469, so a backport to at least 3.8 would be great 😄  
And this is my first PR to CPython, so please let me know if I've missed anything!


https://bugs.python.org/issue26967
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

8 participants