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: Make running a single line work on partial statements (IPython console) #21557

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ccordoba12
Copy link
Member

Description of Changes

  • This was a very confusing and continuous source of frustration for newbies, so I decided to fix it for Spyder 6.

  • A couple of gifs to show how it works:

    run-partial-statements

    run-partial-statements-1

Issue(s) Resolved

Fixes #4431.

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:

@pep8speaks
Copy link

pep8speaks commented Nov 26, 2023

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

Line 519:9: E731 do not assign a lambda expression, use a def

Comment last updated at 2023-11-26 23:07:16 UTC

@jitseniesen
Copy link
Member

Nice work! In the second example, I would have expected Spyder to execute lines 2+3; why is it waiting for line 4?

@ccordoba12
Copy link
Member Author

Nice work!

Thanks @jitseniesen!

In the second example, I would have expected Spyder to execute lines 2+3; why is it waiting for line 4?

Because the print in line 4 is at the same indentation level as the for loop in line 2, which means they are part of the same code block.

So, the way I implemented this feature is that we execute code when we find an empty line or when a line is at a lesser indentation level than the initial one (which means we are at a different code block).

A couple of other gifs to clarify that:

run-partial-statements-with-blank

run-partial-statements-with-dedent

@jitseniesen
Copy link
Member

I understand it now. It's not what I expected. I'd have thought that if the user press F9 on a line that is not a complete statement, Spyder would execute the statement that contains the line. Instead, in this PR the user should keep pressing F9 until a DEDENT or an empty line is encountered. To be honest, I am afraid that this might be just as confusing as the existing behaviour, but I'm curious what others think.

@ccordoba12
Copy link
Member Author

ccordoba12 commented Nov 27, 2023

I'd have thought that if the user press F9 on a line that is not a complete statement, Spyder would execute the statement that contains the line.

That's what was suggested on issue #4431, but it's not easy to implement because it'd require to determine the scope of the line to do it. And what should we do in the case of nested for loops, as those shown above? Should we go up or down in the scope when trying to run an inner loop?

To be honest, I am afraid that this might be just as confusing as the existing behaviour, but I'm curious what others think.

What I noticed is that users are baffled when trying to run a single, incomplete but valid line and get a SyntaxError in return. This PR addresses that by not running those lines but instead leaving them as "pasted" in the console, so they can keep pressing F9.

@dalthviz
Copy link
Member

So gave a check to the original issue and seems like the idea is for the Run line functionality to run full statements, right?

Thinking about the approach here, seems like there are still cases where the Run line functionality could end up triggering a syntax error. Like, taking as an example a code snippet from the added tests, if we have something like:

for i in range(2):         # 1
    print('foo')           # 2
    for j in range(2):     # 3
        if j > 0:          # 4
            print(j)       # 5
        else:              # 6
            print('bar')   # 7
print('baz')               # 8

If you start to use the Run line functionality from line 6 (the else) you can end up pressing F9/triggering Run line three times to end up getting a syntax error. Maybe we should add somehow a way to detect that an initial line to be run will end up in such an error and then prevent running such lines and trying to go for the next line?

Overall seems like the approach here could be in a way an improvement to the run line functionality, but still has some behaviors which could be not so intuitive (like having to press multiples times) and seems to me that it does not completly cover the original issue (although I havent checked myself how RStudio handles this to be honest) 🤔

Maybe could be worthy to discuss this on a dev meeting?

@ccordoba12 ccordoba12 modified the milestones: v6.0alpha3, v6.0beta1 Dec 1, 2023
@ccordoba12 ccordoba12 modified the milestones: v6.0alpha4, v6.0beta1 Feb 6, 2024
@ccordoba12 ccordoba12 modified the milestones: v6.0alpha5, v6.0beta1 Mar 12, 2024
@ccordoba12 ccordoba12 modified the milestones: v6.0beta1, v6.1.0 May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make "Run line" to work on partial statements (e.g. code blocks, loops, etc.)
4 participants