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

Plugins cannot define jump-to-definition handlers (responses are not combined) #3634

Closed
jvanbruegge opened this issue Jun 10, 2023 · 5 comments
Assignees
Labels
level: easy The issue is suited for beginners type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..

Comments

@jvanbruegge
Copy link
Collaborator

As can be seen here https://github.com/haskell/haskell-language-server/blob/master/hls-plugin-api/src/Ide/Types.hs#L555 HLS always returns the first response for jump-to-definition, which happens to be ghcide. Multiple responses could be combined into one, by turning LocationLink into a Location

@jvanbruegge jvanbruegge added status: needs triage type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc.. labels Jun 10, 2023
@michaelpj
Copy link
Collaborator

In fact, I think Location can be turned into LocationLink, which is the richer type:

  • uri -> targetUri
  • range -> targetRange
  • range -> targetSelectionRange
  • don't provide originSelectionRange

And the other direction:

  • targetRange -> range
  • targetUri -> uri

So I think what we would want to do is check the client capability for LocationLink support, and depending on whether it's set translate Locations to LocationLinks or vice versa.

@michaelpj
Copy link
Collaborator

... and then just concatenate the results

@joyfulmantis
Copy link
Collaborator

Currently the type the plugins provide for is the same as the ones lsp sends on to the client. I was thinking it would be nice to have a type the plugins need to provide that is separate from the actual method type the client accepts, specifically by being more specific. In this case only accepting locationlink for example, and then refactor the combine responses to convert that type into the client accepted type (ideally by querying capabilities) before combining. Separate types would allow us to enforce that the plugin is only returning our preferred type.

@michaelpj
Copy link
Collaborator

Yeah, I agree that that would be nice, might be a bit of a pain to implement. But we could almost certainly provide types that are e.g. easier to combine.

@jvanbruegge
Copy link
Collaborator Author

This was fixed by #3846

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
level: easy The issue is suited for beginners type: bug Something isn't right: doesn't work as intended, documentation is missing/outdated, etc..
Projects
None yet
Development

No branches or pull requests

4 participants