-
Notifications
You must be signed in to change notification settings - Fork 45
Code mockup for proposal to change workbench.action.positronConsole.executeCode in Positron
#867
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
base: fix/run-statement-at-cursor-positron
Are you sure you want to change the base?
Code mockup for proposal to change workbench.action.positronConsole.executeCode in Positron
#867
Conversation
…turning position
| }, | ||
| executeSelectionAtPosition: async (uri: Uri, position: Position): Promise<Position> => { | ||
| return await vscode.commands.executeCommand( | ||
| 'workbench.action.positronConsole.executeCode', | ||
| { languageId: language, uri, position } | ||
| ); |
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 is the core change of this PR.
I propose that the 'workbench.action.positronConsole.executeCode' command takes two new properties in its argument object alongside languageId: uri and position. This would make the command suitable for calling in a vdoc.
I also propose that this command returns the position of the next statement after the one being executed (perhaps it may have to return undefined or the same position that was passed in if there is no next statement). This would make it so we do not have to re-implement Positron internal logic for finding the next statement like we were in #825.
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.
OK, I've got a proposal up in posit-dev/positron#10580, if you want to take a look and see what you think!
juliasilge
left a comment
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.
Things are working quite well, with my changes in posit-dev/positron#10580! 🎉
- The name of the command changed one more time, to
positron.executeCodeFromPosition - You might need to get your hands on the new daily build of Positron to really figure this out, but can you re-examine the behavior of where the cursor goes after it executes the last statement in a cell with multiple, multi-line statements in it?
- We should bump the version of Positron required when we add the use of
positron.executeCodeFromPosition, like I did in #736. Probably what makes the most sense is to do 2025.12.0, although that would mean we should NOT do a Quarto extension release until after Positron's 2025.12.0 release (scheduled for around Dec 2).
…ensions that handles uri, position, next position (#10580) - Addresses #7709 - Related to quarto-dev/quarto#867 I've been collaborating with @vezwork on how to get more language features hooked up in Quarto's visual editor, and we think this is what we need for the Quarto custom editor interface to be able to send code statement-by-statement to the console. Over in the Quarto extension, we can now do something like: ```typescript const nextPosition = await vscode.commands.executeCommand( 'positron.executeCodeInConsole', 'python', // language ID vscode.Uri.file('/path/to/script.py'), // document URI new vscode.Position(5, 0) // position ) as vscode.Position | undefined; ``` What we get back is the next position to move the cursor to, in Quarto's case for the virtual doc underlying the visual editor. It's a little weird to now have two commands, both the old `workbench.action.positronConsole.executeCode` and the new `positron.executeCodeInConsole`, but now that we're dealing with positions (and less so, URIs) we need to use the converters that we have access to in `src/vs/workbench/api/common/extHostApiCommands.ts`. I think I'd argue they do different enough things that this is OK? Although you can now technically call `workbench.action.positronConsole.executeCode` with a uri and position, but they would have to be the _internal_ main process versions of those; this feels somewhat weird. Is it worth doing more refactoring here to make this nicer? @:sessions @:console ### Release Notes #### New Features - Added new command `positron.executeCodeInConsole` for extensions to execute code for a given language, URI, and position (such as for the Quarto visual editor) #### Bug Fixes - N/A ### QA Notes Once we get changes merged on both the Positron and Quarto sides, you should be able to execute multi-line statements in the console from the visual editor. Once we get further along, I can share more detailed examples
|
I just merged in posit-dev/positron#10580, which means it will be in tomorrow's daily build. I think then you can more easily work on getting this over the line! 🙌 |
In #825 I ended up re-implementing Positron internal code (that is used in the source editor) for finding a statement range at the cursor, executing the code in that range, and then finding the next statement range. As @juliasilge brought up, this doesn't seem ideal. Ideally we could use Positron to do the work for us.
At the core, this PR modifies #825 to execute the
"workbench.action.positronConsole.executeCode"command with extrapositionanduriarguments, so that it is suitable for use in a vdoc. It also expects this command to return the position of the next statement range. The result of the code changes are to support the use of this functionality. This is not how this command actually works in Positron today, but let this PR serve as a proposal for how to modify this command to support this functionality (@juliasilge suggested putting up a PR with how I want to use the command so that she can do the work on the Positron side).With this modification, we are able to remove the code that re-implements Positron's internals.