Result ordering of textDocument/definition

[lewis6991]

Currently in Neovim, the "go to definition" implementation favours the first result returned by the server. However, we have found at least one server (lua-language-server) that returns this list with the worst match first.

Should the spec be more explicit in how these results can be ordered, or is it up to the client to order the results?

Issue that raised this question: neovim/neovim#30025

[dbaeumer]

So far the spec leaves sorting and scoring up to the client. So from a spec perspective all results are equally relevant.

[lewis6991]

Thank you for the clarification.

We've found a number of servers are already sorting the results (clangd, jedi, lua-language-server). Would it make sense to add a server capability so the client can use this information?

[davidhalter]

Note that Jedi mostly sorts to make results idempotent (I'm the Jedi author), while clangd seems to sort for relevance, which is obviously more advanced and more interesting (and probably more important for C-like languages).

[dbaeumer]

To stay inline with the spec I think a client should let the server know, if it respects the order or not.

[rchl]

(Note that once filtering is involved in the client, I don't think that the client will be able to respect the original ordering. For example clients can and likely will score consecutive matching letters higher. So it's really something that would only be useful before filtering is applied.)

[lewis6991]

For example clients can and likely will score consecutive matching letters higher.

I'm not sure we're talking about the same thing. This issue is for textDocumet/Definition. The most the client can do is order based on path similarity of the results relative to the current document.

The client simply needs to know if the server has ordered the results. If it has, then the client can choose not to re-order since the server can do a better job (e.g. clangd).

[rchl]

The result can include multiple items and then client can provide functionality to filter those items.
That said, textDocument/definition should in theory not produce too many results so maybe filtering is not that common anyway in that specific case.

[lewis6991]

To stay inline with the spec I think a client should let the server know, if it respects the order or not.

@dbaeumer are you sure about this? At the very least, the client needs to know if the server has ordered the results. It makes little/no difference to the server if the client utilizes this or not. I was thinking this should be a field in DefinitionOptions named something like resultSort? boolean?

[mfussenegger]

To add a bit more context:

Neovim provides a way for users to immediately jump to the first result of textDocument/definition. It is therefore in the client and users' interest that the result is ordered with the more relevant items coming first.

But if the server doesn't order the result, we could at least apply some heuristic (like prioritize definitions in the current file) - assuming that that's likely better than a completely random result.
(Users still have the option to show all results and select what they wanted)

To stay inline with the spec I think a client should let the server know, if it respects the order or not.

This would allow the server to skip sorting, but that wouldn't solve the above problem/use-case.

I was thinking this should be a field in DefinitionOptions named something like resultSort? boolean?

+1, although I'd probably switch it to unsorted with absent==false, so that for servers not providing the information the assumption is that the result is sorted - which would match what some servers already do right now.
And the text could include a hint that servers are encouraged to sort the result, placing more relevant matches first.

Edit: But my favourite resolution would be to only add a text that strongly encourages the server to order by relevance without the addition of a new property. Without knowledge of language semantics a client can't order the result in a way that's reliably an improvement (outside of fuzzy matching and scoring after a narrowing filter). Making it a clear responsibility of the server

[DanTup]

Neovim provides a way for users to immediately jump to the first result of textDocument/definition

VS Code also does this and considers the top result the "primary result":

So any server not doing that today is probably giving a less-than-ideal experience if the server is used in VS Code. I think this kind of thing should be specified - it's difficult when creating a client or a server when there's ambiguity like this because you have to start poking around trying to figure out what editors/servers your users will use and whether there is a consensus on behaviour - that's what I want from the spec! (even if it contains things that are just guidelines and not necessary to follow).

It might be nice if the servers could return a sorted: boolean with the results, but the results of the request are unfortunately a bare array. I think it would be better generally of LSP always had "Result" types wrapping results to allow for additional fields to be added in future - though obvious that ship has sailed for most of them (at least unless capabilities are added to allow changing the return type).

[dbaeumer]

The problem with the sorted on the result is that we need to guard this with a capability in the client. Otherwise we would break existing clients since they will not adhere to the spec. If that capability is not set or set to false servers need to assume that clients might not respect the sorting. So there is no win over a single client capability.

The only non breaking way I see is that we either have a single client capability or that the request has an additional param property to tell the server to rank/sort the result. We can have a server capability to let clients know if server adhere to that property.