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

LSP references results deduplication #3328

Open
joeveiga opened this issue Oct 15, 2024 · 5 comments
Open

LSP references results deduplication #3328

joeveiga opened this issue Oct 15, 2024 · 5 comments
Labels
enhancement Enhancement to performance, inner workings or existent features

Comments

@joeveiga
Copy link
Contributor

joeveiga commented Oct 15, 2024

Is your feature request related to a problem? Please describe.
After #3211, now I'm seeing duplicate results in my lsp references results:

image

It looks like this was a known issue when the feature was added. From the PR description:

This leaves us with the issue of what happens if both LSPs returning overlapping locations, so deduplication could be in order
But since this isn't my case and I suspect not a lot of people use multiple LSPs that return the same stuff, I prefer to get your feedback first and maybe implement it later

Unfortunately in my usecase I do need a way to dedup the list :). I'm using both angularls and ts_ls, and they often return the same locations. This is normally just an annoyance, but in my workflow I tend to send telescope results to the qf list and cdo some bulk updates. I now worry I'm gonna start messing things by updating the same location(s) twice.

Describe the solution you'd like
It'd be useful to have an option to remove duplicates from the results (probably enabled by default, even). Or just remove them. I doubt anyone needs duplicates in their list but, who knows :).

Describe alternatives you've considered
This my not-so-great quick solution for now. I'm sure there is a better way to generate a unique key for a location than just stringifying it ;)

image

@joeveiga joeveiga added the enhancement Enhancement to performance, inner workings or existent features label Oct 15, 2024
@jamestrew
Copy link
Contributor

Is angularls and ts_ls intended to be used at the same time on the same buffers?
Seems strange to me that two language server would yield identical references.

Trying to think what the best way of handling this is. Curious how :lua vim.lsp.buf.references() handles this.

@joeveiga
Copy link
Contributor Author

This is the output from vim.lsp.buf.references() on the same symbol as the previous screenshot with telescope:

image

Looks like there aren't any duplicates.

@jamestrew
Copy link
Contributor

If you do :chistory do you see two quickfix lists for lsp reference?

@joeveiga
Copy link
Contributor Author

joeveiga commented Oct 16, 2024

Hey @jamestrew. Yeah, I get two:

image

That's the behavior I was getting before this latest update with telescope as well. The angularls response was replacing the ts_ls results in telescope lsp references window. Which is not the desired behavior, btw :). I think merging both responses is a good feature. I just would like to avoid duplicates if possible.

Seems strange to me that two language server would yield identical references.

So the short version is, angularls is basically a superset of ts_ls (the angular compiler uses the typescript compiler under the hood to compile component files). However, we still need ts_ls to attach to .ts files in cases when we're working on a non-angular typescript library. So we end up with angularls attaching to .ts and .html files, and .ts attaching to .ts files. As an example, doing a lsp references request on a component class field, we'd get typescript references to that field from ts_ls (because it is a typescript class), and we would also get those from angularls plus references to the field coming from html template files (because it is a typescript class for an angular component). Hopefully that makes some sense.

@jamestrew
Copy link
Contributor

Looks like neovim is merging references (and generally handling multiple clients for various lsp handlers similar to how telescope has started doing this) now.
neovim/neovim#30722

It doesn't look like there's any de-duping logic there. I'm curious if they will add it and if so what the implementation will be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to performance, inner workings or existent features
Projects
None yet
Development

No branches or pull requests

2 participants