Skip to content

gh-148615: Handle -- end-of-options separator in pdb argument parsing#148624

Merged
gaogaotiantian merged 9 commits intopython:mainfrom
Shrey-N:e
May 6, 2026
Merged

gh-148615: Handle -- end-of-options separator in pdb argument parsing#148624
gaogaotiantian merged 9 commits intopython:mainfrom
Shrey-N:e

Conversation

@Shrey-N
Copy link
Copy Markdown
Contributor

@Shrey-N Shrey-N commented Apr 15, 2026

The switch from getopt to argparse in gh-125115, python -m pdb rejects the standard -- end-of-options separator. Makes it impossible to pass arguments that share names with pdb flags (like -c) to the target script.

The root cause is that pdb's argparse parser has no positional arguments defined, so parse_known_args() does not consume -- and returns it in the remainder list. The manual post processing logic in parse_args() then incorrectly treats it as an unrecognized flag since it starts with -.

This adds a check for -- in the elif args[0].startswith('-'). When found, the separator is consumed and parsing continues to the script name. If no script follows, an error is raised. All other unrecognized flags continue to produce the existing error message.

Closes gh-148615 :)

@gaogaotiantian
Copy link
Copy Markdown
Member

Thanks for fixing this. We need some regression test on this. We need to test the specific case reported, and also some others. I wonder if -- works as expected when it's on other places.

For the code itself, let's move -- check to it's own case (not under args[0].startswith), less nested logic.

@Shrey-N
Copy link
Copy Markdown
Contributor Author

Shrey-N commented Apr 16, 2026

Thank you, that makes sense, I will update the PR asap! :)

@Shrey-N
Copy link
Copy Markdown
Contributor Author

Shrey-N commented Apr 22, 2026

Hiya @gaogaotiantian, is there anything else needed from my side to move this forward?

@gaogaotiantian
Copy link
Copy Markdown
Member

Sorry I was kind of busy recently and did not get a chance to take a look :)

I don't think this issue worth a while new test class for it. Some test cases in a test method would do. The new test class does not cover anything that the integration test can't cover. Let's just put all the interesting test cases in the same method. It's a relatively corner case and we don't need to overreact.

@python-cla-bot
Copy link
Copy Markdown

python-cla-bot Bot commented Apr 27, 2026

All commit authors signed the Contributor License Agreement.

CLA signed

@Shrey-N Shrey-N force-pushed the e branch 3 times, most recently from 6ab1275 to 3cb7cdf Compare April 28, 2026 12:21
@Shrey-N
Copy link
Copy Markdown
Contributor Author

Shrey-N commented Apr 28, 2026

Hiya @gaogaotiantian I followed your suggestion, and also simplified the news entry, the old one wasn't up to the mark

@gaogaotiantian
Copy link
Copy Markdown
Member

What's the difference between the two test cases? It looks to me that they are testing similar things? Can we combine them together and maybe remove duplicated test cases?

@Shrey-N
Copy link
Copy Markdown
Contributor Author

Shrey-N commented Apr 30, 2026

You are right @gaogaotiantian, I removed the redundant one (test_run_script_with_double_dash)

@read-the-docs-community
Copy link
Copy Markdown

read-the-docs-community Bot commented Apr 30, 2026

Documentation build overview

📚 cpython-previews | 🛠️ Build #32526079 | 📁 Comparing c2a7d7f against main (55c9d60)

  🔍 Preview build  

5 files changed · ± 5 modified

± Modified

@gaogaotiantian
Copy link
Copy Markdown
Member

Could you resolve the conflict?

@Shrey-N
Copy link
Copy Markdown
Contributor Author

Shrey-N commented May 4, 2026

Sorry for the delay @gaogaotiantian, I think it's resolved now :)

@gaogaotiantian gaogaotiantian merged commit 970b043 into python:main May 6, 2026
55 checks passed
@gaogaotiantian gaogaotiantian added the needs backport to 3.14 bugs and security fixes label May 6, 2026
@miss-islington-app
Copy link
Copy Markdown

Thanks @Shrey-N for the PR, and @gaogaotiantian for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

@miss-islington-app
Copy link
Copy Markdown

Sorry, @Shrey-N and @gaogaotiantian, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 970b0433f498fd0d048b1f50229a7f0fb4d9e07a 3.14

@bedevere-app
Copy link
Copy Markdown

bedevere-app Bot commented May 6, 2026

GH-149442 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label May 6, 2026
gaogaotiantian added a commit that referenced this pull request May 6, 2026
#149442)

[3.14] gh-148615: Handle `--` separator in pdb argument parsing (GH-148624)
(cherry picked from commit 970b043)

Co-authored-by: Shrey Naithani <shrey.naithani@shelllite.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pdb: -- argument separator is rejected

3 participants