Skip to content

GH-95468: Prevent argparse from removing -- in cases where used to delineate positional args #124145

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
wants to merge 13 commits into from

Conversation

savannahostrowski
Copy link
Member

@savannahostrowski savannahostrowski commented Sep 16, 2024

@savannahostrowski
Copy link
Member Author

Perhaps @serhiy-storchaka you can take a look? I think you updated this conditional and added some additional tests for -- in #114814

@rruuaanng

This comment was marked as off-topic.

@serhiy-storchaka serhiy-storchaka self-requested a review September 18, 2024 06:34
@serhiy-storchaka
Copy link
Member

This does not work for nargs='+' or nargs=3. Also, I afraid that this does not fix the issue for several positional arguments -- only the first -- should be removed.

This issue may require a fix at different level. I'll try to resolve it.

@savannahostrowski
Copy link
Member Author

Ah, thanks for the review, @serhiy-storchaka. It was my mistake to expect tests to catch some of these. I can take another look and add additional tests here.

@savannahostrowski
Copy link
Member Author

@serhiy-storchaka I've updated the PR. Does this cover the test cases you were thinking of?

Or do you already have another PR in progress?

@serhiy-storchaka
Copy link
Member

This is not enough. p.parse_args(['--', '--', 'a', '--', 'b']) swallows the double dash between 'a' and 'b'. This issue can only be solved at the higher level. See #124233.

@serhiy-storchaka
Copy link
Member

I am afraid that my reaction may not have seemed very friendly to you. In this case, I apologize and make sure that the reason for this is only my bad English and my fatigue. I didn't mean anything bad. It's just that in this case your approach was a dead end, this problem needed to be solved at another level. I've already spent half a day creating a solution, and creating a new PR was much easier than leading you to incrementally modify your PR. It happens to everyone, even major developers close their PRs. My main mistake was that I did not know words how to say this to you.

I look forward to working with you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants