Skip to content
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

Support RStudio plugins #302

Closed
andycraig opened this issue May 2, 2020 · 21 comments
Closed

Support RStudio plugins #302

andycraig opened this issue May 2, 2020 · 21 comments

Comments

@andycraig
Copy link
Collaborator

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:

  • getActiveDocumentContext
  • insertText
  • setDocumentContext

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
Copy link
Member

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
Copy link
Collaborator Author

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
Copy link
Collaborator

MilesMcBain commented Sep 2, 2020

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link
Collaborator Author

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
Copy link
Collaborator

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
Copy link
Collaborator

MilesMcBain commented Sep 8, 2020

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
Copy link
Member

renkun-ken commented Sep 8, 2020

@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
Copy link
Collaborator

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

@MilesMcBain
Copy link
Collaborator

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.

  • I am deferring most of the data wrangling associated with getting things into the rstudioapi format to R. The VSCode side of things is just getting the pieces together. This makes sense to me as there are R classes that need to be added at various levels of the data structure.
  • There will be quite a lot of new R code so I decided against having it in init.R. I have added 2 new R files, which are also deployed to the ~/.vscode-r with the other session watcher stuff by the deploySessionWatcher function:
    • vsc_r_api.R
    • vsc_r_api_util.R
  • There will be quite a lot of new request types that need to be dispatched by the updateRequest function in the case statement.
    • 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.

@MilesMcBain
Copy link
Collaborator

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

@renkun-ken
Copy link
Member

@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
Copy link
Collaborator

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: #302

Kind Regards,
Miles

@MilesMcBain
Copy link
Collaborator

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.

😁

@kevinushey
Copy link
Contributor

kevinushey commented Sep 9, 2020

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:

rstudio/rstudio#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
Copy link
Collaborator

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
Copy link
Contributor

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

@MilesMcBain
Copy link
Collaborator

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
Copy link
Collaborator

Added in #408

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants