Skip to content

Partial fix for https://github.com/dotnet/vscode-csharp/issues/7678 #11681

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ internal static class CompletionListMerger
[return: NotNullIfNotNull(nameof(delegatedCompletionList))]
public static VSInternalCompletionList? Merge(VSInternalCompletionList? razorCompletionList, VSInternalCompletionList? delegatedCompletionList)
{
if (razorCompletionList is null)
// In VSCode case we always think completion was invoked explicitly and create empty Razor completion list,
// so check for empty Items collection as well.
if (razorCompletionList is null || razorCompletionList.Items.Length == 0)
{
return delegatedCompletionList;
}
Expand All @@ -38,7 +40,8 @@ internal static class CompletionListMerger
var mergedIsIncomplete = razorCompletionList.IsIncomplete || delegatedCompletionList.IsIncomplete;
var mergedItems = razorCompletionList.Items.Concat(delegatedCompletionList.Items).ToArray();
var mergedData = MergeData(razorCompletionList.Data, delegatedCompletionList.Data);
var mergedCommitCharacters = razorCompletionList.CommitCharacters ?? delegatedCompletionList.CommitCharacters;
var mergedCommitCharacters = razorCompletionList.CommitCharacters
?? delegatedCompletionList.CommitCharacters;
var mergedSuggestionMode = razorCompletionList.SuggestionMode || delegatedCompletionList.SuggestionMode;

// We don't fully support merging continue characters currently. Razor doesn't currently use them so delegated completion lists always win.
Expand All @@ -58,6 +61,8 @@ internal static class CompletionListMerger
ItemDefaults = new CompletionListItemDefaults()
{
EditRange = mergedItemDefaultsEditRange,
// VSCode won't use VSInternalCompletionList.CommitCharacters, make sure we don't lose item defaults
CommitCharacters = razorCompletionList.ItemDefaults?.CommitCharacters ?? delegatedCompletionList.ItemDefaults?.CommitCharacters
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ internal class CompletionTriggerAndCommitCharacters
private static readonly char[] s_vsCodeHtmlTriggerCharacters = ['#', '.', '!', ',', '-', '<'];
private static readonly char[] s_razorTriggerCharacters = ['<', ':', ' '];
private static readonly char[] s_csharpTriggerCharacters = [' ', '(', '=', '#', '.', '<', '[', '{', '"', '/', ':', '~'];
private static readonly char[] s_commitCharacters = [' ', '>', ';', '='];
private static readonly ImmutableArray<string> s_commitCharacters = [" ", ">", ";", "="];

private readonly HashSet<char> _csharpTriggerCharacters;
private readonly HashSet<char> _delegationTriggerCharacters;
Expand Down Expand Up @@ -68,15 +68,16 @@ public CompletionTriggerAndCommitCharacters(LanguageServerFeatureOptions languag
allTriggerCharacters.UnionWith(razorTriggerCharacters);
allTriggerCharacters.UnionWith(delegationTriggerCharacters);

var commitCharacters = new HashSet<char>();
commitCharacters.UnionWith(s_commitCharacters);

_csharpTriggerCharacters = csharpTriggerCharacters;
_htmlTriggerCharacters = htmlTriggerCharacters;
_razorTriggerCharacters = razorTriggerCharacters;
_delegationTriggerCharacters = delegationTriggerCharacters;

// We shouldn't specify commit characters for VSCode.
// It doesn't appear to need them and they interfere with normal item commit.
// E.g. see https://github.com/dotnet/vscode-csharp/issues/7678
AllCommitCharacters = languageServerFeatureOptions.UseVsCodeCompletionTriggerCharacters ? [] : s_commitCharacters;
AllTriggerCharacters = allTriggerCharacters.SelectAsArray(static c => c.ToString());
AllCommitCharacters = commitCharacters.SelectAsArray(static c => c.ToString());
}

public bool IsValidCSharpTrigger(CompletionContext completionContext)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer.Completion;

public class CompletionListProviderTest : LanguageServerTestBase
{
private readonly VSInternalCompletionList _completionList1;
private readonly VSInternalCompletionList _completionList2;
private readonly VSInternalCompletionList _razorCompletionList;
private readonly VSInternalCompletionList _delegatedCompletionList;
private readonly RazorCompletionListProvider _razorCompletionProvider;
private readonly DelegatedCompletionListProvider _delegatedCompletionProvider;
private readonly VSInternalCompletionContext _completionContext;
Expand All @@ -35,10 +35,10 @@ public class CompletionListProviderTest : LanguageServerTestBase
public CompletionListProviderTest(ITestOutputHelper testOutput)
: base(testOutput)
{
_completionList1 = new VSInternalCompletionList() { Items = [] };
_completionList2 = new VSInternalCompletionList() { Items = [] };
_razorCompletionProvider = new TestRazorCompletionListProvider(_completionList1, LoggerFactory);
_delegatedCompletionProvider = new TestDelegatedCompletionListProvider(_completionList2);
_razorCompletionList = new VSInternalCompletionList() { Items = [] };
_delegatedCompletionList = new VSInternalCompletionList() { Items = [] };
_razorCompletionProvider = new TestRazorCompletionListProvider(_razorCompletionList, LoggerFactory);
_delegatedCompletionProvider = new TestDelegatedCompletionListProvider(_delegatedCompletionList);
_completionContext = new VSInternalCompletionContext();
_documentContext = TestDocumentContext.Create("C:/path/to/file.cshtml");
_clientCapabilities = new VSInternalClientCapabilities();
Expand All @@ -53,12 +53,13 @@ public async Task MultipleCompletionLists_Merges()
var provider = new CompletionListProvider(_razorCompletionProvider, _delegatedCompletionProvider, _triggerAndCommitCharacters);

// Act
var completionList = await provider.GetCompletionListAsync(
var mergedCompletionList = await provider.GetCompletionListAsync(
absoluteIndex: 0, _completionContext, _documentContext, _clientCapabilities, _razorCompletionOptions, correlationId: Guid.Empty, cancellationToken: DisposalToken);

// Assert
Assert.NotSame(_completionList1, completionList);
Assert.NotSame(_completionList2, completionList);
Assert.Empty(mergedCompletionList.Items);
Assert.NotSame(_razorCompletionList, mergedCompletionList);
Assert.Same(_delegatedCompletionList, mergedCompletionList);
}

[Fact]
Expand All @@ -76,7 +77,7 @@ public async Task MultipleCompletionLists_DifferentCommitCharacters_OnlyCallsApp
absoluteIndex: 0, _completionContext, _documentContext, _clientCapabilities, _razorCompletionOptions, correlationId: Guid.Empty, cancellationToken: DisposalToken);

// Assert
Assert.Same(_completionList2, completionList);
Assert.Same(_delegatedCompletionList, completionList);
}

private class TestDelegatedCompletionListProvider : DelegatedCompletionListProvider
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ private async Task VerifyCompletionListAsync(
delegatedItemLabels ??= [InvalidLabel];
var response = new VSInternalCompletionList()
{
Items = delegatedItemLabels.Select((label) => new VSInternalCompletionItem()
Items = delegatedItemLabels.Select((label) => new CompletionItem()
{
Label = label,
CommitCharacters = delegatedItemCommitCharacters,
Expand Down