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.

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

Use different comletion trigger character set for VSCode #11446

Open
wants to merge 5 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 @@ -41,14 +41,14 @@ public async Task SetupAsync()
var documentMappingService = lspServices.GetRequiredService<IDocumentMappingService>();
var clientConnection = lspServices.GetRequiredService<IClientConnection>();
var completionListCache = lspServices.GetRequiredService<CompletionListCache>();
var completionTriggerAndCommitCharacters = lspServices.GetRequiredService<CompletionTriggerAndCommitCharacters>();
var loggerFactory = lspServices.GetRequiredService<ILoggerFactory>();

var delegatedCompletionListProvider = new TestDelegatedCompletionListProvider(documentMappingService, clientConnection, completionListCache);
var delegatedCompletionListProvider = new TestDelegatedCompletionListProvider(documentMappingService, clientConnection, completionListCache, completionTriggerAndCommitCharacters);
var completionListProvider = new CompletionListProvider(razorCompletionListProvider, delegatedCompletionListProvider);
var configurationService = new DefaultRazorConfigurationService(clientConnection, loggerFactory);
var optionsMonitor = new RazorLSPOptionsMonitor(configurationService, RazorLSPOptions.Default);

CompletionEndpoint = new RazorCompletionEndpoint(completionListProvider, telemetryReporter: null, optionsMonitor);
CompletionEndpoint = new RazorCompletionEndpoint(completionListProvider, completionTriggerAndCommitCharacters, telemetryReporter: null, optionsMonitor);

