[LSP] Explicitly define the completion items to resolve#60393
Merged
dibarbet merged 1 commit intodotnet:mainfrom Mar 26, 2022
Merged
[LSP] Explicitly define the completion items to resolve#60393dibarbet merged 1 commit intodotnet:mainfrom
dibarbet merged 1 commit intodotnet:mainfrom
Conversation
dibarbet
commented
Mar 25, 2022
| LSP.Range? defaultRange, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| if (supportsVSExtensions) |
Member
Author
There was a problem hiding this comment.
this method was kind of useless, so I just merged it with the local CreateCompletionItemAsync and do the check there
allisonchou
approved these changes
Mar 25, 2022
| Preselect = ShouldItemBePreselected(item), | ||
| }; | ||
| var completionItem = supportsVSExtensions ? new LSP.VSInternalCompletionItem() : new LSP.CompletionItem(); | ||
| completionItem.Label = completeDisplayText; |
Contributor
There was a problem hiding this comment.
Wish there was a C# construct to do this as one (for non-structs, operating on the same object).
| // Complex text edits (e.g. override and partial method completions) are always populated in the | ||
| // resolve handler, so we leave both TextEdit and InsertText unpopulated in these cases. | ||
| if (item.IsComplexTextEdit) | ||
| if (item.IsComplexTextEdit && completionItem is LSP.VSInternalCompletionItem vsItem) |
Contributor
There was a problem hiding this comment.
Are ComplexTextEdit's only supported in VS? I would have thought LSP.VSInternalCompletionItem should be it's own check but perhaps I don't understand the scenario.
Member
Author
There was a problem hiding this comment.
complex text edits exist outside of VS, but the capability to resolve on commit is VS only. So it doesn't make sense to do this block outside of VS
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Response to https://devdiv.visualstudio.com/DevDiv/_git/VSLanguageServerClient/pullrequest/385546
Instead of relying on the implicit contract of no insert text + text edit (which is not necessarily applicable with item defaults), we specify a property to VS which tells them to resolve on commit.