Skip to content

argparse removing more "--" than it should #81691

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

Closed
jol mannequin opened this issue Jul 5, 2019 · 10 comments
Closed

argparse removing more "--" than it should #81691

jol mannequin opened this issue Jul 5, 2019 · 10 comments
Labels
3.12 only security fixes 3.13 bugs and security fixes 3.14 bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@jol
Copy link
Mannequin

jol mannequin commented Jul 5, 2019

BPO 37510
Nosy @ericvsmith

Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2019-07-05.20:45:30.760>
labels = ['3.7', 'type-bug', 'library']
title = 'argparse removing more "--" than it should'
updated_at = <Date 2019-07-07.19:16:53.792>
user = 'https://bugs.python.org/jol'

bugs.python.org fields:

activity = <Date 2019-07-07.19:16:53.792>
actor = 'paul.j3'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2019-07-05.20:45:30.760>
creator = 'jol'
dependencies = []
files = []
hgrepos = []
issue_num = 37510
keywords = []
message_count = 9.0
messages = ['347378', '347379', '347407', '347413', '347434', '347435', '347437', '347442', '347478']
nosy_count = 3.0
nosy_names = ['eric.smith', 'paul.j3', 'jol']
pr_nums = []
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue37510'
versions = ['Python 3.7']

Linked PRs

@jol
Copy link
Mannequin Author

jol mannequin commented Jul 5, 2019

$ python -c '
import argparse
p = argparse.ArgumentParser()
p.add_argument("first_arg")
p.add_argument("args", nargs="*")
r = p.parse_args(["foo", "--", "bar", "--", "baz", "--", "zap"])
print(r.first_arg + " " + " ".join(r.args))
'                        

returns:

foo bar baz -- zap

when I think it should return:

foo bar -- baz -- zap

@jol jol mannequin added 3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Jul 5, 2019
@jol
Copy link
Mannequin Author

jol mannequin commented Jul 5, 2019

Sorry, I forgot to add details on my machine.

Python: 3.7.3
OS: Archlinux

@jol
Copy link
Mannequin Author

jol mannequin commented Jul 6, 2019

To be clear, my opinion is that a single call of parse_args() should only ever remove the first "--". Right now, it seems that it removes the first of each argument group, as determined by nargs, I guess.

@paulj3
Copy link
Mannequin

paulj3 mannequin commented Jul 6, 2019

There are earlier bug/issues about the '--'.

Also look at the parser code itself. Keep in mind that parsing is done in two passes - once to identify flags versus arguments ('O/A') and then to allocate strings to arguments. I don't recall when '--' is being handled, possibly in both.

@jol
Copy link
Mannequin Author

jol mannequin commented Jul 6, 2019

There are earlier bug/issues about the '--'.

Yes, there are:

https://bugs.python.org/issue9571
https://bugs.python.org/issue22223
https://bugs.python.org/issue14364

But this one seems separate. Though they're related, they don't seem like duplicates, so that's why I thought I'd make this one.

Also look at the parser code itself. Keep in mind that parsing is done in two passes - once to identify flags versus arguments ('O/A') and then to allocate strings to arguments. I don't recall when '--' is being handled, possibly in both.

I'm not sure what your point is here. I did take a quick look at the code, yesterday. I think the identification part you mention is done right. It marks 'O/A' as you mention. Then, when it sees "--", it marks it "-", and the rest of the arguments are marked as "A".

I didn't look at the start of the code of the second pass or how it is concretely linked to the first pass. However, I did see that in the example I gave, _get_values() in argparse.py gets called twice (apparently once per argument group as determined by nargs, I guess) to remove the "--" present in arg_strings. The first time, arg_strings is ["foo", "--"] and the second time it's ["bar", "--", "baz", "--", "zap"].

So, that's what happens, and where part of the fix should probably be. I don't think the removal of "--" should happen in a function that gets called multiple times. Though I didn't spend the time to see where the code should be positioned, I can only imagine the correct behavior would be to remove the argument marked as "-" by the first pass mentioned.

