search

REditorSupport / vscode-R

R Extension for Visual Studio Code
https://marketplace.visualstudio.com/items?itemName=REditorSupport.r
MIT License
1.07k stars 128 forks source link

Support RStudio plugins #302

Closed andycraig closed 3 years ago

andycraig commented 4 years ago

RStudio plugins communicate with RStudio via the rstudioapi package. It looks like this communicates with RStudio via a file-based communication protocol, similar to the session watcher. See callRemote in https://github.com/rstudio/rstudioapi/blob/ac27784fda4ed5a8f7e8d0c8159f841aac22010b/R/remote.R

It seems like we could theoretically support RStudio plugins by running them in a separate R session and supporting the file-based communication protocol. We would have to implement in TypeScript the rstudioapi functions like:

An obstacle is that rstudioapi does some checking of RStudio versions etc. so vscode-R might have to identify itself as RStudio at a couple of points. That seems weird and might have side effects. There could even be some copyright/trademark issue, in which case we would DEFINITELY not want to implement this.

Even if we cleared those obstacles, this would of course be a lot of work, and would place further demand on the system by adding another R session. So even if it's possible it's not necessary something we want.

renkun-ken commented 4 years ago

Thanks for raising this. I thought about this too but didn't have time to investigate the underlying protocol between RStudio and the plugins.

We may raise an issue at RStudio and hear from them about whether the protocol and rstudioapi could be used outside RStudio and let other editors leverage the same protocol?

andycraig commented 4 years ago

We may raise an issue at RStudio and hear from them about whether the protocol and rstudioapi could be used outside RStudio and let other editors leverage the same protocol?

This sounds like a good idea to me. If you would like to raise the issue, please do.

MilesMcBain commented 4 years ago

I had this idea myself today!

I think something analogous to the rstudioapi is highly desirable, elsewise the burden on the developer for developing things like datapasta and fnmate for VSCode is very high. I would have to create my own VSCode extensions or get them merged here to port them.

So I don't know if collaborating with RStudio is absolutely necessary if that is the blocker. VSCode could provide adapter functions that mock the rstudioapi by overwriting the functions in the rstudioapi namespace at run time. So we could literally reassign the likes of rstudioapi::isAvailable and rstudioapi::getActiveDocumentContext to functions that talk instead to VSCode via the session watcher protocol (need to build in a response protocol). For example using assignInNamespace().

It could be done with the other init.R stuff by setting an onLoad hook for rstudioapi, e.g. 

setHook(packageEvent("rstudioapi", "onLoad"),
        function(...) ..replace functions in rstudioapi namespace...)

or another option could be to create some kind of adapter users have to opt into by using it. e.g.

.vsc.with_rstudioapi_adapter(
  datapasta::tribble_paste
)

which could either run the function or return a modified version of it that can be assigned somewhere and referred to in keybindings etc.

I think this kind of thing is fairly low risk for the types of functions @andycraig has highlighted. RStudio can't change them without affecting a great many addins in the wild that use them.

MilesMcBain commented 4 years ago

Follow up on licensing issues: We would not be using the backend protocol that talks to RStudio. Depending on interpretation maybe we're modifying the rstudioapi package and making a derivative work (at run time), but it is licensed MIT so I think that would be fine?

It might be courteous to let them know the plan, but I don't think it we'd need their permission under the design I proposed.

andycraig commented 4 years ago

Hi @MilesMcBain, glad to hear you're interested in this too! I like your idea of reassigning rstudioapi::isAvailable etc. at runtime.

Thank you also for pointing out that the license is MIT. I'd only seen the license file on GitHub, but on the CRAN page it does indeed show as MIT.

I think you've resolved the aspects I was concerned about!

MilesMcBain commented 4 years ago

Excellent!

I am keen to see this happen, full datapasta in VSCode with no changes to the package itself would be so so sweet. Happy to contribute.

The direction I am thinking would be dependent on some kind of response mechanism from VSCode to requests from the R session. We could try mirroring the request setup with a response lockfile and response body file. The simplest thing would be to block the R session until a response is received or timeout. But I am wondering if this was already on the cards and if anyone has done any thinking about it?

andycraig commented 4 years ago

I haven't really done any more thinking about it beyond my post creating this issue. If you might be interested in thinking about the details and writing some code to make this happen, that would of course be fantastic! I can help out as much/as little as required.

There are a bunch of rstudioapi requests that VS Code will ideally be able to fulfil one day, but we could add these incrementally. I'd be very happy to treat 'support datapasta' as an initial goal.

We've been discussing having R sessions start a server that would enable background communication with them that might be able to avoid blocking, but this hasn't progressed very far to my knowledge. Blocking the R session until a response is received or timeout sounds fine by me.

There's code for the session watcher that could be used to watch for requests from the R session, in particular startRequestWatcher and updateRequest in https://github.com/Ikuyadeu/vscode-R/blob/master/src/session.ts I think this would be a good place to start.

MilesMcBain commented 4 years ago

Okay I have the response protocol wired up and can get some document context from VSCode back to R.

