Use different comletion trigger character set for VSCode#11446
Conversation
|
|
||
| [Export(typeof(CompletionTriggerAndCommitCharacters))] | ||
| [method: ImportingConstructor] | ||
| class CohostCompletionTriggerAndCommitCharacters(LanguageServerFeatureOptions languageServerFeatureOptions) : CompletionTriggerAndCommitCharacters(languageServerFeatureOptions); |
| /// <summary> | ||
| /// Indicates that VSCode-compatible completion trigger character set should be used | ||
| /// </summary> | ||
| public abstract bool VsCodeCompletionTriggerCharacters { get; } |
There was a problem hiding this comment.
Nit: Consider adding "Use" to the front of this, to match the existing flags (and make it more verb-y)
…to delegating HTML client if trigger character is not HTML. Doing so causes issues in VS Code client because it will just return bad matches in that case, so we shouldn't even ask. I don't see any side-effects from not sending it to VS client either, seems logical not to send non-HTML trigger character request to HTML language server.
...azor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Remote/RemoteClientInitializationOptions.cs
Outdated
Show resolved
Hide resolved
|
|
||
| [Fact] | ||
| public async Task HtmlDelegation_UnsupportedTriggerCharacter_TranslatesToInvoked() | ||
| public async Task HtmlDelegation_UnsupportedTriggerCharacter_ReturnsNull() |
There was a problem hiding this comment.
It's interesting we had a test for this specific behavior... I don't know if this change would break anything, though I can't see how sending explicit completion invokes to HTML LSP for something that's not an HTML trigger character is a good thing. VS HTML at least behaves decently by returning nothing in inappropriate situation, but VS Code HTML is very aggressive with low quality matches ("better return anything than nothing"?) and that makes typing very annoying. In VS, it seems like a wasted message sent to HTML LSP.
| TriggerKind = RoslynCompletionTriggerKind.TriggerCharacter | ||
| TriggerCharacter = null, | ||
| TriggerKind = RoslynCompletionTriggerKind.Invoked | ||
| }, |
There was a problem hiding this comment.
Here we (probably me) were specifying bad input data. I debugged and confirmed VS actually sends the newly specified data in this change
|
@alexgav are we waiting for anything else before merging? Getting a few more customer reports of teh issue, would be good to get the fix out. |
DustinCampbell
left a comment
There was a problem hiding this comment.
I really appreciate the explanatory comments in your PR. Also, thanks for the extra bits of code clean up.
Summary of the changes
VS Code HTML language service is a lot more aggressive with showing low quality matches than VS HTML language service. We cannot use the same completion trigger characters in VS Code as in VS because it causes unwanted completion list with bad completion non-matches to pop up while user types in plain text.
Fixes:
dotnet/vscode-csharp#7678
dotnet/vscode-csharp#7871