var clientCapabilities = new VSInternalClientCapabilities
{
Expand Down Expand Up @@ -141,8 +141,12 @@ public async Task RazorCompletionAsync()

private class TestDelegatedCompletionListProvider : DelegatedCompletionListProvider
{
public TestDelegatedCompletionListProvider(IDocumentMappingService documentMappingService, IClientConnection clientConnection, CompletionListCache completionListCache)
: base(documentMappingService, clientConnection, completionListCache)
public TestDelegatedCompletionListProvider(
IDocumentMappingService documentMappingService,
IClientConnection clientConnection,
CompletionListCache completionListCache,
CompletionTriggerAndCommitCharacters completionTriggerAndCommitCharacters)
: base(documentMappingService, clientConnection, completionListCache, completionTriggerAndCommitCharacters)
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,22 @@ internal class DelegatedCompletionListProvider
private readonly IDocumentMappingService _documentMappingService;
private readonly IClientConnection _clientConnection;
private readonly CompletionListCache _completionListCache;
private readonly CompletionTriggerAndCommitCharacters _completionTriggerAndCommitCharacters;

public DelegatedCompletionListProvider(
IDocumentMappingService documentMappingService,
IClientConnection clientConnection,
CompletionListCache completionListCache)
CompletionListCache completionListCache,
CompletionTriggerAndCommitCharacters completionTriggerAndCommitCharacters)
{
_documentMappingService = documentMappingService;
_clientConnection = clientConnection;
_completionListCache = completionListCache;
_completionTriggerAndCommitCharacters = completionTriggerAndCommitCharacters;
}

// virtual for tests
public virtual FrozenSet<string> TriggerCharacters => CompletionTriggerAndCommitCharacters.AllDelegationTriggerCharacters;
public virtual FrozenSet<string> TriggerCharacters => _completionTriggerAndCommitCharacters.AllDelegationTriggerCharacters;

// virtual for tests
public virtual async Task<VSInternalCompletionList?> GetCompletionListAsync(
Expand Down Expand Up @@ -74,7 +77,12 @@ public DelegatedCompletionListProvider(
positionInfo = provisionalCompletionValue.DocumentPositionInfo;
}

completionContext = DelegatedCompletionHelper.RewriteContext(completionContext, positionInfo.LanguageKind);
if (DelegatedCompletionHelper.RewriteContext(completionContext, positionInfo.LanguageKind, _completionTriggerAndCommitCharacters) is not { } rewrittenContext)
{
return null;
}

completionContext = rewrittenContext;

var razorCodeDocument = await documentContext.GetCodeDocumentAsync(cancellationToken).ConfigureAwait(false);
// It's a bit confusing, but we have two different "add snippets" options - one is a part of
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.AspNetCore.Razor.LanguageServer.EndpointContracts;
using Microsoft.AspNetCore.Razor.Telemetry;
using Microsoft.CodeAnalysis.Razor.Completion;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.CodeAnalysis.Razor.Workspaces.Telemetry;
using Microsoft.CodeAnalysis.Text;
using Microsoft.VisualStudio.LanguageServer.Protocol;
Expand All @@ -18,11 +19,13 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer.Completion;
[RazorLanguageServerEndpoint(Methods.TextDocumentCompletionName)]
internal class RazorCompletionEndpoint(
CompletionListProvider completionListProvider,
CompletionTriggerAndCommitCharacters completionTriggerAndCommitCharacters,
ITelemetryReporter? telemetryReporter,
RazorLSPOptionsMonitor optionsMonitor)
: IRazorRequestHandler<CompletionParams, VSInternalCompletionList?>, ICapabilitiesProvider
{
private readonly CompletionListProvider _completionListProvider = completionListProvider;
private readonly CompletionTriggerAndCommitCharacters _completionTriggerAndCommitCharacters = completionTriggerAndCommitCharacters;
private readonly ITelemetryReporter? _telemetryReporter = telemetryReporter;
private readonly RazorLSPOptionsMonitor _optionsMonitor = optionsMonitor;

Expand All @@ -37,7 +40,7 @@ public void ApplyCapabilities(VSInternalServerCapabilities serverCapabilities, V
serverCapabilities.CompletionProvider = new CompletionOptions()
{
ResolveProvider = true,
TriggerCharacters = CompletionTriggerAndCommitCharacters.AllTriggerCharacters,
TriggerCharacters = _completionTriggerAndCommitCharacters.AllTriggerCharacters,
AllCommitCharacters = CompletionTriggerAndCommitCharacters.AllCommitCharacters
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,6 @@ internal class DefaultLanguageServerFeatureOptions : LanguageServerFeatureOption
public override bool UseNewFormattingEngine => false;

public override bool SupportsSoftSelectionInCompletion => true;

public override bool UseVsCodeCompletionTriggerCharacters => false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ public static void AddCompletionServices(this IServiceCollection services)
services.AddSingleton<CompletionListProvider>();
services.AddSingleton<DelegatedCompletionListProvider>();
services.AddSingleton<RazorCompletionListProvider>();
services.AddSingleton<CompletionTriggerAndCommitCharacters>();

services.AddSingleton<AggregateCompletionItemResolver>();
services.AddSingleton<CompletionItemResolver, RazorCompletionItemResolver>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ internal class ConfigurableLanguageServerFeatureOptions : LanguageServerFeatureO
public override bool ForceRuntimeCodeGeneration => _forceRuntimeCodeGeneration ?? _defaults.ForceRuntimeCodeGeneration;
public override bool UseNewFormattingEngine => _useNewFormattingEngine ?? _defaults.UseNewFormattingEngine;
public override bool SupportsSoftSelectionInCompletion => false;
public override bool UseVsCodeCompletionTriggerCharacters => true;

public ConfigurableLanguageServerFeatureOptions(string[] args)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,33 @@

using System.Collections.Frozen;
using System.Linq;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.VisualStudio.LanguageServer.Protocol;

namespace Microsoft.CodeAnalysis.Razor.Completion;

internal static class CompletionTriggerAndCommitCharacters
internal class CompletionTriggerAndCommitCharacters(LanguageServerFeatureOptions languageServerFeatureOptions)
{
private readonly LanguageServerFeatureOptions _languageServerFeatureOptions = languageServerFeatureOptions;

private static readonly FrozenSet<string> s_vsHtmlTriggerCharacters = new[] { ":", "@", "#", ".", "!", "*", ",", "(", "[", "-", "<", "&", "\\", "/", "'", "\"", "=", ":", " ", "`" }.ToFrozenSet();
private static readonly FrozenSet<string> s_vsCodeHtmlTriggerCharacters = new[] { "@", "#", ".", "!", ",", "-", "<", }.ToFrozenSet();
private FrozenSet<string>? _allDelegationTriggerCharacters;
private string[]? _allTriggerCharacters;

public static FrozenSet<string> RazorTriggerCharacters { get; } = new[] { "@", "<", ":", " " }.ToFrozenSet();
/// <summary>
/// Tigger characters that can trigger both Razor and Delegation completion (e.g."@" triggers Razor directives and C#)
/// </summary>
public static FrozenSet<string> RazorDelegationTriggerCharacters { get; } = new[] { "@" }.ToFrozenSet();
public static FrozenSet<string> CSharpTriggerCharacters { get; } = new[] { " ", "(", "=", "#", ".", "<", "[", "{", "\"", "/", ":", "~" }.ToFrozenSet();
public static FrozenSet<string> HtmlTriggerCharacters { get; } = new[] { ":", "@", "#", ".", "!", "*", ",", "(", "[", "-", "<", "&", "\\", "/", "'", "\"", "=", ":", " ", "`" }.ToFrozenSet();
public static FrozenSet<string> AllDelegationTriggerCharacters { get; } = RazorDelegationTriggerCharacters.Union(CSharpTriggerCharacters).Union(HtmlTriggerCharacters).ToFrozenSet();
public static string[] AllTriggerCharacters { get; } = RazorTriggerCharacters.Union(AllDelegationTriggerCharacters).ToArray();
public FrozenSet<string> HtmlTriggerCharacters =>
_languageServerFeatureOptions.UseVsCodeCompletionTriggerCharacters ? s_vsCodeHtmlTriggerCharacters : s_vsHtmlTriggerCharacters;

public FrozenSet<string> AllDelegationTriggerCharacters => _allDelegationTriggerCharacters
??= RazorDelegationTriggerCharacters.Union(CSharpTriggerCharacters).Union(HtmlTriggerCharacters).ToFrozenSet();

public string[] AllTriggerCharacters => _allTriggerCharacters ??= [.. RazorTriggerCharacters.Union(AllDelegationTriggerCharacters)];

/// <summary>
/// This is the intersection of C# and HTML commit characters.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,22 @@ internal static class DelegatedCompletionHelper
[new SnippetResponseRewriter(), new TextEditResponseRewriter(), new DesignTimeHelperResponseRewriter()];

// Currently we only have one HTML response re-writer. Should we ever need more, we can create a common base and a collection
private static readonly HtmlCommitCharacterResponseRewriter s_delegatedHtmlCompletionResponseRewriter = new HtmlCommitCharacterResponseRewriter();
private static readonly HtmlCommitCharacterResponseRewriter s_delegatedHtmlCompletionResponseRewriter = new();

/// <summary>
/// Modifies completion context if needed so that it's acceptable to the delegated language.
/// </summary>
/// <param name="context">Original completion context passed to the completion handler</param>
/// <param name="languageKind">Language of the completion position</param>
/// <param name="completionTriggerAndCommitCharacters">Per-client set of trigger and commit characters</param>
/// <returns>Possibly modified completion context</returns>
/// <remarks>For example, if we invoke C# completion in Razor via @ character, we will not
/// want C# to see @ as the trigger character and instead will transform completion context
/// into "invoked" and "explicit" rather than "typing", without a trigger character</remarks>
public static VSInternalCompletionContext RewriteContext(VSInternalCompletionContext context, RazorLanguageKind languageKind)
public static VSInternalCompletionContext? RewriteContext(
VSInternalCompletionContext context,
RazorLanguageKind languageKind,
CompletionTriggerAndCommitCharacters completionTriggerAndCommitCharacters)
{
Debug.Assert(languageKind != RazorLanguageKind.Razor,
$"{nameof(RewriteContext)} should be called for delegated completion only");
Expand All @@ -60,11 +64,12 @@ public static VSInternalCompletionContext RewriteContext(VSInternalCompletionCon
return context;
}

if (languageKind == RazorLanguageKind.Html
&& CompletionTriggerAndCommitCharacters.HtmlTriggerCharacters.Contains(triggerCharacter))
if (languageKind == RazorLanguageKind.Html)
{
// HTML trigger character for HTML content
return context;
// For HTML we don't want to delegate to HTML language server is completion is due to a trigger characters that is not
// HTML trigger character. Doing so causes bad side effects in VSCode HTML client as we will end up with non-matching
// completion entries
return completionTriggerAndCommitCharacters.HtmlTriggerCharacters.Contains(triggerCharacter) ? context : null;
}

// Trigger character not associated with the current language. Transform the context into an invoked context.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ internal static bool TryConvert(
}

var tagHelperCompletionItemKind = CompletionItemKind.TypeParameter;
var supportedItemKinds = clientCapabilities.TextDocument?.Completion?.CompletionItemKind?.ValueSet ?? Array.Empty<CompletionItemKind>();
var supportedItemKinds = clientCapabilities.TextDocument?.Completion?.CompletionItemKind?.ValueSet ?? [];
if (supportedItemKinds?.Contains(CompletionItemKind.TagHelper) == true)
{
tagHelperCompletionItemKind = CompletionItemKind.TagHelper;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,9 @@ internal abstract class LanguageServerFeatureOptions
/// character with a soft-selected item will not commit that item.
/// </summary>
public abstract bool SupportsSoftSelectionInCompletion { get; }

/// <summary>
/// Indicates that VSCode-compatible completion trigger character set should be used
/// </summary>
public abstract bool UseVsCodeCompletionTriggerCharacters { get; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ internal struct RemoteClientInitializationOptions
[JsonPropertyName("useNewFormattingEngine")]
public required bool UseNewFormattingEngine { get; set; }

[JsonPropertyName("avoidExplicitCommitCharacters")]
[JsonPropertyName("supportsSoftSelectionInCompletion")]
public required bool SupportsSoftSelectionInCompletion { get; set; }

[JsonPropertyName("useVSCodeCompletionTriggerCharacters")]
public bool UseVsCodeCompletionTriggerCharacters { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,6 @@ internal class RemoteLanguageServerFeatureOptions : LanguageServerFeatureOptions
public override bool UseNewFormattingEngine => _options.UseNewFormattingEngine;

public override bool SupportsSoftSelectionInCompletion => _options.SupportsSoftSelectionInCompletion;

public override bool UseVsCodeCompletionTriggerCharacters => _options.UseVsCodeCompletionTriggerCharacters;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT license. See License.txt in the project root for license information.

using System.ComponentModel.Composition;
using Microsoft.CodeAnalysis.Razor.Completion;
using Microsoft.CodeAnalysis.Razor.Workspaces;

namespace Microsoft.VisualStudio.Razor.LanguageClient.Cohost;

[Export(typeof(CompletionTriggerAndCommitCharacters))]
[method: ImportingConstructor]
internal sealed class CohostCompletionTriggerAndCommitCharacters(LanguageServerFeatureOptions languageServerFeatureOptions) : CompletionTriggerAndCommitCharacters(languageServerFeatureOptions);
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ internal sealed class CohostDocumentCompletionEndpoint(
IClientSettingsManager clientSettingsManager,
IHtmlDocumentSynchronizer htmlDocumentSynchronizer,
SnippetCompletionItemProvider snippetCompletionItemProvider,
CompletionTriggerAndCommitCharacters completionTriggerAndCommitCharacters,
LSPRequestInvoker requestInvoker,
ILoggerFactory loggerFactory)
: AbstractRazorCohostDocumentRequestHandler<RoslynCompletionParams, VSInternalCompletionList?>, IDynamicRegistrationProvider
Expand All @@ -48,6 +49,7 @@ internal sealed class CohostDocumentCompletionEndpoint(
private readonly IClientSettingsManager _clientSettingsManager = clientSettingsManager;
private readonly IHtmlDocumentSynchronizer _htmlDocumentSynchronizer = htmlDocumentSynchronizer;
private readonly SnippetCompletionItemProvider _snippetCompletionItemProvider = snippetCompletionItemProvider;
private readonly CompletionTriggerAndCommitCharacters _completionTriggerAndCommitCharacters = completionTriggerAndCommitCharacters;
private readonly LSPRequestInvoker _requestInvoker = requestInvoker;
private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<CohostDocumentCompletionEndpoint>();

Expand All @@ -65,7 +67,7 @@ public ImmutableArray<Registration> GetRegistrations(VSInternalClientCapabilitie
RegisterOptions = new CompletionRegistrationOptions()
{
ResolveProvider = false, // TODO - change to true when Resolve is implemented
TriggerCharacters = CompletionTriggerAndCommitCharacters.AllTriggerCharacters,
TriggerCharacters = _completionTriggerAndCommitCharacters.AllTriggerCharacters,
AllCommitCharacters = CompletionTriggerAndCommitCharacters.AllCommitCharacters
}
}];
Expand Down Expand Up @@ -116,7 +118,12 @@ public ImmutableArray<Registration> GetRegistrations(VSInternalClientCapabilitie
var documentPositionInfo = completionPositionInfo.DocumentPositionInfo;
if (documentPositionInfo.LanguageKind != RazorLanguageKind.Razor)
{
completionContext = DelegatedCompletionHelper.RewriteContext(completionContext, documentPositionInfo.LanguageKind);
if (DelegatedCompletionHelper.RewriteContext(completionContext, documentPositionInfo.LanguageKind, _completionTriggerAndCommitCharacters) is not { } rewrittenContext)
{
return null;
}

completionContext = rewrittenContext;
}

// First of all, see if we in HTML and get HTML completions before calling OOP to get Razor completions.
Expand All @@ -129,7 +136,7 @@ public ImmutableArray<Registration> GetRegistrations(VSInternalClientCapabilitie
CommitElementsWithSpace: clientSettings.AdvancedSettings.CommitElementsWithSpace);
using var _ = HashSetPool<string>.GetPooledObject(out var existingHtmlCompletions);

if (CompletionTriggerAndCommitCharacters.IsValidTrigger(CompletionTriggerAndCommitCharacters.HtmlTriggerCharacters, completionContext))
if (CompletionTriggerAndCommitCharacters.IsValidTrigger(_completionTriggerAndCommitCharacters.HtmlTriggerCharacters, completionContext))
{
// We can just blindly call HTML LSP because if we are in C#, generated HTML seen by HTML LSP may return
// results we don't want to show. So we want to call HTML LSP only if we know we are in HTML content.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ private async Task InitializeRemoteClientAsync(RazorRemoteHostClient remoteClien
ShowAllCSharpCodeActions = _languageServerFeatureOptions.ShowAllCSharpCodeActions,
UseNewFormattingEngine = _languageServerFeatureOptions.UseNewFormattingEngine,
SupportsSoftSelectionInCompletion = _languageServerFeatureOptions.SupportsSoftSelectionInCompletion,
UseVsCodeCompletionTriggerCharacters = _languageServerFeatureOptions.UseVsCodeCompletionTriggerCharacters,
};

_logger.LogDebug($"First OOP call, so initializing OOP service.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,6 @@ public VisualStudioLanguageServerFeatureOptions(ILspEditorFeatureDetector lspEdi

// VS actually needs explicit commit characters so don't avoid them.
public override bool SupportsSoftSelectionInCompletion => true;

public override bool UseVsCodeCompletionTriggerCharacters => false;
}
Loading
Loading