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-39546: argparse: Honor allow_abbrev=False for specified prefix_chars #18337
bpo-39546: argparse: Honor allow_abbrev=False for specified prefix_chars #18337
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 |
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
When
allow_abbrev
was 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=False
for 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=False
has 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_tuples
already distinguishes between long and shortoptions, so let's instead piggyback off of that check by moving the
allow_abbrev
condition into_get_option_tuples
.https://bugs.python.org/issue39546
Automerge-Triggered-By: @encukou