I didn't mention this yesterday, because I figured there wouldn't be much value in sharing incomplete research like this, as opposed to a patch. I didn't want to influence the work of whoever chose to invest time in this for a proper fix.

@jol
Copy link
Mannequin Author

jol mannequin commented Jul 6, 2019

to remove the "--" present in arg_strings

*to remove the first "--" present...

@paulj3
Copy link
Mannequin

paulj3 mannequin commented Jul 6, 2019

I looked at this issue way back, in 2013:

https://bugs.python.org/issue13922

I probably shouldn't have tacked this on to a closed issue.

@jol
Copy link
Mannequin Author

jol mannequin commented Jul 6, 2019

Maybe I can find the time to make a patch this weekend (either today or tomorrow). I hope I'm not underestimating this somehow, but I don't think this would take too long. The only issue I can foresee is in disagreement of what the correct behavior should be, which is why I gave my opinion that a single call of parse_args() should only ever remove a single "--".

If I don't submit a patch by Monday (PDT), everyone should assume I decided not to tackle this.

By the way, does this issue tracking platform support submitting to the issue thread by email? Maybe, I'll try that.

@paulj3
Copy link
Mannequin

paulj3 mannequin commented Jul 7, 2019

https://bugs.python.org/file29845/dbldash.patch

while written against an earlier version of argparse, does what you want. I moved the '--' removal out of _get_values and into consume_positionals.

Note that a full patch should include test cases and documentation changes if any.

Also now github pull requests are the preferred patching route. But don't expect rapid response on this. The issue has been around for a long time without causing too many complaints.

Backward compatibility is always a concern when making changes to core functionality. We don't want to mess with someone's working code.
Though I doubt if there are many users who count on multiple '--' removals, one way or other. The argparse docs explicity calls this a 'pseudo-argument'.

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue Sep 19, 2024
…arse

Only the first one has now been removed, all subsequent ones are now
taken literally.
@savannahostrowski savannahostrowski added 3.12 only security fixes 3.13 bugs and security fixes 3.14 bugs and security fixes and removed 3.7 (EOL) end of life labels Sep 19, 2024
serhiy-storchaka added a commit that referenced this issue Sep 20, 2024
…H-124233)

Only the first one has now been removed, all subsequent ones are now
taken literally.
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 20, 2024
…arse (pythonGH-124233)

Only the first one has now been removed, all subsequent ones are now
taken literally.
(cherry picked from commit aae1267)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Sep 20, 2024
…arse (pythonGH-124233)

Only the first one has now been removed, all subsequent ones are now
taken literally.
(cherry picked from commit aae1267)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
serhiy-storchaka added a commit that referenced this issue Sep 20, 2024
…parse (GH-124233) (GH-124267)

Only the first one has now been removed, all subsequent ones are now
taken literally.
(cherry picked from commit aae1267)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
savannahostrowski pushed a commit to savannahostrowski/cpython that referenced this issue Sep 22, 2024
…arse (pythonGH-124233)

Only the first one has now been removed, all subsequent ones are now
taken literally.
savannahostrowski pushed a commit to savannahostrowski/cpython that referenced this issue Sep 22, 2024
…arse (pythonGH-124233)

Only the first one has now been removed, all subsequent ones are now
taken literally.
@serhiy-storchaka
Copy link
Member

Fixed by #124233.

@github-project-automation github-project-automation bot moved this from Bugs to Doc issues in Argparse issues Sep 25, 2024
serhiy-storchaka added a commit that referenced this issue Oct 7, 2024
…parse (GH-124233) (GH-124266)

Only the first one has now been removed, all subsequent ones are now
taken literally.
(cherry picked from commit aae1267)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.12 only security fixes 3.13 bugs and security fixes 3.14 bugs and security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
Status: Doc issues
Development

No branches or pull requests

2 participants