Skip to content
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

Add Debug line functionality to Debug menu and toolbar #17888

Closed
mrclary opened this issue May 15, 2022 · 15 comments · Fixed by #19306
Closed

Add Debug line functionality to Debug menu and toolbar #17888

mrclary opened this issue May 15, 2022 · 15 comments · Fixed by #19306

Comments

@mrclary
Copy link
Contributor

mrclary commented May 15, 2022

So Spyder's menu bar has Run and Debug menus which have items for running/debugging a file and running/debugging a cell, and running the current line, but not debugging the current line. I propose adding this menu item to the Debug menu bar. Similar to running a single line, which pastes the current line into the IPython console and executes, debugging a single line can paste the line into the IPython console prepended with "debug" and thus invoke the debugger for executing the single line of code.

@ccordoba12
Copy link
Member

That's a good idea to be consistent and something we could easily do with the %debug magic. The only problem I see with this is making sure the current line a valid Python statement, otherwise I think we'll fail to enter the debugger.

@ccordoba12
Copy link
Member

@impact27, do you have other ideas about this?

@mrclary
Copy link
Contributor Author

mrclary commented May 16, 2022

That's a good idea to be consistent and something we could easily do with the %debug magic. The only problem I see with this is making sure the current line a valid Python statement, otherwise I think we'll fail to enter the debugger.

Currently, when the code following a %debug command is not valid, it appears to enter the debugger, raise an exception, and exit the debugger, e.g.

In [1]: debug print('hello'
NOTE: Enter 'c' at the ipdb>  prompt to continue execution.
Traceback (most recent call last):
  File "/Users/rclary/opt/micromamba/envs/c2w_37/lib/python3.7/site-packages/IPython/core/magics/execution.py", line 960, in _run_with_debugger
    deb.run(code, code_ns)
  File "/Users/rclary/opt/micromamba/envs/c2w_37/lib/python3.7/site-packages/spyder_kernels/customize/spyderpdb.py", line 776, in run
    super(SpyderPdb, self).run(cmd, globals, locals)
  File "/Users/rclary/opt/micromamba/envs/c2w_37/lib/python3.7/bdb.py", line 575, in run
    cmd = compile(cmd, "<string>", "exec")
  File "<string>", line 1
    print('hello'
                 ^
SyntaxError: unexpected EOF while parsing

This seems to be appropriate behavior to me; was there something else that you were thinking about or did I misunderstand your concern?

@ccordoba12
Copy link
Member

I'd just like to avoid that error and simply forbid debugging a line if it's not valid. IPython has a way to check for complete statements with this method called check_complete (I think):

https://github.com/ipython/ipython/blob/32c393b6babeb82dab97aa4f2ac73edc358b4ebf/IPython/core/inputtransformer2.py#L633

@mrclary
Copy link
Contributor Author

mrclary commented May 16, 2022

What about ast.parse?

@mrclary
Copy link
Contributor Author

mrclary commented May 16, 2022

check_complete will only return 'complete', 'incomplete', or 'invalid'. Would the traceback returned by ast.parse be more useful to the user to explain why the code cannot be run?

@ccordoba12
Copy link
Member

I disagree with showing a traceback because users would think it's an error in Spyder. I mean, I doubt people would pay attention to the traceback. They'd probably just read the word error at the end and associate it intuitively to a Spyder error.

What I propose is that before trying to run %debug <line> in the console, we check if the line is complete. If it's not, then we could either:

  • Show a QMessageBox saying that the line can't be debugged because it's not a complete Python statement.
  • Do nothing. That could sound a bit too much, but I guess people would grasp what kind of lines can and can't be debugged.

@mrclary
Copy link
Contributor Author

mrclary commented May 16, 2022

I disagree with showing a traceback because users would think it's an error in Spyder. I mean, I doubt people would pay attention to the traceback. They'd probably just read the word error at the end and associate it intuitively to a Spyder error.

I completely agree. I think we should "groom" the message. My point was only the tool check_complete provides so little information.

What I propose is that before trying to run %debug <line> in the console, we check if the line is complete. If it's not, then we could either:

  • Show a QMessageBox saying that the line can't be debugged because it's not a complete Python statement.
  • Do nothing. That could sound a bit too much, but I guess people would grasp what kind of lines can and can't be debugged.

Yep. I think that's right.

@mrclary
Copy link
Contributor Author

mrclary commented May 16, 2022

However, to reconsider my previous agreement, the result of "Run selection or current line" for an invalid code line is:

In [1]: print('hello'
  File "/var/folders/5v/28jqvwxs2cd5fj93gvwnykdrqc926z/T/ipykernel_70626/3871997710.py", line 1
    print('hello'
                 ^
SyntaxError: unexpected EOF while parsing

Perhaps we want the debug counterpart to behave the same...

@impact27
Copy link
Contributor

I agree that "run line" raises a SyntaxError so it would make sense that debug line raises one too.

@ccordoba12
Copy link
Member

I think that's a bug that we should fix. It was reported a long time ago (see issue #4431), but we haven't had time to address it.

@mrclary, if could fix it as part of implementing this feature, it'd be great. If not, I'll try to do it in a future release.

@mrclary
Copy link
Contributor Author

mrclary commented May 21, 2022

@ccordoba12, after reviewing #4431, I think I understand better what you mean.

  • Yes, I think that @CAM-Gerlach's comment could be incorporated into the PR for this issue.
  • However, for both the Run line and Debug line, after considering the smallest appropriate code block, if there is a legitimate syntax error, I think that an exception should be raised.

@ccordoba12
Copy link
Member

Yes, I think that @CAM-Gerlach's #4431 (comment) could be incorporated into the PR for this issue.

Instead of trying to expand the current selection to reach a valid multi-line statement, I think we should simply check the current line, paste it in the console if valid but incomplete, and generate a new line expecting that it'll be completed eventually.

However, for both the Run line and Debug line, after considering the smallest appropriate code block, if there is a legitimate syntax error, I think that an exception should be raised.

Agreed.

@mrclary mrclary self-assigned this May 21, 2022
@CAM-Gerlach
Copy link
Member

Instead of trying to expand the current selection to reach a valid multi-line statement, I think we should simply check the current line, paste it in the console if valid but incomplete, and generate a new line expecting that it'll be completed eventually.

This isn't really useful, at least if I understand you correctly. More often than not on such multi-line statements, my cursor is at the end of it (or at least not on the first line), so I'd have to navigate all the way up to the first line and then press the run line shortcut on each line until it gets to the bottom, which is around the same if not more work than just selecting the text (with the KB or mouse) and then pressing the run line shortcut once.

If we're already parsing the AST, it should be fairly straightforward to extract the smallest complete statement from that, no? And we could just issue an error or just run the current line on edge cases, like we already do, for the sake of handling the vast majority of useful ones.

@ccordoba12
Copy link
Member

ccordoba12 commented Jun 28, 2022

@CAM-Gerlach, you already argued that point at length on issue #4431, which was the main reason it was not implemented at the end (and five years have passed since it was opened).

If you need multi-line evaluation, you can easily create cells. Otherwise, I think what you propose is bad UX (and something no one else has requested): if users want to evaluate a line, then we should simply do that.

@ccordoba12 ccordoba12 added this to the v6.0alpha1 milestone Sep 2, 2022
@ccordoba12 ccordoba12 changed the title Add Debug line functionality to Debug menubar Add Debug line functionality to Debug menu and toolbar Sep 4, 2022
@ccordoba12 ccordoba12 assigned impact27 and unassigned mrclary Sep 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants