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

PR: Add functionality run to and from current line to editor #16509

Merged
merged 8 commits into from
Mar 17, 2022
Merged

PR: Add functionality run to and from current line to editor #16509

merged 8 commits into from
Mar 17, 2022

Conversation

rhkarls
Copy link
Contributor

@rhkarls rhkarls commented Sep 29, 2021

Description of Changes

This PR adds functionality to run all lines either above or below the current cursor position.
If there is interest from the Spyder team to have this functionality I'd be happy to continue to work on this PR (see suggested todo list below).

This feature is available in other IDEs, for example VS Code with interactive python and R-Studio.
Currently the implementation in this WIP PR is identical to VS Code and R-Studio as it sends all the lines above or below the cursor verbose to the editor. An alternative is also to silently execute the lines not to fill the console.

Todo list

  • Wrote at least one-line docstrings (for any new functions)
  • Added unit test(s) covering the changes
  • Included a screenshot or animation
  • Icons for the menu items (perhaps something along the lines of mdi.ArrowCollapseDown with a play icons)
  • Keyboard short cuts for the menu items?
    • Currently Shift+F9 for "run to line" and CTRL+F9 for "run from line". Set for testing purposes
  • Naming of functionality in UI (e.g. "Run to line/Run from beginning to line/Run lines above?)

spyder_run_lines_from_cursor

Issue(s) Resolved

Fixes #16431

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: @rhkarls

@pep8speaks
Copy link

pep8speaks commented Sep 29, 2021

Hello @rhkarls! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-10-23 11:42:50 UTC

@rhkarls rhkarls changed the title WIP: Add run to or from current line to editor [WIP] PR: Add functionality run to and from current line to editor Sep 29, 2021
@dalthviz
Copy link
Member

Hi @rhkarls thanks for your interest in adding new functionalities to Spyder! From my side seems like a good idea 👍
however what do you think @spyder-ide/core-developers ?

@mrclary
Copy link
Contributor

mrclary commented Sep 29, 2021

I think it's a good idea.

@rhkarls
Copy link
Contributor Author

rhkarls commented Oct 5, 2021

Thanks a lot - I will work on getting this ready when I have time in between traveling. Would be great to get some feedback on keyboard shortcuts and naming in the UI at a later stage

@rhkarls rhkarls marked this pull request as ready for review October 5, 2021 17:10
@rhkarls
Copy link
Contributor Author

rhkarls commented Oct 5, 2021

The mac tests seem to fail due to some conda env issues and using python 2.7 (?). Otherwise this is ready for the most part I think - see todo list on top, and would be nice with a review.

@mrclary
Copy link
Contributor

mrclary commented Oct 5, 2021

@ccordoba12, could the failing mac tests be due to some changes we made regarding mamba and conda-forge?

@dalthviz
Copy link
Member

dalthviz commented Oct 6, 2021

@rhkarls could you do a rebase on top of the latest 5.x branch to see if that helps with the failing tests ?

@rhkarls
Copy link
Contributor Author

rhkarls commented Oct 7, 2021

Cheers @dalthviz ! Seems that the fast mac test froze though. Due to my mediocre git skills I managed to do a merge, but looks clean. To do a proper rebase as you suggested, what git commands should be issued? What I did now was

git checkout add-feature-run_to_from_line
git pull upstream 5.x
git push origin add-feature-run_to_from_line

@mrclary
Copy link
Contributor

mrclary commented Oct 7, 2021

@rhkarls

$ git rebase 5.x

@dalthviz
Copy link
Member

dalthviz commented Oct 7, 2021

@rhkarls I think a merge is ok too so the rebase is not needed anymore. I will retrigger the action that was cancelled to check if that helps 👍

@mrclary
Copy link
Contributor

mrclary commented Oct 18, 2021

I recommend "Run to current line" and "Run from current line". And to be clear, the lines run should be [beginning, current line), and [current line, end], respectively.

@rhkarls
Copy link
Contributor Author

rhkarls commented Oct 20, 2021

Thanks @mrclary . I agree with your recommendation, both the naming and what lines are executed. I can add that [beginning, current line) and [current line, end] is the same as VS code as well. @dalthviz how should we proceed from here?

@dalthviz
Copy link
Member

Thanks @mrclary for the feedback and @rhkarls for the work done here! Now we will review this work and if there are any changes to make we will let you know (otherwise I think we will merge this 👍 )

@dalthviz dalthviz changed the title [WIP] PR: Add functionality run to and from current line to editor PR: Add functionality run to and from current line to editor Oct 20, 2021
Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thank you @rhkarls ! I left a suggestion regarding the comment in the test for the functionality and also I would suggest to change the icon used or not to use icons at all for this actions at least for now. I think that re-using the 'run_selection' icon could be a little bit confusing, but I'm not sure what other icon is suitable to represent this.

Also, just in case regarding the icons, what do you think @ccordoba12 ?

spyder/plugins/editor/widgets/codeeditor.py Outdated Show resolved Hide resolved
spyder/plugins/editor/widgets/codeeditor.py Outdated Show resolved Hide resolved
spyder/app/tests/test_mainwindow.py Outdated Show resolved Hide resolved
spyder/plugins/editor/plugin.py Outdated Show resolved Hide resolved
spyder/plugins/editor/plugin.py Outdated Show resolved Hide resolved
@dalthviz dalthviz added this to the v5.2.0 milestone Oct 25, 2021
Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thank you @rhkarls for your work here! This LGTM 👍🏼 but just in case what do you think @ccordoba12 ?

@ccordoba12
Copy link
Member

Thanks a lot for your contribution @rhkarls! I'll try to take a look at it soon.

@ccordoba12 ccordoba12 modified the milestones: v5.2.0, v5.2.1 Nov 1, 2021
@dalthviz dalthviz linked an issue Nov 12, 2021 that may be closed by this pull request
@ccordoba12 ccordoba12 modified the milestones: v5.2.1, v5.2.2 Nov 26, 2021
@ccordoba12 ccordoba12 modified the milestones: v5.2.2, v5.3.0 Dec 19, 2021
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

This is a great addition! Thanks @rhkarls for your help to implement this and your patience to get it merged.

@ccordoba12 ccordoba12 merged commit 232cf14 into spyder-ide:5.x Mar 17, 2022
ccordoba12 added a commit that referenced this pull request Mar 17, 2022
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.

Feature request: "Run to line" and "Run from line"
5 participants