nvim-telescope / telescope.nvim

Find, Filter, Preview, Pick. All lua, all the time.
MIT License
15.83k stars 835 forks source link

LSP references results deduplication #3328

Open joeveiga opened 1 week ago

joeveiga commented 1 week ago

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

jamestrew commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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

joeveiga commented 1 week ago

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 commented 5 days ago

Looks like neovim is merging references (and generally handling multiple clients for various lsp handlers similar to how telescope has started doing this) now. https://github.com/neovim/neovim/pull/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.