Skip to content

"textDocument/completion" answer items not alphabetically sorted #40346

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

Closed
jonas-jonas opened this issue Jan 28, 2020 · 7 comments
Closed

"textDocument/completion" answer items not alphabetically sorted #40346

jonas-jonas opened this issue Jan 28, 2020 · 7 comments
Labels
devexp-completion Issues with the analysis server's code completion feature devexp-lsp Issues with analysis server's support of Language Server Protocol legacy-area-analyzer Use area-devexp instead. type-question A question about expected behavior or functionality

Comments

@jonas-jonas
Copy link
Contributor

In Dartboard we noticed that textDocument/completion items are not alphabetically sorted, which makes it hard to predict for the user. In VSCode these entries seem to be sorted on the client? What is this sorting based on? Is it just the default behavior, or is there some way to determine how a response's items should be sorted in the LSP?

@natebosch
Copy link
Member

The spec does not require the server to send items in any particular order. The client can choose to sort in whatever way makes sense. Alpha sorting on sortText (if present), and falling back on alpha sorting the label is probably common. Clients may also choose to sort deprecated suggestions lower. Clients may also choose to allow some sorting based on either prefix or fuzzy matching of characters the user has already typed.

See the spec at https://microsoft.github.io/language-server-protocol/specification#textDocument_completion

Closing this issue for now since I don't think we plan on adding server side sorting.

@natebosch natebosch added devexp-lsp Issues with analysis server's support of Language Server Protocol legacy-area-analyzer Use area-devexp instead. type-question A question about expected behavior or functionality labels Jan 29, 2020
@bwilkerson
Copy link
Member

I don't think we plan on adding server side sorting.

We might want to re-think that. Server already computes relevance information, and it seems a shame to not share that with LSP clients.

@bwilkerson bwilkerson reopened this Jan 29, 2020
@bwilkerson bwilkerson added the devexp-completion Issues with the analysis server's code completion feature label Jan 29, 2020
@vogella
Copy link

vogella commented Jan 29, 2020 via email

@bwilkerson
Copy link
Member

It seems like it would be fairly easy to sort before returning the items. It looks like LSP also defines a sortText to allow clients to sort items. I'm not sure what the best approach would be.

@DanTup In case you have ideas.

@DanTup
Copy link
Collaborator

DanTup commented Jan 29, 2020

We are already sorting (via sortText), but it's deliberately not alphabetical, it's based on the relevance calculated by the server (we have to convert the numeric value to a strong since LSP is text-based sorting):

(1000000 - itemRelevance).toString(),

I think this is the correct thing to do - it will put the most relevant things at the top (mostly - we do still have #38739).

I don't think we should spend resources sorting the collection though as the client should always be sorting based on sortText (if it's not set, it should fall back to label - so I don't think a client would/should ever not sort).

As for VS Code, it will use the order we provide it initially, but as soon as the user types any characters (such that the results are filtered client-side), it will take over sorting using its own relevance calculations (something I think is bad in some ways - it has no language context - but good in others - since the server only got to provide the order for the full unfiltered list).

@jonas-jonas does this seem to match what you're seeing?

@DanTup
Copy link
Collaborator

DanTup commented May 13, 2020

I believe there's nothing to do here - we provide sortText that's sorted by our relevance. Clients can use this, or implement their own sorting (VS Code uses our sort initially, but then does its own relevance ranking as the user types more characters, since it doesn't go back to the server for updated relevance).

Unless anyone thinks this is incorrect, I think we can probably close this? (I don't have permission to do so though).

@bwilkerson
Copy link
Member

I agree. Happy to re-open if more discussion is needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-completion Issues with the analysis server's code completion feature devexp-lsp Issues with analysis server's support of Language Server Protocol legacy-area-analyzer Use area-devexp instead. type-question A question about expected behavior or functionality
Projects
None yet
Development

No branches or pull requests

5 participants