bpo-39546: argparse: Honor allow_abbrev=False for specified prefix_chars#18337
bpo-39546: argparse: Honor allow_abbrev=False for specified prefix_chars#18337miss-islington merged 3 commits intopython:masterfrom
Conversation
When `allow_abbrev` was first added, disabling the abbreviation of long options broke the grouping of short flags (bpo-26967). 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`.
|
Thanks for the report and fix! |
IMO, it would be better if the |
|
@encukou @shihai1991 Thanks for the review. Your suggestions on how specifically to change the tests conflicted. I've gone with the first suggestion of adding a dedicated test. |
|
Well, a test similar to |
Sure, I'm happy to do that here. I wonder, though, whether it'd be better to just modify Edit: Actually "switch one of its short options to use |
Not a strong one, but I would prefer separate tests for one feature and a combination of features. |
|
Kyle, thanks for update. |
|
@shihai1991 Thanks for taking another look! |
|
You are welcome. cc @rhettinger Hi, raymond, pls review this PR if you have free time. |
|
Thanks @kyleam for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8. |
|
GH-18543 is a backport of this pull request to the 3.8 branch. |
…fix_chars (GH-18337) (GH-18543) 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> https://bugs.python.org/issue39546 Automerge-Triggered-By: @encukou
7 participants
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
Automerge-Triggered-By: @encukou