I'm attacking getActiveDocumentContext, first. There's more to do, but happily the formats for selections returned by the VSCode are already very similar to RStudio. I think it's just a renaming exercise. E.g. this is nearly it: https://github.com/MilesMcBain/vscode-R/blob/vsc-r-api/src/vsc_r_api.ts

MilesMcBain commented 4 years ago

Can I ask a question regarding the request protocol? In the case of multiple VSCode instances running how is the communication with VSCode scoped? It seems that currently all communication between R and VSCode runs through the same request file @ ~/.vscode-r/request.log? Is this somehow scoped such that all VSCode instances do not receive the requests?

Edit: Even though scoping does not seem to be happening, things seem to hold up under different tests, and the instance from which the request is made seems to be the one to get it, despite having verified all instances are writing requests to the same file. A bit spooky. Is this relying on processing preference to foregrounded instance?

renkun-ken commented 4 years ago

@MilesMcBain Currently, all requests are written to ~/.vscode-R/request.log with the session pid, working dir, etc. All VSCode instances with vscode-R activated will watch this file and be notified if the file is changed.

Currently, each vscode instance compares the vscode workspace folder with the working directory written in the request file and see if it should process the request. It does only if the working directory is the folder of subfolder of the vscode workspace root (see https://github.com/Ikuyadeu/vscode-R/blob/master/src/session.ts#L466). If vscode does not open a folder (thus has no workspace root), then it only process requests with working directory being home folder.

MilesMcBain commented 4 years ago

Excellent, thanks for the answer. The synchronous response thing I've rigged up in my WIP should work without issue then.

MilesMcBain commented 4 years ago

Good news! I have a proof of concept for rstudioapi::getActiveDocumentContext on my WIP branch. That is, if you call this function from within a VSCode-R session you get the expected output, as if you had called it from within RStudio.

Now might be a good time to check we're okay with the overall approach.

MilesMcBain commented 4 years ago

It might be easier to review what I've done if I make a PR?

renkun-ken commented 4 years ago

@MilesMcBain Nice work! Looks quite promising! The approach makes good sense to me.

There will be quite a lot of new R code so I decided against having it in init.R

Looks good to me. We could always improve the organization of code later.

I was wondering if it might be worth having a nested structure, so there's a top level request type of 'vsc_r_api` and another data field that determines the api call. This would allow the API requests to be dispatched in another api-specific handler function.

Feel free to change the existing session watcher code to fit the purpose. I'd suggest that we could start from a simple structure and then see if we have to make it more complex to meet the demand.

It might be easier to review what I've done if I make a PR?

Feel free to create a PR so that it is easier to know the progress and to discuss around the implementation.

MilesMcBain commented 4 years ago

Cool the PR is made. I thought since it's obvious that this is being actively worked on now, it would be a good time to let authors of the rstudioapi know what's happening.

I sent this email just now:

Hello JJ, Kevin, Hadley and Gary,

This is friendly heads up to authors of the {rstudioapi} that I am working on a PR for the VSCode R extension to support RStudio Addins in VSCode via an {rstudioapi} emulation layer. It has only just been proven as a concept, so release is still a while away.

It remains to be seen how far we will get, but I think we should be able to make a decent subset of RStudio addins viable in VSCode.

If you have any issues with this or want to discuss anything feel free to email me or to chime in on the issue thread at the VSCode repo: https://github.com/Ikuyadeu/vscode-R/issues/302

Kind Regards, Miles

MilesMcBain commented 4 years ago

Here is the response:

Hi Miles,

That's great! Let us know if you have questions or problems along the way. Also keep an eye on the rstudioapi repo as we have some new selection oriented APIs coming soon.

Best,

J.J.

:grin:

kevinushey commented 4 years ago

This looks awesome!

We're also adding a somewhat simpler selection-based API to the next version of RStudio. We don't have any official documentation yet, but the pull request is here:

https://github.com/rstudio/rstudio/pull/7738

The ultimate idea is that rather than providing the user with the "richer" document context object (with document contents, selection coordinates, etc.) we'll just supply them the text contents of the current selection (assuming only a single cursor, single selection), and then ask the user to provide some transformation of that text to be used when replacing the selection.

These APIs will be recommended in the future (where applicable) as they'll be compatible with RStudio's new visual mode editor (which is more like a cell-based WYSIWYG editor) as well as the regular source editor.

We're also considering what other APIs we could form that would be compatible both in visual + source modes for RStudio; e.g. we're thinking about whether a chunk-based API (for mutating the contents within the current chunk, or all code chunks in a document, or similar) would be feasible. We'd definitely appreciate your feedback as well, and we'd like to tool this in a way that is easy for VSCode (or other tools) to hook or shim as needed.

MilesMcBain commented 4 years ago

Thanks for your support @kevinushey!

I have one question if you don't mind?

Is hasFun a way of checking that the rstudioapi function in question can be called on the running RStudio instance? I had thought this was the case, but then looking at the source I became unsure.

kevinushey commented 4 years ago

Right, hasFun() is used to double-check that RStudio defines the API requested by the rstudioapi package.

MilesMcBain commented 4 years ago

Great! I am thinking it's best for VSCode to avoid claiming to be a specific RStudio version if possible. So hasFun and findFun will be the way for people to check deps for both.

MilesMcBain commented 3 years ago

Added in #408