Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Emulate suggestion mode in LSP completion by always soft-select #69327

Merged
merged 1 commit into from
Aug 2, 2023
Merged

Emulate suggestion mode in LSP completion by always soft-select #69327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 19 additions & 12 deletions .../LanguageServer/Protocol/Handler/Completion/AbstractLspCompletionResultCreationService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,15 @@ internal abstract class AbstractLspCompletionResultCreationService : ILspComplet
CompletionList list, bool isIncomplete, long resultId,
CancellationToken cancellationToken)
{
var isSuggestionMode = list.SuggestionModeItem is not null;
if (list.ItemsList.Count == 0)
{
return new LSP.VSInternalCompletionList
{
Items = Array.Empty<LSP.CompletionItem>(),
// If we have a suggestion mode item, we just need to keep the list in suggestion mode.
// We don't need to return the fake suggestion mode item.
SuggestionMode = list.SuggestionModeItem is not null,
SuggestionMode = isSuggestionMode,
IsIncomplete = isIncomplete,
};
}
Expand Down Expand Up @@ -132,18 +133,24 @@ internal abstract class AbstractLspCompletionResultCreationService : ILspComplet
lspItem.Kind = GetCompletionKind(item.Tags, capabilityHelper.SupportedItemKinds);
lspItem.Preselect = item.Rules.MatchPriority == MatchPriority.Preselect;

if (!lspItem.Preselect &&
!lspVSClientCapability &&
typedText.Length == 0 &&
item.Rules.SelectionBehavior != CompletionItemSelectionBehavior.HardSelection)
if (lspVSClientCapability)
{
// VSCode does not have the concept of soft selection, the list is always hard selected.
// In order to emulate soft selection behavior for things like argument completion, regex completion,
// datetime completion, etc. we create a completion item without any specific commit characters.
// This means only tab / enter will commit. VS supports soft selection, so we only do this for non-VS clients.
//
// Note this only applies when user hasn't actually typed anything and completion provider does not request the item
// to be hard-selected. Otherwise, we set its commit characters as normal. This also means we'd need to set IsIncomplete to true
lspItem.CommitCharacters = GetCommitCharacters(item, commitCharactersRuleCache);
return lspItem;
}

// VSCode does not have the concept of soft selection, the list is always hard selected.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm curious about this. It's not just VSCode that doesn't have soft selection, VS doesn't have it either in the LSP client. Should the above block (checking lspVSClientCapability) even be present? Certainly we have this issue in VS in Razor, so is it just that nobody turns on LSP for C# files in VS, so this code is incorrect but we're just assuming it's correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VS client has suggestion mode (VSInternalCompletionList.SuggestionMode), so I assumed it has similar behavior s in VS. Maybe I was wrong, will need to try it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VS doesn't have it either in the LSP client.

Wat.

Copy link
Member Author

@genlu genlu Aug 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the doc-comment for VS client's suggestion mode, which suggests there's soft-selection. But as David said, it might not work as we expected.

 //
 // Summary:
 //     Gets or sets a value indicating whether the completion list should use suggestion
 //     mode. In suggestion mode items are "soft-selected" by default.
 [DataMember(Name = "_vs_suggestionMode")]
 [JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
 public bool SuggestionMode { get; set; }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah VS LSP client does have some support for soft selection with the SuggestionMode. I don't remember the extent of it, but it definitely exists.

Copy link
Contributor

@davidwengier davidwengier Aug 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL. Sounds like we need to try that! Thanks

// In order to emulate soft selection behavior for things like suggestion mode, argument completion, regex completion,
// datetime completion, etc. we create a completion item without any specific commit characters.
// This means only tab / enter will commit. VS supports soft selection, so we only do this for non-VS clients.
if (isSuggestionMode)
{
lspItem.CommitCharacters = Array.Empty<string>();
}
else if (!lspItem.Preselect && typedText.Length == 0 && item.Rules.SelectionBehavior != CompletionItemSelectionBehavior.HardSelection)
{
// Note this also applies when user hasn't actually typed anything and completion provider does not request the item
// to be hard-selected. Otherwise, we set its commit characters as normal. This means we'd need to set IsIncomplete to true
// to make sure the client will ask us again when user starts typing so we can provide items with proper commit characters.
lspItem.CommitCharacters = Array.Empty<string>();
isIncomplete = true;
Expand Down
36 changes: 36 additions & 0 deletions src/Features/LanguageServer/ProtocolUnitTests/Completion/CompletionFeaturesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.LanguageServer.Protocol;
using Roslyn.Test.Utilities;
using Roslyn.Utilities;
using Xunit;
using Xunit.Abstractions;
using LSP = Microsoft.VisualStudio.LanguageServer.Protocol;
Expand Down Expand Up @@ -555,4 +556,39 @@ void M()
var expectedAdditionalEdit2 = new TextEdit() { NewText = "using Namespace2;\r\n\r\n", Range = new() { Start = new(1, 0), End = new(1, 0) } };
AssertJsonEquals(new[] { expectedAdditionalEdit2 }, resolvedItem2.AdditionalTextEdits);
}

[Theory, CombinatorialData, WorkItem("https://github.com/dotnet/vscode-csharp/issues/5732")]
public async Task TestEmptyCommitCharsInSuggestionMode(bool mutatingLspWorkspace)
{
var markup =
@"
using System.Collections.Generic;
using System.Linq;
public class C
{
public Foo(List<int> myList)
{
var foo = myList.Where(i{|caret:|})
}
}";
await using var testLspServer = await CreateTestLspServerAsync(markup, mutatingLspWorkspace, DefaultClientCapabilities);
var caret = testLspServer.GetLocations("caret").Single();
var completionParams = new LSP.CompletionParams()
{
TextDocument = CreateTextDocumentIdentifier(caret.Uri),
Position = caret.Range.Start,
Context = new LSP.CompletionContext()
{
TriggerKind = LSP.CompletionTriggerKind.Invoked,
}
};

var document = testLspServer.GetCurrentSolution().Projects.First().Documents.First();

var results = await testLspServer.ExecuteRequestAsync<LSP.CompletionParams, LSP.CompletionList>(LSP.Methods.TextDocumentCompletionName, completionParams, CancellationToken.None);
AssertEx.NotNull(results);
Assert.NotEmpty(results.Items);
Assert.Empty(results.ItemDefaults.CommitCharacters);
Assert.True(results.Items.All(item => item.CommitCharacters is null));
}
}