[3.8] bpo-39546: argparse: Honor allow_abbrev=False for specified prefix_chars (GH-18337)#18543
Merged
miss-islington merged 1 commit intopython:3.8from Feb 18, 2020
Merged
Conversation
…ars (pythonGH-18337) When `allow_abbrev` was first added, disabling the abbreviation of long options broke the grouping of short flags ([bpo-26967](https://bugs.python.org/issue26967)). As a fix, b1e4d1b (contained in v3.8) ignores `allow_abbrev=False` for a given argument string if the string does _not_ start with "--" (i.e. it doesn't look like a long option). This fix, however, doesn't take into account that long options can start with alternative characters specified via `prefix_chars`, introducing a regression: `allow_abbrev=False` has no effect on long options that start with an alternative prefix character. The most minimal fix would be to replace the "starts with --" check with a "starts with two prefix_chars characters". But `_get_option_tuples` already distinguishes between long and short options, so let's instead piggyback off of that check by moving the `allow_abbrev` condition into `_get_option_tuples`. https://bugs.python.org/issue39546 (cherry picked from commit 8edfc47) Co-authored-by: Kyle Meyer <kyle@kyleam.com>
Contributor
Author
|
@kyleam: Status check is done, and it's a success ✅ . |
1 similar comment
Contributor
Author
|
@kyleam: Status check is done, and it's a success ✅ . |
Codecov Report
@@ Coverage Diff @@
## 3.8 #18543 +/- ##
==========================================
+ Coverage 82.08% 82.10% +0.02%
==========================================
Files 1919 1918 -1
Lines 582193 576875 -5318
Branches 43736 43736
==========================================
- Hits 477866 473620 -4246
+ Misses 94749 93683 -1066
+ Partials 9578 9572 -6
Continue to review full report at Codecov.
|
Contributor
Author
|
@kyleam: Status check is done, and it's a success ✅ . |
2 similar comments
Contributor
Author
|
@kyleam: Status check is done, and it's a success ✅ . |
Contributor
Author
|
@kyleam: Status check is done, and it's a success ✅ . |
encukou
approved these changes
Feb 18, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When
allow_abbrevwas first added, disabling the abbreviation oflong options broke the grouping of short flags (bpo-26967). As a fix,
b1e4d1b (contained in v3.8) ignores
allow_abbrev=Falsefor agiven argument string if the string does not start with "--"
(i.e. it doesn't look like a long option).
This fix, however, doesn't take into account that long options can
start with alternative characters specified via
prefix_chars,introducing a regression:
allow_abbrev=Falsehas no effect on longoptions that start with an alternative prefix character.
The most minimal fix would be to replace the "starts with --" check
with a "starts with two prefix_chars characters". But
_get_option_tuplesalready distinguishes between long and shortoptions, so let's instead piggyback off of that check by moving the
allow_abbrevcondition into_get_option_tuples.https://bugs.python.org/issue39546
(cherry picked from commit 8edfc47)
Co-authored-by: Kyle Meyer kyle@kyleam.com
https://bugs.python.org/issue39546
Automerge-Triggered-By: @encukou