Implement executeCommand for rstudioapi as OpenRPC contract
#2356
Conversation
| @@ -144,6 +153,11 @@ export class ExtHostMethods implements extHostProtocol.ExtHostMethodsShape { | |||
| }; | |||
| } | |||
|
|
|||
| async executeCommand(commandId: string): Promise<null> { | |||
There was a problem hiding this comment.
I guess we'll quickly run into the need of passing arguments? These could be implemented as a Array<any> in the OpenRPC contract (IIRC we have support for this, or at least partial support) and then applied with the spread syntax.
This means a downside of executeCommand compared to a normal request is decreased type-checking of arguments.
There was a problem hiding this comment.
None of the commands I am using so far have any arguments, so I would prefer to leave this as is for now, until we need args here and can test it out better.
There was a problem hiding this comment.
In my view we should keep this executeCommand as the RStudio API emulator -- it accepts only RStudio command IDs and maps them to VS Code commands and/or emulates them in some other way.
If we want an API that accepts VS Code/Positron command IDs and semantics (can take args and return values), I think that'd be a wholly separate API.
There was a problem hiding this comment.
Ah OK, that is different from how this is implemented now; right now this accepts only VS Code commands. We had a discussion about that here in amalthea.
What do you think on this? Should we go with Davis' suggestion there and move the mapping from RStudio commands to VS Code commands over here in Positron?
There was a problem hiding this comment.
I also was thinking that the RStudio API emulation would remain fully contained in Ark. From that perspective, the fact that the emulation happens to use this new UI comm API for executing VS Code commands would be an implementation detail.
(In any case, regarding extending to supporting arguments I agree it's better left to another PR)
There was a problem hiding this comment.
Clearly reasonable people can disagree on this 😄 but I want to join Lionel in gently advocating for the current implementation, i.e. rstudioapi emulation belongs in Ark.
|
TODO:
|
|
@jmcphers I suspect I may now be encroaching on the runtime session work you are doing; is this OK to do now? If so, can you review when you get a chance? |
Intent
Addresses #1312 by adding a new UI frontend OpenRPC contract. The immediate motivation is being able to do
rstudioapi::executeCommand("activateConsole")which will correspond to Positron'sexecuteCommand("workbench.action.positronConsole.focusConsole"), but we'll be able to set up lots of commands this way (hopefully). Here's the list of what RStudio supports FWIW.Approach
This PR makes a new comm contract as outlined here and then hooks it up to the regular Positron command service
executeCommand().QA Notes
This will be accompanied by partner PRs to amalthea and the Python extension, still to come.