autozimu / LanguageClient-neovim

Language Server Protocol (LSP) support for vim and neovim.
MIT License
3.55k stars 273 forks source link

Support sorbet: URIs #1112

Closed jez closed 3 years ago

jez commented 3 years ago

Mostly trying to guage how much interest there is in this feature. I see something similar already exists for jdt:// URIs, and I used that to model this implementation.

Sorbet, a type checker for Ruby, returns sorbet: URIs when it knows about a file but that file does not exist on the client's disk. That happens in two situations:

In both cases, Sorbet supports a sorbet/readFile file operation that takes in a single sorbet:... URI and returns the text content of the file (among other things--it actually returns a TextDocumentItem from the LSP spec).

If Sorbet receieves this option in initializationOptions at startup:

{
  "supportsSorbetURIs": true
}

It will send sorbet: URIs when appropriate. The expectation is that the client will use those URIs to send sorbet/readFile requests and handle them appropriately.

I'd love your thoughts on whether you think this is the sort of change that would be worth committing to LanguageClient-neovim. I'm happy to write tests for this change, but I didn't want to spend that effort until I got feedback on the idea and the implementation.

I have tested this on my laptop for a few days while using Sorbet and it has not had any problems.

Implementation of sorbet/readFile in Sorbet (~10 lines long): https://github.com/sorbet/sorbet/blob/master/main/lsp/requests/sorbet_read_file.cc

martskins commented 3 years ago

You seem to have based your PR on the branch next, which is the release branch, please target your PR at dev (you'll also see the latest developments there).

As for the feature, I think it would be welcome! We already have some implementations of server specific functionality, such as textDocument/switchSourceHeader for clangd. So my first suggestion would be to move this to a file under src/extensions/sorbet.rs. Other than that I would model request the same way we did for switchSourceHeader, that is: create a struct SorbetReadFile {} and implement lsp_types::request::Request on it. It doesn't really change much but it makes the codebase a little more homogeneous and it makes sense to do that, since it's really an LSP request.

Here's a link to the example I'm talking about: https://github.com/autozimu/LanguageClient-neovim/blob/0e0f99d291453c42a4e6bede636a9b703eee6081/src/extensions/clangd.rs#L11

Also, thank you for this and your recent issue reports, much appreciated!

jez commented 3 years ago

Going to close this since it's stale. I'll pick it up again when I have time.

Also, thank you for this and your recent issue reports, much appreciated!

No problem, thank you!