Update 'workbench.action.positronConsole.executeCode' to be used in Quarto #3107
Conversation
There was a problem hiding this comment.
This PR is currently sort of half of a solution. The remaining challenge is that provideStatementRange() takes as an argument a model (because we implemented these as a LanguageFeatureRegistry). Our statement range provider doesn't know what to do with an ITextModel for a Quarto file; it fails in different ways for languageId = "r" as languageId = "quarto" but either way, it cannot currently identify the current statement.
I am wondering if we can do some parsing in ark to make a little fake document of the current chunk, much like we did parsing for roxygen comments. I believe it would be much easier to do it in ark than here in Positron or in the Quarto extension, because of how the VS Code types are being passed around.
| @@ -269,7 +269,7 @@ export function registerPositronConsoleActions() { | |||
| * Runs action. | |||
| * @param accessor The services accessor. | |||
| */ | |||
| async run(accessor: ServicesAccessor, opts: { allowIncomplete?: boolean } = {}) { | |||
| async run(accessor: ServicesAccessor, opts: { allowIncomplete?: boolean; languageId?: string } = {}) { | |||
There was a problem hiding this comment.
The Quarto extension knows what language we are using and can send it over in opts.
src/vs/workbench/contrib/positronConsole/browser/positronConsoleActions.ts
Outdated
Show resolved
Hide resolved
| @@ -499,7 +510,7 @@ export function registerPositronConsoleActions() { | |||
| } | |||
|
|
|||
| // Now that we've gotten this far, ensure we have a target language. | |||
| const languageId = editorService.activeTextEditorLanguageId; | |||
| const languageId = opts.languageId ? opts.languageId : editorService.activeTextEditorLanguageId; | |||
There was a problem hiding this comment.
We need to update here to get the console to execute it in the correct runtime.
|
It would be great to get some feedback on this Monday when you get online @DavisVaughan. |
|
This is getting there, together with the changes in quarto-dev/quarto#436 so that Quarto can register a statement range provider, to then in turn call the correct statement range provider for the document's language via a command. |
src/vs/editor/contrib/positronStatementRange/browser/provideStatementRange.ts
Show resolved
Hide resolved
| if (!result) { | ||
| // there is no provider or it didn't return a statement range | ||
| return undefined; | ||
| } | ||
|
|
||
| return result; |
There was a problem hiding this comment.
IIUC correctly you can just return result unconditionally. It is either a real IStatementRange or undefined, both of which are the expected return value of this function
src/vs/workbench/contrib/positronConsole/browser/positronConsoleActions.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Davis Vaughan <[email protected]> Signed-off-by: Julia Silge <[email protected]>
Intent
Addresses #1518
Goes together with quarto-dev/quarto#436
Approach
I'm putting this up as draft for some discussion, and I'll add some notes to point out the current challenges.
QA Notes
When we QA this change, we'll want to use the current development version of the Quarto extension (download
.vsixfrom https://github.com/quarto-dev/quarto) and check out the statement-by-statement execution in Quarto chunks.