castwide.solargraph (solargraph) is an extension dependency

[wingrunr21]

This extension directly depends on the castwide.solargraph extension despite the extension itself being optional (see here).

I'm of the opinion we should not be directly depending on this extension (as not all users are going to be using it) but should instead provide README documentation regarding support for the extension via the ruby.codeCompletion and ruby.intellisense options.

I'm opening this issue for comment so we can talk through whether this is the right choice.

See #248 and #259 for more background

@rebornix @castwide

[rebornix]

I don't have a strong opinion on whether we should have direct dependence on castwide.solargraph. The main reason that I did so is I'm convinced that we need a good/alive language service and solargraph looks promising. But the side effect is obvious, users might get confused on this.

To mitigate it, one workaround can be

• Remove dependency
• When users set solargraph as their intellisense provider, and there is no castwide.solargraph, ask users gently to install that extension. It can be one click.

However, I'm not satisfied with current code structure. It's a combination of debugger, formatter, linter, completion provider but the boundaries are not clear. Ideally we can have four standalone components:

1. Basic language configurations: Syntax files, Snippets, Language configurations
2. Debugger (where this extension started from)
3. Language Service
   • Formatters
   • Linters
   • Completion providers
   • Go To Def, Find All References (Intellisense)
4. Task/Test runners, others

No.3 is interesting here. There is already one ruby language server out there but it's based on rcodetools. Right now we are replacing rcodetools and our inhouse intellisense code with castwide.solargraph but I wish other language feature providers like formatters, linters can sit next to solargraph. By that I mean I expect castwide.solargraph to be a language server and ship as a library.

BTW, feel free to join RubyIde Slack if the discussion goes long.

[castwide]

That workaround for solargraph dependency sounds reasonable to me. I considered proposing something similar.

Support for the language server protocol in the solargraph gem has been on the roadmap for a while (castwide/solargraph#8). One of my concerns was overlapping too much functionality with other tools, particularly vscode-ruby. If you want to transfer the formatting and linting responsibilities to solargraph, I can start moving in that direction.

LSP support is liable to have an impact on this issue. On the one hand, if the solargraph extension handles all four language service features, it makes more sense as a dependency; but on the other, if all the features are available through LSP, it might be easier to integrate the server directly into vscode-ruby.

[castwide]

I've started work on LSP support in the language-server branch at https://github.com/castwide/solargraph.

For rebornix.Ruby integration, I'm leaning toward combining the codeCompletion and intellisense options into a single languageServer option, and a prompt to install castwide.solargraph if required.

[wingrunr21]

@rebornix if we wish to delegate the linting, highlighting, etc to a language server, I think that's more of an argument not to depend on solargraph. We can pretty easily recommend a language server implementation (either via a popup or in the README), but if we don't have a direct dependency on which language server is used I don't see a reason to force it.

[gurgeous]

I agree that delegating to a language server is desireable, and would make it easy for vscode-ruby to pick a winner among competing LSP implementations.

On the other hand, getting vscode-ruby up and running is challenging. This is jarring compared to the excellent out of the box experience for eslint/prettier/etc. Unfortunately, several Ruby developers on my team immediately gave up on linting, formatting and go-to-definition. I tried to rescue them by showing off vscode-ruby demos, putting rubocop in the Gemfile, checking in workspace settings, etc. but I was never really able to get them on board.

Personally I think fixing this is the main priority. If it were up to me, I would jettison the myriad config options and just focus on rubocop and solargraph. Maybe rufo as well. If amazing Ruby LSP implementations appear, we can always switch to one later. In the meantime let's double down on our best-of-breed dependencies. Happy to help.

[escobera]

As pointed out in #281 atm is unclear how to disable the solargraph extension. I'm using ruby 2.1.5 and can't install solargraph, this way I cannot get rid of the error bessage about it.

[dmke]

To bring in another PoV against the solargraph integration (copied from #281):

I don't have a system-wide Ruby installed, all my Ruby projects are managed with rbenv. The project-specific Ruby is selected upon evaluating a .ruby-version in the project directory. Adding the solargraph gem to the Gemfile is also not a viable option, since I am the only developer in my team using VS Code.

Personally, I also don't need code completion for Ruby. It's a nice-to-have, but every attempt I've seen implementing this in a usable fashion (so far) has failed. I simply don't bother with editor code-completion. If I really need to inspect an unknown API, I'll just open a pry session :-)

[castwide]

It looks like we're moving away from castwide.solargraph as an extension dependency. If that's the case, problems with disabling it will eventually be resolved.

In the meantime, the next minor revision of castwide.solargraph will suppress the initial installation message, at least temporarily. If the solargraph gem is installed, the extension will still run a version check, but you can also disable that with the solargraph.checkGemVersion setting. There have been a couple other changes waiting for release, so I plan to publish an update tomorrow.

As for the future of integration, it might be best for rebornix.Ruby and castwide.solargraph to be orthogonal, but I'd still like to cooperate to make sure they play nice together. The only features I expect Solargraph to provide are related to language servers, i.e., completion, go to definition, etc. Features like debugging and task management are out of its scope.

[castwide]

@dmke Regarding your issue with rbenv, Solargraph tries to run in the context of your open workspace folder, so it should use your .ruby-version file if it exists. Issues with multiple Ruby installations are being tracked in castwide/vscode-solargraph#12. (Of course, the point is moot if you don't want to run Solargraph at all. I just wanted to throw that out there.)

[dmke]

@castwide: Good to know :-)

[castwide]

Version 0.13.0 of castwide.solargraph disables the installation message. If you don't have the gem installed, the extensions will proceed without trying to run the server.

[castwide]

I've opened related issue #285 to discuss changes that defer language server features to external providers.

[gurgeous]

Shall we continue the discussion?

I think as a practical matter we should remove the solargraph dependency. Many of the incoming issues are related to solargraph functionality, unfortunately. There's no need to tie the extensions together.

I want to be absolutely clear that I use solargraph personally and I respect all the hard work that @castwide is putting into the extension. Solargraph is great! I just think we should de-couple the extensions so we can focus on fixing up vscode-ruby. Installation is already somewhat challenging with rubocop/rbenv/rvm/chruby etc.

In our docs section we can point people toward solargraph and other popular Ruby extensions.

[castwide]

@gurgeous I'm on board with decoupling, as discussed in #285. I'll submit the PR for it shortly.

[gurgeous]

Great, thank you. I'd submit more PRs but I'm scared I already overwhelmed the maintainers :)

[castwide]

#309

[wingrunr21]

Resolved via #309