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

Add option for element completion commit characters #9379

Merged
merged 11 commits into from
Oct 9, 2023
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.LanguageServer.Protocol;
using Microsoft.VisualStudio.LanguageServer.Protocol;

namespace Microsoft.AspNetCore.Razor.LanguageServer.Completion.Delegation;

internal class HtmlCommitCharacterResponseRewriter(RazorLSPOptionsMonitor razorLSPOptionsMonitor) : DelegatedCompletionResponseRewriter
{
private readonly RazorLSPOptionsMonitor _razorLSPOptionsMonitor = razorLSPOptionsMonitor;

public override int Order => ExecutionBehaviorOrder.ChangesCompletionItems;

public override Task<VSInternalCompletionList> RewriteAsync(VSInternalCompletionList completionList, int hostDocumentIndex, DocumentContext hostDocumentContext, DelegatedCompletionParams delegatedParameters, CancellationToken cancellationToken)
{
if (delegatedParameters.ProjectedKind != RazorLanguageKind.Html)
{
return Task.FromResult(completionList);
}

if (_razorLSPOptionsMonitor.CurrentValue.CommitElementsWithSpace)
{
return Task.FromResult(completionList);
}

string[]? itemCommitChars = null;
if (completionList.CommitCharacters is { } commitCharacters)
{
if (commitCharacters.TryGetFirst(out var commitChars))
{
itemCommitChars = commitChars.Where(s => s != " ").ToArray();

// If the default commit characters didn't include " " already, then we set our list to null to avoid over-specifying commit characters
if (itemCommitChars.Length == commitChars.Length)
{
itemCommitChars = null;
}
}
else if (commitCharacters.TryGetSecond(out var vsCommitChars))
{
itemCommitChars = vsCommitChars.Where(s => s.Character != " ").Select(s => s.Character).ToArray();

// If the default commit characters didn't include " " already, then we set our list to null to avoid over-specifying commit characters
if (itemCommitChars.Length == vsCommitChars.Length)
{
itemCommitChars = null;
}
}
}

foreach (var item in completionList.Items)
Copy link
Contributor

Choose a reason for hiding this comment

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

If 'itemCommitChars' is null can we just early return and do nothing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because each item could specify its own commit characters, so we still have to go through them all and update that. If itemCommitChars is null it just means the server didn't specify a default set.

{
if (item.Kind == CompletionItemKind.Element)
{
if (item.CommitCharacters is null)
{
if (itemCommitChars is not null)
{
// This item wants to use the default commit characters, so change it to our updated version of them, without the space
item.CommitCharacters = itemCommitChars;
}
}
else
{
// This item has its own commit characters, so just remove spaces
item.CommitCharacters = item.CommitCharacters?.Where(s => s != " ").ToArray();
}
}
}

return Task.FromResult(completionList);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ internal class TagHelperCompletionProvider : IRazorCompletionItemProvider
internal static readonly IReadOnlyList<RazorCommitCharacter> AttributeSnippetCommitCharacters = RazorCommitCharacter.FromArray(new[] { "=" }, insert: false);

private static readonly IReadOnlyList<RazorCommitCharacter> s_elementCommitCharacters = RazorCommitCharacter.FromArray(new[] { " ", ">" });
private static readonly IReadOnlyList<RazorCommitCharacter> s_elementCommitCharacters_WithoutSpace = RazorCommitCharacter.FromArray(new[] { ">" });
private static readonly IReadOnlyList<RazorCommitCharacter> s_noCommitCharacters = Array.Empty<RazorCommitCharacter>();
private readonly HtmlFactsService _htmlFactsService;
private readonly TagHelperCompletionService _tagHelperCompletionService;
Expand Down Expand Up @@ -110,7 +111,7 @@ public ImmutableArray<RazorCompletionItem> GetCompletionItems(RazorCompletionCon
var stringifiedAttributes = _tagHelperFactsService.StringifyAttributes(attributes);

return GetAttributeCompletions(parent, containingTagNameToken.Content, selectedAttributeName, stringifiedAttributes, context.TagHelperDocumentContext, context.Options);

static bool InOrAtEndOfAttribute(SyntaxNode attributeSyntax, int absoluteIndex)
{
// When we are in the middle of writing an attribute it is treated as a minimilized one, e.g.:
Expand Down Expand Up @@ -248,13 +249,17 @@ private ImmutableArray<RazorCompletionItem> GetElementCompletions(
var completionResult = _tagHelperCompletionService.GetElementCompletions(elementCompletionContext);
using var completionItems = new PooledArrayBuilder<RazorCompletionItem>();

var commitChars = _optionsMonitor.CurrentValue.CommitElementsWithSpace
? s_elementCommitCharacters
: s_elementCommitCharacters_WithoutSpace;

foreach (var (displayText, tagHelpers) in completionResult.Completions)
{
var razorCompletionItem = new RazorCompletionItem(
displayText: displayText,
insertText: displayText,
kind: RazorCompletionItemKind.TagHelperElement,
commitCharacters: s_elementCommitCharacters);
commitCharacters: commitChars);

var tagHelperDescriptions = tagHelpers.SelectAsArray(BoundElementDescriptionInfo.From);
var elementDescription = new AggregateBoundElementDescription(tagHelperDescriptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,23 +94,27 @@ internal RazorLSPOptions BuildOptions(JObject[] result)
}
else
{
ExtractVSCodeOptions(result, out var trace, out var enableFormatting, out var autoClosingTags);
return new RazorLSPOptions(trace, enableFormatting, autoClosingTags, ClientSettings.Default);
ExtractVSCodeOptions(result, out var trace, out var enableFormatting, out var autoClosingTags, out var commitElementsWithSpace);
return new RazorLSPOptions(trace, enableFormatting, autoClosingTags, commitElementsWithSpace, ClientSettings.Default);
}
}

private void ExtractVSCodeOptions(
JObject[] result,
out Trace trace,
out bool enableFormatting,
out bool autoClosingTags)
out bool autoClosingTags,
out bool commitElementsWithSpace)
{
var razor = result[0];
var html = result[1];

trace = RazorLSPOptions.Default.Trace;
enableFormatting = RazorLSPOptions.Default.EnableFormatting;
autoClosingTags = RazorLSPOptions.Default.AutoClosingTags;
// Deliberately not using the "default" here because we want a different default for VS Code, as
// this matches VS Code's html servers commit behaviour
commitElementsWithSpace = false;

if (razor != null)
{
Expand All @@ -127,6 +131,15 @@ private void ExtractVSCodeOptions(
enableFormatting = GetObjectOrDefault(parsedEnableFormatting, enableFormatting);
}
}

if (razor.TryGetValue("completion", out var parsedCompletion))
{
if (parsedCompletion is JObject jObject &&
jObject.TryGetValue("commitElementsWithSpace", out var parsedCommitElementsWithSpace))
{
commitElementsWithSpace = GetObjectOrDefault(parsedCommitElementsWithSpace, commitElementsWithSpace);
}
}
}

if (html != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ public static void AddCompletionServices(this IServiceCollection services, Langu
services.AddSingleton<RazorCompletionListProvider>();
services.AddSingleton<DelegatedCompletionResponseRewriter, TextEditResponseRewriter>();
services.AddSingleton<DelegatedCompletionResponseRewriter, DesignTimeHelperResponseRewriter>();
services.AddSingleton<DelegatedCompletionResponseRewriter, HtmlCommitCharacterResponseRewriter>();

services.AddSingleton<AggregateCompletionItemResolver>();
services.AddSingleton<CompletionItemResolver, RazorCompletionItemResolver>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,15 @@ internal record RazorLSPOptions(
int TabSize,
bool FormatOnType,
bool AutoInsertAttributeQuotes,
bool ColorBackground)
bool ColorBackground,
bool CommitElementsWithSpace)
{
public RazorLSPOptions(Trace trace, bool enableFormatting, bool autoClosingTags, ClientSettings settings)
: this(trace, enableFormatting, autoClosingTags, !settings.ClientSpaceSettings.IndentWithTabs, settings.ClientSpaceSettings.IndentSize, settings.AdvancedSettings.FormatOnType, settings.AdvancedSettings.AutoInsertAttributeQuotes, settings.AdvancedSettings.ColorBackground)
public RazorLSPOptions(Trace trace, bool enableFormatting, bool autoClosingTags, bool commitElementsWithSpace, ClientSettings settings)
: this(trace, enableFormatting, autoClosingTags, !settings.ClientSpaceSettings.IndentWithTabs, settings.ClientSpaceSettings.IndentSize, settings.AdvancedSettings.FormatOnType, settings.AdvancedSettings.AutoInsertAttributeQuotes, settings.AdvancedSettings.ColorBackground, commitElementsWithSpace)
{
}

public readonly static RazorLSPOptions Default = new(Trace: default, EnableFormatting: true, AutoClosingTags: true, InsertSpaces: true, TabSize: 4, FormatOnType: true, AutoInsertAttributeQuotes: true, ColorBackground: false);
public readonly static RazorLSPOptions Default = new(Trace: default, EnableFormatting: true, AutoClosingTags: true, InsertSpaces: true, TabSize: 4, FormatOnType: true, AutoInsertAttributeQuotes: true, ColorBackground: false, CommitElementsWithSpace: true);

public LogLevel MinLogLevel => GetLogLevelForTrace(Trace);

Expand All @@ -42,5 +43,6 @@ internal static RazorLSPOptions From(ClientSettings clientSettings)
=> new(Default.Trace,
Default.EnableFormatting,
clientSettings.AdvancedSettings.AutoClosingTags,
clientSettings.AdvancedSettings.CommitElementsWithSpace,
clientSettings);
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ internal sealed record ClientSpaceSettings(bool IndentWithTabs, int IndentSize)
public int IndentSize { get; } = IndentSize >= 0 ? IndentSize : throw new ArgumentOutOfRangeException(nameof(IndentSize));
}

internal sealed record ClientAdvancedSettings(bool FormatOnType, bool AutoClosingTags, bool AutoInsertAttributeQuotes, bool ColorBackground)
internal sealed record ClientAdvancedSettings(bool FormatOnType, bool AutoClosingTags, bool AutoInsertAttributeQuotes, bool ColorBackground, bool CommitElementsWithSpace)
{
public static readonly ClientAdvancedSettings Default = new(FormatOnType: true, AutoClosingTags: true, AutoInsertAttributeQuotes: true, ColorBackground: false);
public static readonly ClientAdvancedSettings Default = new(FormatOnType: true, AutoClosingTags: true, AutoInsertAttributeQuotes: true, ColorBackground: false, CommitElementsWithSpace: true);
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ internal class OptionsStorage : IAdvancedSettingsStorage
private const string AutoClosingTagsName = "AutoClosingTags";
private const string AutoInsertAttributeQuotesName = "AutoInsertAttributeQuotes";
private const string ColorBackgroundName = "ColorBackground";
private const string CommitElementsWithSpaceName = "CommitElementsWithSpace";

public bool FormatOnType
{
Expand All @@ -51,6 +52,12 @@ public bool ColorBackground
set => SetBool(ColorBackgroundName, value);
}

public bool CommitElementsWithSpace
{
get => GetBool(CommitElementsWithSpaceName, defaultValue: true);
set => SetBool(CommitElementsWithSpaceName, value);
}

[ImportingConstructor]
public OptionsStorage(SVsServiceProvider vsServiceProvider, ITelemetryReporter telemetryReporter)
{
Expand All @@ -63,7 +70,7 @@ public OptionsStorage(SVsServiceProvider vsServiceProvider, ITelemetryReporter t

public event EventHandler<ClientAdvancedSettingsChangedEventArgs>? Changed;

public ClientAdvancedSettings GetAdvancedSettings() => new(FormatOnType, AutoClosingTags, AutoInsertAttributeQuotes, ColorBackground);
public ClientAdvancedSettings GetAdvancedSettings() => new(FormatOnType, AutoClosingTags, AutoInsertAttributeQuotes, ColorBackground, CommitElementsWithSpace);

public bool GetBool(string name, bool defaultValue)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ internal class AdvancedOptionPage : DialogPage
private bool? _autoClosingTags;
private bool? _autoInsertAttributeQuotes;
private bool? _colorBackground;
private bool? _commitElementsWithSpace;

public AdvancedOptionPage()
{
Expand Down Expand Up @@ -58,6 +59,15 @@ public bool AutoInsertAttributeQuotes
set => _autoInsertAttributeQuotes = value;
}

[LocCategory(nameof(VSPackage.Completion))]
[LocDescription(nameof(VSPackage.Setting_CommitElementsWithSpaceDescription))]
[LocDisplayName(nameof(VSPackage.Setting_CommitElementsWithSpaceDisplayName))]
public bool CommitElementsWithSpace
{
get => _commitElementsWithSpace ?? _optionsStorage.Value.CommitElementsWithSpace;
set => _commitElementsWithSpace = value;
}

[LocCategory(nameof(VSPackage.Formatting))]
[LocDescription(nameof(VSPackage.Setting_ColorBackgroundDescription))]
[LocDisplayName(nameof(VSPackage.Setting_ColorBackgroundDisplayName))]
Expand Down Expand Up @@ -88,6 +98,11 @@ protected override void OnApply(PageApplyEventArgs e)
{
_optionsStorage.Value.ColorBackground = _colorBackground.Value;
}

if (_commitElementsWithSpace is not null)
{
_optionsStorage.Value.CommitElementsWithSpace = _commitElementsWithSpace.Value;
}
}

protected override void OnClosed(EventArgs e)
Expand All @@ -96,5 +111,6 @@ protected override void OnClosed(EventArgs e)
_autoClosingTags = null;
_autoInsertAttributeQuotes = null;
_colorBackground = null;
_commitElementsWithSpace = null;
}
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema

<!--
Microsoft ResX Schema
Version 2.0

The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
associated with the data types.

Example:

... ado.net/XML headers & schema ...
<resheader name="resmimetype">text/microsoft-resx</resheader>
<resheader name="version">2.0</resheader>
Expand All @@ -26,36 +26,36 @@
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
<comment>This is a comment</comment>
</data>

There are any number of "resheader" rows that contain simple
There are any number of "resheader" rows that contain simple
name/value pairs.

Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
mimetype set.

The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
extensible. For a given mimetype the value must be set accordingly:

Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
read any of the formats listed below.

mimetype: application/x-microsoft.net.object.binary.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
: and then encoded with base64 encoding.

mimetype: application/x-microsoft.net.object.soap.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Soap.SoapFormatter
: and then encoded with base64 encoding.

mimetype: application/x-microsoft.net.object.bytearray.base64
value : The object must be serialized into a byte array
value : The object must be serialized into a byte array
: using a System.ComponentModel.TypeConverter
: and then encoded with base64 encoding.
-->
Expand Down Expand Up @@ -165,6 +165,12 @@
<data name="Setting_ColorBackgroundDisplayName" xml:space="preserve">
<value>Background for C# Code</value>
</data>
<data name="Setting_CommitElementsWithSpaceDescription" xml:space="preserve">
<value>If true, pressing space will commit suggestions for Html elements and components</value>
</data>
<data name="Setting_CommitElementsWithSpaceDisplayName" xml:space="preserve">
<value>Commit Elements with Space</value>
</data>
<data name="Setting_FormattingOnTypeDescription" xml:space="preserve">
<value>If true, formatting will be enabled while typing</value>
</data>
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading