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

Disallow abbreviated command-line options #5469

Merged
merged 1 commit into from Jun 25, 2019

Conversation

Zac-HD
Copy link
Member

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

This was previously allowed where the ArgumentParser thought it was unambiguous, because this could be incorrect due to delayed parsing of options for plugins.

Fixes #1149, fixes #3413, and fixes #4009 - it's been a known problem for quite a while, but this fix is only available on Python 3.5+... which means we can finally do it! Replaces #5467, in that it also defines MyOptionParser._parse_optional, in order to work around an upstream bug that disables combining short options, such as -vv.

We should avoid merging this until python/cpython#14316 is merged and we know that we don't need to do our conditional backport on 3.8.

@Zac-HD
Copy link
Member Author

Zac-HD commented Jun 24, 2019

Closing and reopening, in hopes of getting codecov unstuck:

@Zac-HD Zac-HD closed this Jun 24, 2019
@Zac-HD Zac-HD reopened this Jun 24, 2019
@pytest-dev pytest-dev deleted a comment from codecov bot Jun 24, 2019
changelog/1149.feature.rst Outdated Show resolved Hide resolved
changelog/1149.feature.rst Outdated Show resolved Hide resolved
changelog/1149.feature.rst Outdated Show resolved Hide resolved
src/_pytest/config/argparsing.py Outdated Show resolved Hide resolved
changelog/1149.feature.rst Outdated Show resolved Hide resolved
changelog/1149.feature.rst Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jun 25, 2019

Codecov Report

Merging #5469 into features will decrease coverage by 0.63%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           features    #5469      +/-   ##
============================================
- Coverage     94.48%   93.84%   -0.64%     
============================================
  Files           114      114              
  Lines         25528    25528              
  Branches       2482     2482              
============================================
- Hits          24119    23957     -162     
- Misses         1095     1251     +156     
- Partials        314      320       +6
Impacted Files Coverage Δ
testing/acceptance_test.py 97.98% <ø> (ø) ⬆️
src/_pytest/config/argparsing.py 88.2% <100%> (+0.1%) ⬆️
testing/test_capture.py 99.21% <100%> (ø) ⬆️
testing/test_parseopt.py 97.93% <100%> (-0.02%) ⬇️
testing/test_pastebin.py 100% <100%> (ø) ⬆️
testing/test_pytester.py 56.41% <0%> (-30.49%) ⬇️
testing/test_warnings.py 48.17% <0%> (-18.91%) ⬇️
src/_pytest/pytester.py 87.55% <0%> (-2.61%) ⬇️
src/_pytest/python.py 92.54% <0%> (-0.49%) ⬇️
testing/python/metafunc.py 86.54% <0%> (-0.35%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f57d40...d72fb73. Read the comment docs.

@nicoddemus
Copy link
Member

Great work! 👍

@Zac-HD Zac-HD merged commit 4d5780f into pytest-dev:features Jun 25, 2019
@Zac-HD Zac-HD deleted the disallow-abbrev branch June 25, 2019 12:39
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>
miss-islington pushed a commit to python/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/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
encukou pushed a commit to python/cpython 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

2 participants