Skip to content

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

Merged
merged 5 commits into from
Feb 15, 2019

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Feb 7, 2019

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

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`.
Copy link
Member

@zware zware left a 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).

@blueyed
Copy link
Contributor Author

blueyed commented Feb 7, 2019

Can you add a test for this?

Done.


def test_syntaxerror_debug(self):
stdout, _ = self.run_pdb_module("", "debug print(")
self.assertIn('(Pdb) *** SyntaxError: unexpected EOF while parsing\n(Pdb) ', stdout)
Copy link
Contributor Author

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor

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.

Currently there are problems with Azure test

Copy link
Contributor Author

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).


def test_syntaxerror_debug(self):
stdout, _ = self.run_pdb_module("", "debug print(")
self.assertIn('(Pdb) *** SyntaxError: unexpected EOF while parsing\n(Pdb) ', stdout)
Copy link
Contributor

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``.
Copy link
Contributor

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)


def test_syntaxerror_debug(self):
stdout, _ = self.run_pdb_module("", "debug print(")
self.assertIn('(Pdb) *** SyntaxError: unexpected EOF while parsing\n(Pdb) ', stdout)
Copy link
Contributor

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.

Currently there are problems with Azure test

Copy link
Contributor

@eamanu eamanu left a 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

@zware zware changed the title bpo-35931: pdb: do_debug: handle SyntaxError bpo-35931: Gracefully handle SyntaxError in pdb debug command Feb 15, 2019
Copy link
Member

@zware zware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@miss-islington
Copy link
Contributor

@blueyed: Status check is done, and it's a success ✅ .

@miss-islington miss-islington merged commit 4327705 into python:master Feb 15, 2019
@miss-islington
Copy link
Contributor

Thanks @blueyed for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Feb 15, 2019
…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>
@bedevere-bot
Copy link

GH-11886 is a backport of this pull request to the 3.7 branch.

@blueyed blueyed deleted the do_debug-handle-SyntaxError branch February 15, 2019 21:07
miss-islington added a commit that referenced this pull request Feb 15, 2019
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>
blueyed added a commit to pdbpp/pdbpp that referenced this pull request Feb 15, 2019
NOTE: will be fixed in Python 3.7.3 itself
(python/cpython#11782).
blueyed added a commit to pdbpp/pdbpp that referenced this pull request Feb 16, 2019
NOTE: will be fixed in Python 3.7.3 itself
(python/cpython#11782).
@blueyed
Copy link
Contributor Author

blueyed commented Feb 28, 2019

Followup in #12103.

blueyed added a commit to blueyed/rebased-pdbpp that referenced this pull request May 25, 2019
NOTE: will be fixed in Python 3.7.3 itself
(python/cpython#11782).
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.

6 participants