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

vscode solargraph server does not enable "go to definition" #127

Closed
njyrbecker opened this issue Jun 19, 2019 · 9 comments
Closed

vscode solargraph server does not enable "go to definition" #127

njyrbecker opened this issue Jun 19, 2019 · 9 comments

Comments

@njyrbecker
Copy link

njyrbecker commented Jun 19, 2019

VSCode version: 1.34.0
ruby solargraph extension: 0.19.6
solargraph gem version: 0.33.1

If I simply allow vscode to run the solargraph server, I see it running via ps with options "solargraph socket --port 0", and while intellisense completions appear, go to definition is not among the options when I right-click on a function call.

If I start solargraph manually (the extension-managed server and manually run servers will be running side-by-side) with "solargraph socket" - the only difference being that I am not specifying a port, go-to-definition is immediately available when I right-click a function name.

@njyrbecker
Copy link
Author

Figured out a bit more. Apparently the "Ruby" extension is what's providing the "go to definition", rather than solargraph ruby. Is solargraph ruby expected to provide "go to definition" ?

It appears that the "ruby" extension is providing the "go-to-definition" functionality rather than the "ruby solargraph" extension, and it appears that the ruby extension doesn't like talking to the solargraph server the ruby solargraph extension runs, thus I need to run a second solargraph server for the ruby extension to get go-to-definition to work.

If I disable the "ruby" extension, I cannot get "go-to-definition" to show at all. Either extension appears to enable autocomplete, but only ruby solargraph seems to provide the ability to rename a symbol.

Should I need both extensions? Are they expected to work side-by-side? Should I need two solargraph socket servers?

@castwide
Copy link
Owner

castwide commented Jun 19, 2019

Is it possible that the extension-managed server hasn't finished initializing yet? The status message in the lower left should change from "Starting the Solargraph language server" to "Solargraph is ready." It might take a while depending on the size of your workspace. For comparison, initializing the castwide/solargraph repo takes about 8 seconds on my PC, while rails/rails takes about 37.

Completions during initialization are probably coming from VS Code's word-based suggestions instead of Solargraph.

Edit: Solargraph does provide its own "go to definition," and you definitely shouldn't need to run a second server. The extension shouldn't see the manually started server at all.

@njyrbecker
Copy link
Author

OK... it does appear I wasn't waiting long enough for the solargraph server to initialize. I'll run with just the solargraph extension for a while and see if it appears to handle all the ruby bits I'm looking for.

Are you aware of any substantial functionality provided by the "ruby" extension which solargraph ruby doesn't provide?

As a side note, solargraph seems pretty slow; "find all references" on a single function takes ~30 seconds. Code base is ~5 yrs old, a "find . -name "*.rb" | xargs wc -l shows ~140k lines, but still, I can run find | xargs grep in under 2 seconds. I get that solargraph is probably doing something far more sophisticated, but any way for it to do a gross search first and then examine the results with the more sophisticated techniques?

@castwide
Copy link
Owner

castwide commented Jun 19, 2019

One major feature in vscode-ruby that vscode-solargraph doesn't provide is the debugger. It also provides some color syntax rules and similar cosmetic features that are somewhat outside of a language server's scope. I use them side-by-side and intend to continue keeping them compatible with each other.

A gross search for find-all-references isn't a bad idea. The current implementation does significantly more than a grep; for example, it extrapolates the difference between calls to Array#include? and String#include?. It might be worthwhile to makes its behavior configurable.

In the meantime, vscode-ruby does provide its own version of go-to-definition that's less context-aware but should be a lot faster on large workspaces. You can switch between them by setting a couple of preferences in VS Code:

"solargraph.definitions": false
"ruby.intellisense": "rubyLocate"

@njyrbecker
Copy link
Author

njyrbecker commented Jun 19, 2019

What I was suggesting was doing a gross search first, then processing the results of said gross search through whatever sophisticated mechanism you have. 30 seconds for 140k lines implies you're processing all or most of those lines through some pretty intensive code. Gross reduction as a first pass should help a great deal. I do understand that some ruby language features such as delegate-to and aliasing complicate things.

VSCode already provides "find in files" for pure gross search functionality.

My apologies if I've got a lot of questions; I'm coming from Java development in IntelliJ and coming up to speed on an enormous repository and ruby tooling at the same time, so it's a bit challenging. I would say that understanding the key unique features provided by both vscode ruby and vscode ruby solargraph, notable differences between the implementations, and any config bits allowing to switch between them would be really useful to include in documentation if that's not impossible to maintain.

I do acknowledge this is free software, and if I'm eating too much support time, you're well within your rights to tell me to go figure things out myself.

@castwide
Copy link
Owner

What I was suggesting was doing a gross search first, then processing the results of said gross search through whatever sophisticated mechanism you have.

Ah, I see what you're saying. Good call. The process already uses two passes, but the first pass is way more intensive than it needs to be. It traverses a cached AST of each file in the workspace, and I'm sure it winds up drilling into nodes that it should have already been able to filter. I'll look into it.

My apologies if I've got a lot of questions;

No worries. I provide support through GitHub issues as well as I can, and especially appreciate any feedback that helps improve the software. I'm also working on more documentation to cover FAQs and hope to get it online in the next few weeks.

@njyrbecker
Copy link
Author

njyrbecker commented Jun 19, 2019

One more note- solargraph does not appear to understand rails before_actions; in a controller, if I've got something like:

before_action :validate_something, only: [:create]
even if the controller itself contains the validate_something method, go to definition does not seem to be able to figure out where it is. This is consistent across pretty much any use of activemodel concerns.

@castwide
Copy link
Owner

Yeah, Rails support is very much a work in progress. There's an open issue for it that includes some workarounds to fill in some of the gaps: castwide/solargraph#87

Controller callbacks like before_action are an especially weird issue. Not only are they magically included at runtime, the methods themselves don't even appear in Rails' generated YARD documentation, so there's no way to explicitly include them yourself. I have a possible solution in progress, but it's not trivial.

@njyrbecker
Copy link
Author

Thank you for the pointer.

Read the thread, updated .solargraph.yml, dropped the contents of the gist into config/solargraph.rb, and the basic case of go to definition for a concern still doesn't work. My personal take is that the most important functions of a code graphing tool are syntax highlighting > go to definition > find all references > typing completion, fwiw. Code completion suggestions are sexy, but kind of the least important on the list, as once you learn a code base, the code will flow, and ruby in particular enables a great deal of brevity over boilerplate. If that's helpful for feature prioritization, great, otherwise cest la vie.

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

No branches or pull requests

2 participants