-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
PR: Transition Debugger plugin actions to the new Run architecture and cleanup Editor plugin #20557
Conversation
@ccordoba12 This is ready for review |
e27f9a9
to
8590ecf
Compare
Add context modificator
8590ecf
to
a710788
Compare
Quick review:
I don't like this change because it makes the interface more complicated and crowded, and the toolbar of the Debugger plugin doesn't look good now: So, please revert it. Instead, I'd like to add the "Stop debugging" button (i.e. the blue square one) to the main toolbar, because I think it's missing to quickly stop debugging.
What happens now if the user wants to run/debug something and move the focus automatically to the IPython console? |
This feature doesn't work in the master now, and I am not sure why the user would want the focus to jump around. Wouldn't it make more sense to keep the focus where it is? What would be the usecase if that feature was fixed? |
@ccordoba12 I reverted it: |
Yeah, I forgot about it. Then it's fine to remove that option. |
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.
Thanks @impact27 for your work on this!
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
07f6ced
to
9db27eb
Compare
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
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.
This looks almost ready, thanks @impact27!
Besides my small suggestions, please also remove the QGroupBox
highlighted in the red square below
Since the tab on which it's placed has the same name, it's a bit redundant to have that QGroupBox
now.
One more thing: I noticed that you removed the |
f07db98
to
d9440eb
Compare
All the logic is now in run_script |
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.
Looks good to me now, thanks @impact27!
Description of Changes
run_cell_copy
work with the debugger by appending%%debug
to the cell copyfocus_to_editor
option that was now mostly unused ("Maintain focus in the editor" option has no effect when running cells #17028 and Editor loses cursor focus after running cell when IPython console is detached #3221 are still ok) (It was a strange option anyway becausefocus_to_editor
means "do nothing" andnot focus_to_editor
means focus to the console)run_cell
doesn't have to deal with run_cell_copy because the copy is run as a selectionfrom line
is not an extra action but a modifier)Issue(s) Resolved
Fixes #
Affirmation
By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.
I certify the above statement is true and correct: