Hi,
FIRST: you really do a great job :1st_place_medal:
Do you plan to support LSP (https://github.com/Microsoft/language-server-protocol)
I think it would be great if the server could speak LSP :-)
Maybe it does already and I just didn't find it in code. This wouldn't be the first time ;-)
Thanks!
I've looked into the possibility of LSP implementation, and it's a definite candidate for a future version. One consideration: LSP supports a lot of features that the Solargraph gem historically leaves to other tools, such as validation and document formatting. We need to determine which features to integrate and how to handle it.
There is another (actively developed) language server project but it seems lacking type inference.
https://github.com/mtsmfm/language_server-ruby
Perhaps this library could be integrated to that server.
(NOTE: Owner of that lib also published LSP server core.)
https://github.com/mtsmfm/language_server-protocol-ruby
I've started work on LSP support. The first version will use stdio for client connections.
I'm pivoting to socket connections for the first version of LSP. The performance on stdio dropped with the addition of linting because it depends on a spawned rubocop process, and it would hang while the parent was reading from stdin. The socket server is able to run rubocop in the background.
Work in progress is on the language-server branch. I expect to add helpers to solargraph-utils to make it easy to use with Node.js in general and vscode-languageclient in particular.
To run the socket server, use solargraph socket (default port is 7658).
The language-server branch is merged into master. Version 0.18.0 of the gem and version 0.14.0 of the VSCode extension will use LSP.
Documentation is in progress at https://github.com/castwide/solargraph/blob/master/LANGUAGE_SERVER.md.
Support for LSP is published in gem version 0.18.0.
Version 0.19.0 is published. The VSCode extension uses Microsoft's language client for integration, and at least one person has gotten it to work with Sublime.
Hello @castwide awesome work. We took a look at experimentally integrating solargraph into https://sourcegraph.com/ via your new LSP support: https://github.com/sourcegraph/lsp-adapter/pull/19
I came across two issues, I'll just mention them here but can file individual issues:
didOpen notification before any operations on a file. LSP does not require this https://github.com/Microsoft/language-server-protocol/issues/177I see above you mention requiring TCP. I'm interested if solargraph would work with multiple workspaces (a workspace per TCP connection) once the CWD issue is solved? Also interested in why server mode fixes rubocop?
I'm not a ruby programmer, but anything we (Sourcegraph) can do to help solve that? This seems like the most promising ruby LS we have tried out.
Thanks, @keegancsmith!
didOpen notification. That might resolve a recent issue with the VSCode extension as well.A workspace per TCP connection should be feasible.
When I experimented with a stdio transport, child RuboCop processes would block I/O while they were running, so any other requests and notifications would be frozen until RuboCop exited. I'm open to adding a stdio transport, but that issue would need to be resolved.
I appreciate any help you'd like to provide. Please feel free to open new issues or PRs.
The master branch includes a Library update that allows mapping of files without a didOpen notification. It'll be published in version 0.21.0.
The master branch includes a Library update that allows mapping of files without a didOpen notification. It'll be published in version 0.21.0.
great!
Solargraph sets the workspace root as the CWD so the process can use whatever Ruby environment is configured for the project using .ruby-version, gemsets, bundler, etc. This is especially necessary for environments using Ruby installation managers like rvm and rbenv. There's a related discussion at castwide/vscode-solargraph#12. It should be possible to remove that assumption from the server, however. It's mostly a function of the client that starts the process.
Yeah, I can image a very simple fix just being Dir.chdir on the workspace root.
A workspace per TCP connection should be feasible.
It's not really that important TBH. It's useful if there are resources that can be shared between workspaces, but otherwise a subprocess per workspace is quite a useful thing to do.
child RuboCop processes would block I/O while they were running, so any other requests and notifications would be frozen until RuboCop exited.
Could that be an issue with however you do subprocesses? Maybe you are sending your stdin to the subprocess? I find it interesting that somehow using server fixed it, but yeah I am likely missing something due to lack of ruby experience.
I appreciate any help you'd like to provide. Please feel free to open new issues or PRs.
Thanks for the feedback!
Yeah, I can image a very simple fix just being Dir.chdir on the workspace root.
Something like that might work for most cases, but rvm/rbenv, gemsets, bundles, etc. add a lot of moving parts that require the process to modify the environment at startup. Using the workspace as the CWD was the easiest way to resolve the most common issues. There might be other solutions, but it's been an ongoing concern.
Could that be an issue with however you do subprocesses? Maybe you are sending your stdin to the subprocess? I find it interesting that somehow using server fixed it, but yeah I am likely missing something due to lack of ruby experience.
I tried a few different methods, but the subprocess always blocked stdio, even when I started it in a separate thread and used Open3 to pipe the streams. Using a TCP socket for I/O bypassed the problem, so I went with that for the first transport implementation. There could very well be some other solution I missed, though.
Gem version 0.21.0 is published with correct handling of files without the didOpen notification.
LSP is now the standard integration method.
Most helpful comment
Thanks!
I've looked into the possibility of LSP implementation, and it's a definite candidate for a future version. One consideration: LSP supports a lot of features that the Solargraph gem historically leaves to other tools, such as validation and document formatting. We need to determine which features to integrate and how to handle it.