Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Disabling LSP features (rcodetools, rubyLocate, formatting) #285

Closed
castwide opened this issue Mar 6, 2018 · 11 comments
Closed

Disabling LSP features (rcodetools, rubyLocate, formatting) #285

castwide opened this issue Mar 6, 2018 · 11 comments

Comments

@castwide
Copy link
Contributor

castwide commented Mar 6, 2018

If language server features are going to be offloaded to other providers (whether castwide.solargraph or something else), it might be practical to make features like rcodetools, rubyLocate, and formatting optional, if not removing them altogether, since there are potential issues with multiple providers performing redundant functions or interfering with each other.

@castwide castwide changed the title Disabling rcodetools and rubyLocate Disabling LSP features (rcodetools, rubyLocate, formatting) Mar 6, 2018
@doudou
Copy link
Contributor

doudou commented Mar 6, 2018

Aren't they already ?

vscode-ruby does not run any of the code formatting/linting unless explicitely told to do so.

For what it's worth, I don't believe there is another rubocop integration extension, is there ?

@wingrunr21
Copy link
Collaborator

I think this extension should support a minimum set of features out of the box and then allow delegation to a more feature-rich implementation if a user chooses (sort of like how VSCode provides very basic out of the box Ruby support).

Formatting and linting IMO fit in this category but things like Intellisense do not

@castwide
Copy link
Contributor Author

castwide commented Mar 6, 2018

There are a couple other extensions that use rubocop for linting and formatting. I also expect to use it in solargraph as part of its support for LSP.

Minimum features out of the box makes sense, but 1) where is the line, and 2) does the extension need configuration options to enable/disable them? I'm leaning toward yes for (2), since I've already run into a conflict with multiple definition providers giving redundant results; but maybe there's another way to resolve it?

Part of my concern also comes from open issues like #209 and #200, which @rebornix suggested as features that could be handled externally.

@castwide
Copy link
Contributor Author

castwide commented Mar 6, 2018

@doudou To clarify, you're correct that linting needs to be enabled explicitly, but some of the other features do not.

@wingrunr21
Copy link
Collaborator

wingrunr21 commented Mar 6, 2018

  1. I don't know where the line is TBH. Even if we keep linting here, we're going to have to come up with some way of performing linting in the background (similar to how the eslint is implemented) which probably requires an embedded language server (also how eslint is implemented). How we normalize that with downstream providers would be a challenge. Does VS Code have the internal APIs to alert users when there are multiple providers?
  2. Yes, probably. For the reasons outlined above. I wonder if we need to have "linter provider", "completion provider", etc as config options.

I may look through some of the JS community's solutions to these issues. There's a lot of conflicting tooling there so I'd be curious how they worked around things. I know for my personal setup that the extensions are more 'single responsibility' vs this extension which is trying to do everything.

@castwide
Copy link
Contributor Author

castwide commented Mar 6, 2018

Does VS Code have the internal APIs to alert users when there are multiple providers?

Good question. I don't know of a way, but it's worth looking into. When I tried to guard against conflicts, I resorted to explicitly querying one extension from the other, which is less than optimal.

I'll put some more thought into this as well. Looking to JS tools for inspiration is a good idea.

@castwide
Copy link
Contributor Author

castwide commented Mar 8, 2018

I've started some changes in my fork of vscode-ruby for this and #263. I'm waiting to submit a PR because I'd like to hear some other opinions and feedback. Current status:

  • castwide.solargraph is no longer a dependency
  • ruby.codeCompletion can be set to rcodetools or other
  • ruby.intellisense can be set to rubyLocate or other

The other option disables the feature for when another extension provides it. This should allow vscode-ruby to be more agnostic to other language servers. One thought I had, if the options are simply to turn them on or off, maybe they should be booleans instead?

I dug around to see if there was a way to automatically detect downstream providers, but couldn't come up with a strong solution.

@gurgeous
Copy link
Contributor

I think this is a positive direction. We want to offload work where possible and encourage an ecosystem of best-of-breed extensions rather than a single monolithic extension that moves forward at a glacial pace. Let @castwide lead the way withsolargraph!

Another example - I want to use rufo for formatting since rubocop is too slow. Rubocop formatting is actually so slow that it doesn't currently work in vscode for me at all (see microsoft/vscode#41194). Unfortunately, in vscode-ruby formatting config is the same as linting config so it's not really possible to use vscode-ruby/rubocop with vscode-rufo. This is another area where we may need to specify "other".

@gurgeous
Copy link
Contributor

BTW, I prefer false to 'other'. Just a thought.

@castwide
Copy link
Contributor Author

#309

@wingrunr21
Copy link
Collaborator

This will be addressed via #318

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

No branches or pull requests

4 participants