-
-
Notifications
You must be signed in to change notification settings - Fork 31.2k
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
gh-124703: Change back to raising bdb.BdbQuit when exiting pdb in 'inline' mode in a REPL session #130395
base: main
Are you sure you want to change the base?
Conversation
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
pdb.set_trace(commands=['x * 3', 'q']) | ||
print('Afterward') | ||
""" | ||
p = spawn_repl() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we import the spawn_repl
from test_repl
here? So we don't need to keep a copy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I wasn't sure about this. I see three options:
- Keep a copy of
spawn_repl
here - it duplicates some non-obvious code and risks things getting out of sync - Do
from test.test_repl import spawn_repl
. I tried it and it works. But this introduces a dependency/import between test files, which seems discouraged and rare from grepping through the test directory. - Move
spawn_repl
from test_repl.py to somewhere in test/support and import fromtest.support
in both test_repl and test_pdb. This is more invasive and would involve touching at least two other files, but probably more maintainable and keeps test files independent/isolated.
Which do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For 2, we can do that inside the test function, so there won't be any dependencies. It's not that common to import a util function from other tests, that's true, but in this case I think it's fine. It's a single specific test using repl.
Misc/NEWS.d/next/Library/2025-02-21-09-05-44.gh-issue-124703.AMJD4Y.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
…MJD4Y.rst Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
This change should mean it's again possible to call breakpoint() in a REPL session and return to the REPL session with q/quit/exit/EOF.
The test has a slight workaround/hack because I couldn't figure out how to call breakpoint() and simulate using pdb interactively within a simulated REPL session in a subprocess. After set_trace() was called, pdb would receive an EOF and exit before it could receive further commands.
If there's interest, I could also add the code from PR129768 to shorten the BdbQuit traceback by one frame.