Support MatchPriority comparison in LSP completion#83164
Support MatchPriority comparison in LSP completion#83164ToddGrun merged 3 commits intodotnet:mainfrom
Conversation
When using C# LSP-based completion, typing "nu" incorrectly selects nuint instead of null. In Roslyn's native (non-LSP) completion, null is correctly selected. Changes This PR adds a _vs_matchPriority VS-internal extension property to VSInternalCompletionItem so the full numeric MatchPriority value is available to the VS client for tiebreaking during best-match selection. There is a companion VsLanguageServerClient PR (https://devdiv.visualstudio.com/DevDiv/_git/VSLanguageServerClient/pullrequest/728272) that adds a custom IAsyncCompletionItemManager which consumes _vs_matchPriority as a tiebreaker in best-match selection.
ToddGrun
requested review from
davidwengier,
dibarbet,
jasonmalinowski,
kayle,
olegtk and
sandyarmstrong
dibarbet
left a comment
dibarbet
left a comment
There was a problem hiding this comment.
Will this break anything if we merge before the client side change?
dibarbet
left a comment
dibarbet
left a comment
There was a problem hiding this comment.
Actually... is this not something we could solve in the base LSP completion with sort text? E.g. the sort text for null puts it above nunit?
Discussed offline - passing match priority allows the server to modify selection, without changing list order. This is technically possible with the base lsp spec using |
|
I don't think so (will verify), but was planning on waiting for the work on the VS LSP side to get approved and merged before proceeding on this. In reply to: 4108793162 |
Verified that the behavior isn't bad without the VSEditor or VSLanguageServerClient side. Both VSEditor and VSLanguageServerClient have been merged (VSEditor is already in IntMain, VSLanguageServerClient will be there in the next day or two). Going ahead and merging the Roslyn side and this should all work once they meet in VS Main. |
…ture * upstream/main: (77 commits) Fix ArgumentNullException in VB Edit and Continue (dotnet#83250) Fix property pattern completion filtering out member being edited (dotnet#83230) Add branch merge skill (dotnet#83229) [main] Source code updates from dotnet/dotnet (dotnet#83215) Support MatchPriority comparison in LSP completion (dotnet#83164) Have CompleteStatement handle EOF statements (dotnet#83205) Minor cleanups related to attributes in VB (dotnet#83206) Simplify Address additional PR feedback Port remaining unit test projects to Linux (dotnet#83153) Unsafe evolution: allow unsafe property accessors (dotnet#83115) Address PR review feedback Allow cohost rename in Razor source-generated docs Refine code review skill (dotnet#82666) Review feedback Allow creation of DocumentUri instances even if System.Uri cannot parse it Improve TextLine and line table performance by packing existing data into unused bits (dotnet#83000) Skip TestFindReferencesAsync_UsingAlias on non-Windows platforms (dotnet#83188) [main] Source code updates from dotnet/dotnet (dotnet#83174) fix comment ...
2 participants
When using C# LSP-based completion, typing "nu" incorrectly selects nuint instead of null. In Roslyn's native (non-LSP) completion, null is correctly selected.
Approach
This PR adds a _vs_matchPriority VS-internal extension property to VSInternalCompletionItem so the full numeric MatchPriority value is available to the VS client for tiebreaking during best-match selection.
There are two companion PRs for to support this change:
1: VSLanguageServerClient PR
2: VSEditor PR
Microsoft Reviewers: Open in CodeFlow