-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
bpo-35931: Gracefully handle SyntaxError in pdb debug command #11782
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-35931: Gracefully handle SyntaxError in pdb debug command #11782
Conversation
On the pdb prompt `debug print(` currently crashes, but `print(` displays that there's a SyntaxError. This patch fixes this by pre-compiling the code for `Pdb.run`.
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 you add a test for this? It will also need a NEWS entry (you can use https://blurb-it.herokuapp.com/ for that purpose).
Done. |
Lib/test/test_pdb.py
Outdated
|
||
def test_syntaxerror_debug(self): | ||
stdout, _ = self.run_pdb_module("", "debug print(") | ||
self.assertIn('(Pdb) *** SyntaxError: unexpected EOF while parsing\n(Pdb) ', stdout) |
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.
Would appreciate feedback on this.. should it be put into a single test? (mainly wanting this to be fast)
As for this change only test_syntaxerror_debug
could be added also.
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.
Also needs to be fixed for Windows to include \r
/ use splitlines
.
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.
Hi, yes IMO would be better add whole in a single test, because you are testing the same things.
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.
Also needs to be fixed for Windows to include
\r
/ usesplitlines
.
Currently there are problems with Azure test
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.
because you are testing the same things.
Not really.. the test for just print(
was working before this patch already (and uses another code path).
Lib/test/test_pdb.py
Outdated
|
||
def test_syntaxerror_debug(self): | ||
stdout, _ = self.run_pdb_module("", "debug print(") | ||
self.assertIn('(Pdb) *** SyntaxError: unexpected EOF while parsing\n(Pdb) ', stdout) |
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.
Hi, yes IMO would be better add whole in a single test, because you are testing the same things.
@@ -0,0 +1 @@ | |||
pdb: handle SyntaxError with ``debug``. |
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.
I will change for: "Add handle SyntaxError with debug
on pdb" (just an opinon)
Lib/test/test_pdb.py
Outdated
|
||
def test_syntaxerror_debug(self): | ||
stdout, _ = self.run_pdb_module("", "debug print(") | ||
self.assertIn('(Pdb) *** SyntaxError: unexpected EOF while parsing\n(Pdb) ', stdout) |
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.
Also needs to be fixed for Windows to include
\r
/ usesplitlines
.
Currently there are problems with Azure test
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.
LGMT. I test it. And work fine on Ubuntu 18
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.
LGTM.
@blueyed: Status check is done, and it's a success ✅ . |
Thanks @blueyed for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7. |
…GH-11782) Previously, `debug print(` would cause the interpreter to exit on a SyntaxError whereas `print(` would properly display the error and return to the pdb prompt. This patch fixes this by pre-compiling the code before passing it to `Pdb.run`. https://bugs.python.org/issue35931 (cherry picked from commit 4327705) Co-authored-by: Daniel Hahler <github@thequod.de>
GH-11886 is a backport of this pull request to the 3.7 branch. |
Previously, `debug print(` would cause the interpreter to exit on a SyntaxError whereas `print(` would properly display the error and return to the pdb prompt. This patch fixes this by pre-compiling the code before passing it to `Pdb.run`. https://bugs.python.org/issue35931 (cherry picked from commit 4327705) Co-authored-by: Daniel Hahler <github@thequod.de>
NOTE: will be fixed in Python 3.7.3 itself (python/cpython#11782).
NOTE: will be fixed in Python 3.7.3 itself (python/cpython#11782).
Followup in #12103. |
NOTE: will be fixed in Python 3.7.3 itself (python/cpython#11782).
Previously,
debug print(
would cause the interpreter to exit on a SyntaxError whereasprint(
would properly display the error and return to the pdb prompt.This patch fixes this by pre-compiling the code before passing it to
Pdb.run
.https://bugs.python.org/issue35931