Skip to content

Python: Cmd+Enter does not send portion of multi-line statement above cursor to console #1260

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

Closed
jmcphers opened this issue Sep 6, 2023 · 5 comments
Assignees
Labels
bug Something isn't working lang: python
Milestone

Comments

@jmcphers
Copy link
Collaborator

jmcphers commented Sep 6, 2023

To reproduce, create a Python file with the following contents:

sixty = 20 + \
    20 + \
    20

Place the cursor on the middle line and press Cmd Enter.

Only the middle line is sent to the Console. However, because this is one statement spread over 3 lines, the whole statement should be sent to the Console, not just the current line.

Note

This happens because the Python statement range finder does not ever look above the cursor point (only beneath it). https://github.com/posit-dev/positron-python/blob/main/src/client/positron/statementRange.ts

@chendaniely
Copy link

chendaniely commented Sep 21, 2023

same behavior is reproduced if you use ( ) instead of \

sixty = (
    20
    + 20
    + 20
)

when you have the cursor on the first line, it stops sending the code up until the last closing parenthesis, but does not include it. so the user needs to run cmd+enter twice.

we use this kind of notation a lot in pandas for method chaining.

@seeM seeM added the bug Something isn't working label Oct 18, 2023
@jmcphers
Copy link
Collaborator Author

jmcphers commented Nov 1, 2023

@jthomasmock pointed out that there's a new experimental Python feature in 1.84, "Smart Send":

https://code.visualstudio.com/updates/v1_84#_python

With the new experimental Smart Send feature, the Python extension sends the smallest block of runnable code surrounding the cursor position to the REPL for execution. This ensures that only complete and executable sections of code are sent to the REPL. The cursor will also be automatically moved to the next executable line, to provide a smooth experience when executing multiple chunks iteratively.
To try it out, you can add the following User setting: "python.experiments.optInto: ["pythonREPLSmartSend"]. While this feature is currently behind an experiment, we expect it to be the default behavior in the future. If you have feedback or suggestions on how we can further improve this feature, please let us know!

We should see if we can hook this behavior up to Positron's Statement Range Provider so that we can take advantage of the Smart Send logic (presuming it's more sophisticated than what we have, which it probably is -- we just use indentation).

@seeM
Copy link
Contributor

seeM commented Nov 15, 2023

I did some digging into how the Smart Send feature works.

The workhorse is the traverse_files function in the normalizeSelection.py script (source), which uses the standard library ast module to parse the entire file, and return the code corresponding to the current top-level node as well as the line number of the next node.

Some thoughts:

  • In my attempts to break it with really long files containing weird and complex nested statements, it was still fast (on an M2 Air). The official docs do state that sufficiently long/complex strings can crash ast.parse "due to stack depth limitations in Python's AST compiler".
  • One very unfortunate downside, though, is that ast.parse only works for Python with valid syntax. Currently, Smart Send does nothing if your script has a syntax error. We could fall back to a less sophisticated heuristic in that case, but I'm worried that it may be confusing for users if the statement execution behavior depended on whether your code had a syntax error. On the other hand, the only way I can think of to have consistent behavior is to use an incremental parser like tree-sitter. I'm not sure how much work that would take, but probably quite a bit longer than wiring us up to the smart send functionality. What do folks think?

@jjallaire
Copy link
Contributor

I think falling back to a less sophisticated heuristic in the case of an ast parse failure is fine. This will work often, and critically the user won't be confused by inconsistent behavior (as we will always try to execute code). Also, diagnostics should be present that indicate that something is awry. I agree that tree-sitter could be a better solution but I also agree that this isn't worth the effort at this point.

@juliasilge
Copy link
Contributor

In build 1368 I can send a multi-line statement to the console using cmd + enter from any of the lines of the statement (here, with my cursor on any of lines 19, 20, 21):

multiline

@wesm wesm added lang: python bug Something isn't working and removed bug Something isn't working labels Feb 29, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working lang: python
Projects
None yet
Development

No branches or pull requests

6 participants