From e0f9c750803d6da7e4befd3e4070910cb684b2b1 Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Tue, 9 Mar 2021 20:44:19 -0800 Subject: [PATCH 01/21] Preliminary changes --- .../RazorContentTypeChangeListener.cs | 25 ------------------- .../RazorPackage.cs | 2 +- .../VSPackage.resx | 2 +- 3 files changed, 2 insertions(+), 27 deletions(-) diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorContentTypeChangeListener.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorContentTypeChangeListener.cs index 6522387d6f4..0f6fab7a2ce 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorContentTypeChangeListener.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorContentTypeChangeListener.cs @@ -103,11 +103,6 @@ internal void RazorBufferCreated(ITextBuffer textBuffer) throw new ArgumentNullException(nameof(textBuffer)); } - // Initialize the buffer with editor options. - // Temporary: Ideally in remote scenarios, we should be using host's settings. - // But we need this until that support is built. - InitializeOptions(textBuffer); - if (!_lspEditorFeatureDetector.IsRemoteClient()) { // Renames on open files don't dispose buffer state so we need to separately monitor the buffer for document renames to ensure @@ -177,25 +172,5 @@ private void StopMonitoringDocumentForRenames(ITextBuffer textBuffer) textDocument.FileActionOccurred -= TextDocument_FileActionOccurred; } - - private void InitializeOptions(ITextBuffer textBuffer) - { - // Ideally we would initialize options based on Razor specific options in the context menu. - // But since we don't have support for that yet, we will temporarily use the settings from Html. - - var textManager = _serviceProvider.GetService(typeof(SVsTextManager)) as IVsTextManager2; - Assumes.Present(textManager); - - var langPrefs2 = new LANGPREFERENCES2[] { new LANGPREFERENCES2() { guidLang = HtmlLanguageServiceGuid } }; - if (VSConstants.S_OK == textManager.GetUserPreferences2(null, null, langPrefs2, null)) - { - var insertSpaces = langPrefs2[0].fInsertTabs == 0; - var tabSize = langPrefs2[0].uTabSize; - - var razorOptions = _editorOptionsFactory.GetOptions(textBuffer); - razorOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, insertSpaces); - razorOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, (int)tabSize); - } - } } } diff --git a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/RazorPackage.cs b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/RazorPackage.cs index 17b8d2fc0ef..d20bd3a826d 100644 --- a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/RazorPackage.cs +++ b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/RazorPackage.cs @@ -18,7 +18,7 @@ namespace Microsoft.VisualStudio.RazorExtension { [PackageRegistration(UseManagedResourcesOnly = true)] - [AboutDialogInfo(PackageGuidString, "ASP.NET Core Razor Language Services", "#110", "#112", IconResourceID = "#400")] + [AboutDialogInfo(PackageGuidString, "Razor (Preview)", "#110", "#112", IconResourceID = "#400")] [ProvideService(typeof(RazorLanguageService))] [ProvideLanguageService(typeof(RazorLanguageService), RazorLSPConstants.RazorLSPContentTypeName, 110)] [ProvideBrokeredServiceHubService("Microsoft.VisualStudio.Razor.TagHelperProvider", Audience = Shell.ServiceBroker.ServiceAudience.Local)] diff --git a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/VSPackage.resx b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/VSPackage.resx index f46d3bd27d5..0f57402b503 100644 --- a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/VSPackage.resx +++ b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/VSPackage.resx @@ -118,7 +118,7 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - ASP.NET Core Razor Language Services + Razor (Preview) Provides languages services for ASP.NET Core Razor. From c9ab4e1184029606f0763f98416eea63d9b80a5b Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Fri, 12 Mar 2021 13:41:52 -0800 Subject: [PATCH 02/21] Further updates --- global.json | 6 ++--- .../Formatting/CSharpFormatter.cs | 19 +++++++++++----- ....VisualStudio.RazorExtension.Custom.pkgdef | 22 ++++++++++++++++++- .../RazorPackage.cs | 2 +- .../VSPackage.resx | 2 +- 5 files changed, 40 insertions(+), 11 deletions(-) diff --git a/global.json b/global.json index 6fb2fb1af06..173138882b1 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "tools": { - "dotnet": "6.0.100-preview.1.21103.13", + "dotnet": "5.0.100", "runtimes": { "dotnet": [ "2.1.11", @@ -15,10 +15,10 @@ } }, "sdk": { - "version": "6.0.100-preview.1.21103.13" + "version": "5.0.100" }, "msbuild-sdks": { - "Microsoft.DotNet.Arcade.Sdk": "6.0.0-beta.21152.1", + "Microsoft.DotNet.Arcade.Sdk": "6.0.0-beta.21125.5", "Yarn.MSBuild": "1.22.5" } } diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormatter.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormatter.cs index 90e491be1d4..7062e4c8f19 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormatter.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Formatting/CSharpFormatter.cs @@ -179,12 +179,16 @@ private async Task> GetCSharpIndentationCoreAsync(Formattin // Assuming the C# formatter did the right thing, let's extract the indentation offset from // the line containing trivia and token that has our attached annotations. - ExtractTriviaAnnotations(); - ExtractTokenAnnotations(); + ExtractTriviaAnnotations(context, formattedRoot, formattedText, desiredIndentationMap); + ExtractTokenAnnotations(context, formattedRoot, formattedText, indentationMap, desiredIndentationMap); return desiredIndentationMap; - void ExtractTriviaAnnotations() + static void ExtractTriviaAnnotations( + FormattingContext context, + SyntaxNode formattedRoot, + SourceText formattedText, + Dictionary desiredIndentationMap) { var formattedTriviaList = formattedRoot.GetAnnotatedTrivia(MarkerId); foreach (var trivia in formattedTriviaList) @@ -205,7 +209,12 @@ void ExtractTriviaAnnotations() } } - void ExtractTokenAnnotations() + static void ExtractTokenAnnotations( + FormattingContext context, + SyntaxNode formattedRoot, + SourceText formattedText, + Dictionary indentationMap, + Dictionary desiredIndentationMap) { var formattedTokenList = formattedRoot.GetAnnotatedTokens(MarkerId); foreach (var token in formattedTokenList) @@ -317,7 +326,7 @@ private SyntaxNode AttachAnnotations( return root; } - private int GetIndentationOffsetFromLine(FormattingContext context, TextLine line) + private static int GetIndentationOffsetFromLine(FormattingContext context, TextLine line) { var offset = line.GetFirstNonWhitespaceOffset() ?? 0; if (!context.Options.InsertSpaces) diff --git a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/Microsoft.VisualStudio.RazorExtension.Custom.pkgdef b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/Microsoft.VisualStudio.RazorExtension.Custom.pkgdef index 0bfba462982..d72934c9e04 100644 --- a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/Microsoft.VisualStudio.RazorExtension.Custom.pkgdef +++ b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/Microsoft.VisualStudio.RazorExtension.Custom.pkgdef @@ -15,4 +15,24 @@ "Description"="Enables the Razor Language Server Protocol powered editor experience for all Razor editors." "Value"=dword:00000000 "Title"="Enable experimental Razor editor (requires restart)" -"PreviewPaneChannels"="*" \ No newline at end of file +"PreviewPaneChannels"="*" + +// Sets up Razor's default settings in Tools->Options. +// Razor's default settings can be found in the VS repo at: +// src/VSCommonContent/profiles/General.vssettings +[$RootKey$\AutomationProperties\TextEditor\RazorLSP] +@="#110" +"Description"="#112" +"Name"="RazorLSP" +"Package"="{f5e7e720-1401-11d1-883b-0000f87579d2}" +"ResourcePackage"="{13b72f58-279e-49e0-a56d-296be02f0805}" +"ProfileSave"=dword:00000001 +"VSSettingsMigration"=dword:00000001 + +// Provides additional Tools->Options settings not covered +// by the settings in General.vssettings. +[$RootKey$\Languages\Language Services\RazorLSP] +@="{4513FA64-5B72-4B58-9D4C-1D3C81996C2C}" +"Package"="{13b72f58-279e-49e0-a56d-296be02f0805}" +"ShowBraceCompletion"=dword:00000001 +"ShowSmartIndent"=dword:00000001 \ No newline at end of file diff --git a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/RazorPackage.cs b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/RazorPackage.cs index d20bd3a826d..abde8a9968d 100644 --- a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/RazorPackage.cs +++ b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/RazorPackage.cs @@ -18,7 +18,7 @@ namespace Microsoft.VisualStudio.RazorExtension { [PackageRegistration(UseManagedResourcesOnly = true)] - [AboutDialogInfo(PackageGuidString, "Razor (Preview)", "#110", "#112", IconResourceID = "#400")] + [AboutDialogInfo(PackageGuidString, "Razor", "#110", "#112", IconResourceID = "#400")] [ProvideService(typeof(RazorLanguageService))] [ProvideLanguageService(typeof(RazorLanguageService), RazorLSPConstants.RazorLSPContentTypeName, 110)] [ProvideBrokeredServiceHubService("Microsoft.VisualStudio.Razor.TagHelperProvider", Audience = Shell.ServiceBroker.ServiceAudience.Local)] diff --git a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/VSPackage.resx b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/VSPackage.resx index 0f57402b503..37e4d322197 100644 --- a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/VSPackage.resx +++ b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/VSPackage.resx @@ -118,7 +118,7 @@ System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089 - Razor (Preview) + ASP.NET Core Razor Provides languages services for ASP.NET Core Razor. From 8a080bcec2685b6f80f2feb850d9ab042057e23f Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Mon, 15 Mar 2021 14:34:40 -0700 Subject: [PATCH 03/21] revert changes to global.json --- NuGet.config | 2 ++ eng/Versions.props | 32 ++++++++++++++++---------------- global.json | 6 +++--- 3 files changed, 21 insertions(+), 19 deletions(-) diff --git a/NuGet.config b/NuGet.config index bf60a0b1df7..490fddd0514 100644 --- a/NuGet.config +++ b/NuGet.config @@ -12,6 +12,8 @@ + + diff --git a/eng/Versions.props b/eng/Versions.props index 74b0ca28214..fc8d2e0f116 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -48,24 +48,24 @@ --> - 6.0.0-preview.3.21165.4 - 6.0.0-preview.3.21165.4 - 6.0.0-preview.3.21165.4 - 6.0.0-preview.3.21165.4 - 6.0.0-preview.3.21165.4 - 6.0.0-preview.3.21165.4 - 6.0.0-preview.3.21165.4 - 6.0.0-preview.3.21164.8 - 6.0.0-preview.3.21164.8 + 6.0.0-dev + 6.0.0-dev + 6.0.0-dev + 6.0.0-dev + 6.0.0-dev + 6.0.0-dev + 6.0.0-dev + 6.0.0-preview.3.21159.2 + 6.0.0-preview.3.21159.2 6.0.0-preview.3.21164.8 - 6.0.0-preview.3.21164.8 - 6.0.0-preview.3.21164.8 - 6.0.0-preview.3.21164.8 - 6.0.0-preview.3.21164.8 + 6.0.0-preview.3.21159.2 + 6.0.0-preview.3.21159.2 + 6.0.0-preview.3.21159.2 + 6.0.0-preview.3.21159.2 6.0.0-alpha.1.21072.5 - 6.0.0-preview.3.21164.8 - 6.0.0-preview.3.21164.8 - 6.0.0-preview.3.21164.8 + 6.0.0-preview.3.21159.2 + 6.0.0-preview.3.21159.2 + 6.0.0-preview.3.21159.2 - - diff --git a/eng/Versions.props b/eng/Versions.props index 632d527e0e2..dc0d5bfc34d 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -44,17 +44,17 @@ DO NOT UPDATE THESE MANUALLY. Use the `darc` command line tool to update this file so it stays in sync with Version.Details.xml. - See https://github.com/dotnet/arcade/blob/master/Documentation/Darc.md for instructions on using darc. + See https://github.com/dotnet/arcade/blob/main/Documentation/Darc.md for instructions on using darc. --> - 6.0.0-dev - 6.0.0-dev - 6.0.0-dev - 6.0.0-dev - 6.0.0-dev - 6.0.0-dev - 6.0.0-dev + 6.0.0-preview.3.21166.12 + 6.0.0-preview.3.21166.12 + 6.0.0-preview.3.21166.12 + 6.0.0-preview.3.21166.12 + 6.0.0-preview.3.21166.12 + 6.0.0-preview.3.21166.12 + 6.0.0-preview.3.21166.12 6.0.0-preview.3.21164.8 6.0.0-preview.3.21164.8 6.0.0-preview.3.21164.8 @@ -88,7 +88,7 @@ 16.10.57-preview1 1.0.1-beta1.21103.2 - 16.9.30921.310 + 16.9.30927.25 5.0.0-preview.4.20205.1 @@ -103,17 +103,16 @@ $(Tooling_MicrosoftCodeAnalysisTestingVersion) $(Tooling_MicrosoftCodeAnalysisTestingVersion) 3.9.0-2.20573.10 - 2.7.339 + 2.7.454 16.10.8 - 16.9.184-preview + 16.9.214 $(MicrosoftVisualStudioShellPackagesVersion) - 16.8.272 + 16.9.214 16.10.8 16.10.8 - 16.9.150 + 16.10.138 16.7.30204.53-pre 0.3.1074 - 16.9.1043 7.10.6071 16.8.1-beta1-10222-04 16.10.81-pre @@ -167,4 +166,4 @@ 1.4.1 2.4.1 - + \ No newline at end of file diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs index 88d84532ccb..d1c788f8c38 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs @@ -2,11 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.IO; -using System.Text; using System.Threading; using System.Threading.Tasks; -using Microsoft.Extensions.Configuration; using Microsoft.Extensions.Logging; using Newtonsoft.Json.Linq; using OmniSharp.Extensions.LanguageServer.Protocol.Models; @@ -60,7 +57,7 @@ public async override Task GetLatestOptionsAsync(CancellationTo var response = await _server.SendRequestAsync("workspace/configuration", request); var result = await response.Returning(cancellationToken); - // Spec indicates result should be the same length as the number of ConfigurationItems we pass in. + // LSP spec indicates result should be the same length as the number of ConfigurationItems we pass in. if (result == null || result.Length < 3 || result[0] == null) { _logger.LogWarning("Client failed to provide the expected configuration."); diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs index 33254d9f5b2..892514d38b6 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs @@ -14,7 +14,6 @@ using Microsoft.VisualStudio.LanguageServer.Protocol; using Microsoft.VisualStudio.LanguageServerClient.Razor.HtmlCSharp; using Microsoft.VisualStudio.Threading; -using Newtonsoft.Json; using Newtonsoft.Json.Linq; using SemanticTokens = OmniSharp.Extensions.LanguageServer.Protocol.Models.Proposals.SemanticTokens; using Task = System.Threading.Tasks.Task; @@ -331,6 +330,8 @@ public override Task UpdateConfigurationAsync( var index = 0; foreach (var item in configParams.Items) { + // Right now in VS we only care about editor settings, but we should update this logic later if + // we want to support Razor and HTML settings as well. result[index] = item.Section == "editor" ? JObject.FromObject(new JsonEditorSettings { InsertSpaces = insertSpaces, TabSize = tabSize }) : new JObject(); diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs index c2a5fa59da5..0fbd35b404b 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs @@ -68,6 +68,11 @@ public RazorLSPTextViewConnectionListener( throw new ArgumentNullException(nameof(requestInvoker)); } + if (clientOptionsMonitor is null) + { + throw new ArgumentNullException(nameof(clientOptionsMonitor)); + } + if (serviceProvider is null) { throw new ArgumentNullException(nameof(serviceProvider)); @@ -102,6 +107,7 @@ public void SubjectBuffersConnected(ITextView textView, ConnectionReason reason, RazorLSPTextViewFilter.CreateAndRegister(vsTextView); + // Initialize the user's options and start listening for changes. _razorEditorOptions = _editorOptionsFactory.GetOptions(textView); Assumes.Present(_razorEditorOptions); RazorOptions_OptionChanged(null, null); @@ -117,13 +123,21 @@ public void SubjectBuffersDisconnected(ITextView textView, ConnectionReason reas private async void RazorOptions_OptionChanged(object sender, EditorOptionChangedEventArgs e) #pragma warning restore VSTHRD100 // Avoid async void methods { + // Retrieve current space/tabs settings from from Tools->Options. var (insertSpaces, tabSize) = GetRazorEditorOptions(_textManager); + // Update settings in the actual editor. _razorEditorOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, insertSpaces); _razorEditorOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, tabSize); + // Keep track of accurate settings on the client side. This is so we can easily retrieve the + // options later when the server sends us a workspace/configuration request. _clientOptionsMonitor.UpdateOptions(insertSpaces, tabSize); + // Make sure the server updates the settings on their side by sending a + // workspace/didChangeConfiguration request. This notifies the server that the user's + // settings have changed. The server will then query the client's options monitor (already + // updated via the line above) by sending a workspace/configuration request. await _requestInvoker.ReinvokeRequestOnServerAsync( Methods.WorkspaceDidChangeConfigurationName, RazorLSPConstants.RazorLSPContentTypeName, @@ -131,7 +145,7 @@ await _requestInvoker.ReinvokeRequestOnServerAsync UpdateConfigurationAsync(OmniSharp.Extensions.LanguageServer.Protocol.Models.ConfigurationParams configParams, CancellationToken cancellationToken); diff --git a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/AssemblyBindingRedirects.cs b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/AssemblyBindingRedirects.cs index 2ba1bc3f160..9701c28be13 100644 --- a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/AssemblyBindingRedirects.cs +++ b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/AssemblyBindingRedirects.cs @@ -8,36 +8,36 @@ GenerateCodeBase = true, PublicKeyToken = "adb9793829ddae60", OldVersionLowerBound = "0.0.0.0", - OldVersionUpperBound = "42.42.42.42", - NewVersion = "42.42.42.42")] + OldVersionUpperBound = "6.0.0.0", + NewVersion = "6.0.0.0")] [assembly: ProvideBindingRedirection( AssemblyName = "Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X", GenerateCodeBase = true, PublicKeyToken = "adb9793829ddae60", OldVersionLowerBound = "0.0.0.0", - OldVersionUpperBound = "42.42.42.42", - NewVersion = "42.42.42.42")] + OldVersionUpperBound = "6.0.0.0", + NewVersion = "6.0.0.0")] [assembly: ProvideBindingRedirection( AssemblyName = "Microsoft.AspNetCore.Mvc.Razor.Extensions.Version2_X", GenerateCodeBase = true, PublicKeyToken = "adb9793829ddae60", OldVersionLowerBound = "0.0.0.0", - OldVersionUpperBound = "42.42.42.42", - NewVersion = "42.42.42.42")] + OldVersionUpperBound = "6.0.0.0", + NewVersion = "6.0.0.0")] [assembly: ProvideBindingRedirection( AssemblyName = "Microsoft.AspNetCore.Razor.Language", GenerateCodeBase = true, PublicKeyToken = "adb9793829ddae60", OldVersionLowerBound = "0.0.0.0", - OldVersionUpperBound = "42.42.42.42", - NewVersion = "42.42.42.42")] + OldVersionUpperBound = "6.0.0.0", + NewVersion = "6.0.0.0")] [assembly: ProvideBindingRedirection( AssemblyName = "Microsoft.CodeAnalysis.Razor", GenerateCodeBase = true, PublicKeyToken = "adb9793829ddae60", OldVersionLowerBound = "0.0.0.0", - OldVersionUpperBound = "42.42.42.42", - NewVersion = "42.42.42.42")] + OldVersionUpperBound = "6.0.0.0", + NewVersion = "6.0.0.0")] [assembly: ProvideBindingRedirection( AssemblyName = "Microsoft.Extensions.Configuration.Abstractions", GenerateCodeBase = true, diff --git a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/RazorPackage.cs b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/RazorPackage.cs index abde8a9968d..8bd2f50ee9d 100644 --- a/src/Razor/src/Microsoft.VisualStudio.RazorExtension/RazorPackage.cs +++ b/src/Razor/src/Microsoft.VisualStudio.RazorExtension/RazorPackage.cs @@ -18,7 +18,7 @@ namespace Microsoft.VisualStudio.RazorExtension { [PackageRegistration(UseManagedResourcesOnly = true)] - [AboutDialogInfo(PackageGuidString, "Razor", "#110", "#112", IconResourceID = "#400")] + [AboutDialogInfo(PackageGuidString, "Razor (ASP.NET Core)", "#110", "#112", IconResourceID = "#400")] [ProvideService(typeof(RazorLanguageService))] [ProvideLanguageService(typeof(RazorLanguageService), RazorLSPConstants.RazorLSPContentTypeName, 110)] [ProvideBrokeredServiceHubService("Microsoft.VisualStudio.Razor.TagHelperProvider", Audience = Shell.ServiceBroker.ServiceAudience.Local)] From 2091aaa73a34ee7ea6f1df84fce6214782733ff1 Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Wed, 24 Mar 2021 00:54:30 -0700 Subject: [PATCH 11/21] Add options monitor file --- .../RazorLSPClientOptionsMonitor.cs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPClientOptionsMonitor.cs diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPClientOptionsMonitor.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPClientOptionsMonitor.cs new file mode 100644 index 00000000000..72b40eb23c6 --- /dev/null +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPClientOptionsMonitor.cs @@ -0,0 +1,21 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.ComponentModel.Composition; + +namespace Microsoft.VisualStudio.LanguageServerClient.Razor +{ + [Export(typeof(RazorLSPClientOptionsMonitor))] + internal class RazorLSPClientOptionsMonitor + { + public bool InsertSpaces { get; private set; } + + public int TabSize { get; private set; } + + public void UpdateOptions(bool insertSpaces, int tabSize) + { + InsertSpaces = insertSpaces; + TabSize = tabSize; + } + } +} From 0ae1376f95ec143a0da736e642a947e02f5dcb12 Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Wed, 24 Mar 2021 00:56:08 -0700 Subject: [PATCH 12/21] Cleanup --- .../RazorLSPClientOptionsMonitor.cs | 4 ++++ .../RazorLSPTextViewConnectionListener.cs | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPClientOptionsMonitor.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPClientOptionsMonitor.cs index 72b40eb23c6..eea8e3af4ee 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPClientOptionsMonitor.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPClientOptionsMonitor.cs @@ -5,6 +5,10 @@ namespace Microsoft.VisualStudio.LanguageServerClient.Razor { + /// + /// Keeps track of accurate settings on the client side so we can easily retrieve the + /// options later when the server sends us a workspace/configuration request. + /// [Export(typeof(RazorLSPClientOptionsMonitor))] internal class RazorLSPClientOptionsMonitor { diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs index 0fbd35b404b..a2edf5290c3 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs @@ -130,7 +130,7 @@ private async void RazorOptions_OptionChanged(object sender, EditorOptionChanged _razorEditorOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, insertSpaces); _razorEditorOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, tabSize); - // Keep track of accurate settings on the client side. This is so we can easily retrieve the + // Keep track of accurate settings on the client side so we can easily retrieve the // options later when the server sends us a workspace/configuration request. _clientOptionsMonitor.UpdateOptions(insertSpaces, tabSize); From ee2cb61070efc2aadda3a735d5b8f0e695fcb8ab Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Wed, 24 Mar 2021 01:03:20 -0700 Subject: [PATCH 13/21] Cleanup --- eng/Versions.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/Versions.props b/eng/Versions.props index dc0d5bfc34d..521290a4432 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -166,4 +166,4 @@ 1.4.1 2.4.1 - \ No newline at end of file + From 86e82c0c2827e179db580271c14cc8970e26cec6 Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Thu, 25 Mar 2021 16:03:39 -0700 Subject: [PATCH 14/21] Code review feedback --- .../DefaultRazorConfigurationService.cs | 113 ++++++++++++------ .../RazorLSPOptions.cs | 4 +- ...tRazorLanguageServerCustomMessageTarget.cs | 32 ++--- .../RazorLSPClientOptionsMonitor.cs | 13 +- .../RazorLSPTextViewConnectionListener.cs | 41 +++++-- .../RazorLanguageServerCustomMessageTarget.cs | 5 +- .../DefaultRazorConfigurationServiceTest.cs | 91 ++++++++++++-- ...orLanguageServerCustomMessageTargetTest.cs | 35 ++++-- 8 files changed, 247 insertions(+), 87 deletions(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs index d1c788f8c38..8c873140d7e 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs @@ -4,6 +4,7 @@ using System; using System.Threading; using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Razor.Editor; using Microsoft.Extensions.Logging; using Newtonsoft.Json.Linq; using OmniSharp.Extensions.LanguageServer.Protocol.Models; @@ -35,24 +36,7 @@ public async override Task GetLatestOptionsAsync(CancellationTo { try { - var request = new ConfigurationParams() - { - Items = new[] - { - new ConfigurationItem() - { - Section = "razor" - }, - new ConfigurationItem() - { - Section = "html" - }, - new ConfigurationItem() - { - Section = "editor" - }, - } - }; + var request = GenerateConfigParams(); var response = await _server.SendRequestAsync("workspace/configuration", request); var result = await response.Returning(cancellationToken); @@ -74,48 +58,107 @@ public async override Task GetLatestOptionsAsync(CancellationTo } } - private static RazorLSPOptions BuildOptions(JObject[] result) + private static ConfigurationParams GenerateConfigParams() + { + // NOTE: Do not change the ordering of sections without updating + // the code in the BuildOptions method below. + return new ConfigurationParams() + { + Items = new[] + { + new ConfigurationItem() + { + Section = "razor" + }, + new ConfigurationItem() + { + Section = "html" + }, + new ConfigurationItem() + { + Section = "vs.editor.razor" + }, + } + }; + } + + // Internal for testing + internal static RazorLSPOptions BuildOptions(JObject[] result) { - var instance = RazorLSPOptions.Default; + var defaultOptions = RazorLSPOptions.Default; + + UpdateVSCodeOptions(defaultOptions, result, out var trace, out var enableFormatting, out var autoClosingTags); + UpdateVSOptions(defaultOptions, result, out var insertSpaces, out var tabSize); + + return new RazorLSPOptions(trace, enableFormatting, autoClosingTags, insertSpaces, tabSize); + } + private static void UpdateVSCodeOptions( + RazorLSPOptions defaultOptions, + JObject[] result, + out Trace trace, + out bool enableFormatting, + out bool autoClosingTags) + { var razor = result[0]; var html = result[1]; - var editor = result[2]; - var trace = instance.Trace; + trace = defaultOptions.Trace; if (razor.TryGetValue("trace", out var parsedTrace)) { - trace = parsedTrace.ToObject(); + trace = JTokenToObject(parsedTrace, trace); } - var enableFormatting = instance.EnableFormatting; + enableFormatting = defaultOptions.EnableFormatting; if (razor.TryGetValue("format", out var parsedFormat)) { - if (((JObject)parsedFormat).TryGetValue("enable", out var parsedEnableFormatting)) + if (parsedFormat is JObject jObject && + jObject.TryGetValue("enable", out var parsedEnableFormatting)) { - enableFormatting = parsedEnableFormatting.ToObject(); + enableFormatting = JTokenToObject(parsedEnableFormatting, enableFormatting); } } - var autoClosingTags = instance.AutoClosingTags; + autoClosingTags = defaultOptions.AutoClosingTags; if (html.TryGetValue("autoClosingTags", out var parsedAutoClosingTags)) { - autoClosingTags = parsedAutoClosingTags.ToObject(); + autoClosingTags = JTokenToObject(parsedAutoClosingTags, autoClosingTags); } + } + + private static void UpdateVSOptions( + RazorLSPOptions defaultOptions, + JObject[] result, + out bool insertSpaces, + out int tabSize) + { + var vsEditor = result[2]; - var insertSpaces = instance.InsertSpaces; - if (editor.TryGetValue("InsertSpaces", out var parsedInsertSpaces)) + insertSpaces = defaultOptions.InsertSpaces; + if (vsEditor.TryGetValue(nameof(EditorSettings.IndentWithTabs), out var parsedInsertTabs)) { - insertSpaces = parsedInsertSpaces.ToObject(); + insertSpaces = !JTokenToObject(parsedInsertTabs, insertSpaces); } - var tabSize = instance.TabSize; - if (editor.TryGetValue("TabSize", out var parsedTabSize)) + tabSize = defaultOptions.TabSize; + if (vsEditor.TryGetValue(nameof(EditorSettings.IndentSize), out var parsedTabSize)) { - tabSize = parsedTabSize.ToObject(); + tabSize = JTokenToObject(parsedTabSize, tabSize); } + } - return new RazorLSPOptions(trace, enableFormatting, autoClosingTags, insertSpaces, tabSize); + private static T JTokenToObject(JToken token, T defaultValue) + { + try + { + // JToken.ToObject could potentially throw here if the user provides malformed options. + // If this occurs, catch the exception and return the default value. + return token.ToObject(); + } + catch + { + return defaultValue; + } } } } diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLSPOptions.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLSPOptions.cs index 15511b4f32c..f226448c499 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLSPOptions.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLSPOptions.cs @@ -50,7 +50,9 @@ public bool Equals(RazorLSPOptions other) other != null && Trace == other.Trace && EnableFormatting == other.EnableFormatting && - AutoClosingTags == other.AutoClosingTags; + AutoClosingTags == other.AutoClosingTags && + InsertSpaces == other.InsertSpaces && + TabSize == other.TabSize; } public override bool Equals(object obj) diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs index 892514d38b6..43dc0c83deb 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Composition; using System.Diagnostics; using System.Linq; @@ -15,6 +16,7 @@ using Microsoft.VisualStudio.LanguageServerClient.Razor.HtmlCSharp; using Microsoft.VisualStudio.Threading; using Newtonsoft.Json.Linq; +using OmniSharpConfigurationParams = OmniSharp.Extensions.LanguageServer.Protocol.Models.ConfigurationParams; using SemanticTokens = OmniSharp.Extensions.LanguageServer.Protocol.Models.Proposals.SemanticTokens; using Task = System.Threading.Tasks.Task; @@ -319,34 +321,32 @@ private static bool SupportsCSharpCodeActions(JToken token) return providesCodeActions && resolvesCodeActions; } - public override Task UpdateConfigurationAsync( - OmniSharp.Extensions.LanguageServer.Protocol.Models.ConfigurationParams configParams, + // NOTE: This method is a polyfill for VS. We only intend to do it this way until VS formally + // supports sending workspace configuration requests. + public override Task WorkspaceConfigurationAsync( + OmniSharpConfigurationParams configParams, CancellationToken cancellationToken) { - var insertSpaces = _clientOptionsMonitor.InsertSpaces; - var tabSize = _clientOptionsMonitor.TabSize; + if (configParams is null) + { + throw new ArgumentNullException(nameof(configParams)); + } - var result = new JObject[configParams.Items.Count()]; + var result = new List(); var index = 0; foreach (var item in configParams.Items) { // Right now in VS we only care about editor settings, but we should update this logic later if // we want to support Razor and HTML settings as well. - result[index] = item.Section == "editor" - ? JObject.FromObject(new JsonEditorSettings { InsertSpaces = insertSpaces, TabSize = tabSize }) - : new JObject(); + var setting = item.Section == "vs.editor.razor" + ? _clientOptionsMonitor.EditorSettings + : new object(); + result.Add(setting); index++; } - return Task.FromResult(result); - } - - private struct JsonEditorSettings - { - public bool InsertSpaces { get; set; } - - public int TabSize { get; set; } + return Task.FromResult(result.ToArray()); } } } diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPClientOptionsMonitor.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPClientOptionsMonitor.cs index eea8e3af4ee..a1b019574b7 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPClientOptionsMonitor.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPClientOptionsMonitor.cs @@ -1,7 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System.ComponentModel.Composition; +using System.Composition; +using Microsoft.CodeAnalysis.Razor.Editor; namespace Microsoft.VisualStudio.LanguageServerClient.Razor { @@ -9,17 +10,15 @@ namespace Microsoft.VisualStudio.LanguageServerClient.Razor /// Keeps track of accurate settings on the client side so we can easily retrieve the /// options later when the server sends us a workspace/configuration request. /// + [Shared] [Export(typeof(RazorLSPClientOptionsMonitor))] internal class RazorLSPClientOptionsMonitor { - public bool InsertSpaces { get; private set; } + public EditorSettings EditorSettings { get; private set; } - public int TabSize { get; private set; } - - public void UpdateOptions(bool insertSpaces, int tabSize) + public void UpdateOptions(EditorSettings editorSettings) { - InsertSpaces = insertSpaces; - TabSize = tabSize; + EditorSettings = editorSettings; } } } diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs index a2edf5290c3..a6bc034bcc5 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs @@ -7,6 +7,7 @@ using System.Threading; using MediatR; using Microsoft.AspNetCore.Razor.LanguageServer; +using Microsoft.CodeAnalysis.Razor.Editor; using Microsoft.CodeAnalysis.Razor.Workspaces; using Microsoft.VisualStudio.Editor; using Microsoft.VisualStudio.LanguageServer.ContainedLanguage; @@ -108,36 +109,54 @@ public void SubjectBuffersConnected(ITextView textView, ConnectionReason reason, RazorLSPTextViewFilter.CreateAndRegister(vsTextView); // Initialize the user's options and start listening for changes. - _razorEditorOptions = _editorOptionsFactory.GetOptions(textView); - Assumes.Present(_razorEditorOptions); - RazorOptions_OptionChanged(null, null); - _razorEditorOptions.OptionChanged += RazorOptions_OptionChanged; + // We only want to attach the option changed event once so we don't receive multiple + // notifications if there is more than one TextView active. + if (_razorEditorOptions == null && textView.TextBuffer.IsRazorLSPBuffer()) + { + _razorEditorOptions = _editorOptionsFactory.GetOptions(textView); + Assumes.Present(_razorEditorOptions); + RazorOptions_OptionChanged(null, null); + _razorEditorOptions.OptionChanged += RazorOptions_OptionChanged; + } } public void SubjectBuffersDisconnected(ITextView textView, ConnectionReason reason, IReadOnlyCollection subjectBuffers) { // When the TextView goes away so does the filter. No need to do anything more. + // However, we do need to detach from listening for option changes to avoid leaking. + if (_razorEditorOptions != null) + { + _razorEditorOptions.OptionChanged -= RazorOptions_OptionChanged; + _razorEditorOptions = null; + } } #pragma warning disable VSTHRD100 // Avoid async void methods private async void RazorOptions_OptionChanged(object sender, EditorOptionChangedEventArgs e) #pragma warning restore VSTHRD100 // Avoid async void methods { + if (_razorEditorOptions == null) + { + return; + } + // Retrieve current space/tabs settings from from Tools->Options. - var (insertSpaces, tabSize) = GetRazorEditorOptions(_textManager); + var settings = GetRazorEditorOptions(_textManager); // Update settings in the actual editor. - _razorEditorOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, insertSpaces); - _razorEditorOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, tabSize); + _razorEditorOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, !settings.IndentWithTabs); + _razorEditorOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, settings.IndentSize); // Keep track of accurate settings on the client side so we can easily retrieve the // options later when the server sends us a workspace/configuration request. - _clientOptionsMonitor.UpdateOptions(insertSpaces, tabSize); + _clientOptionsMonitor.UpdateOptions(settings); // Make sure the server updates the settings on their side by sending a // workspace/didChangeConfiguration request. This notifies the server that the user's // settings have changed. The server will then query the client's options monitor (already - // updated via the line above) by sending a workspace/configuration request. + // updated via the line above) by sending a workspace/configuration request. + // NOTE: This flow uses polyfilling and is used because VS doesn't yet support workspace + // configuration updates. Once they do, we can get rid of this extra logic. await _requestInvoker.ReinvokeRequestOnServerAsync( Methods.WorkspaceDidChangeConfigurationName, RazorLSPConstants.RazorLSPContentTypeName, @@ -145,7 +164,7 @@ await _requestInvoker.ReinvokeRequestOnServerAsync UpdateConfigurationAsync(OmniSharp.Extensions.LanguageServer.Protocol.Models.ConfigurationParams configParams, CancellationToken cancellationToken); + public abstract Task WorkspaceConfigurationAsync(OmniSharp.Extensions.LanguageServer.Protocol.Models.ConfigurationParams configParams, CancellationToken cancellationToken); // Called by the Razor Language Server to update the contents of the virtual CSharp buffer. [JsonRpcMethod(LanguageServerConstants.RazorUpdateCSharpBufferEndpoint, UseSingleObjectParameterDeserialization = true)] diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorConfigurationServiceTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorConfigurationServiceTest.cs index 2fbdf2479f4..abfab3934c8 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorConfigurationServiceTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorConfigurationServiceTest.cs @@ -18,13 +18,8 @@ public class DefaultRazorConfigurationServiceTest : LanguageServerTestBase public async Task GetLatestOptionsAsync_ReturnsExpectedOptions() { // Arrange - var expectedOptions = new RazorLSPOptions(Trace.Messages, enableFormatting: false, autoClosingTags: false, insertSpaces: true, tabSize: 8); - var editorJsonString = @" -{ - ""tabSize"": 8, - ""insertSpaces"": ""true"" -} -".Trim(); + var expectedOptions = new RazorLSPOptions( + Trace.Messages, enableFormatting: false, autoClosingTags: false, insertSpaces: true, tabSize: 8); var razorJsonString = @" { ""trace"": ""Messages"", @@ -39,7 +34,14 @@ public async Task GetLatestOptionsAsync_ReturnsExpectedOptions() ""autoClosingTags"": ""false"" } ".Trim(); - var result = new JObject[] { JObject.Parse(editorJsonString), JObject.Parse(razorJsonString), JObject.Parse(htmlJsonString) }; + var vsEditorJsonString = @" +{ + ""IndentSize"": 8, + ""IndentWithTabs"": ""false"" +} +".Trim(); + + var result = new JObject[] { JObject.Parse(razorJsonString), JObject.Parse(htmlJsonString), JObject.Parse(vsEditorJsonString) }; var languageServer = GetLanguageServer(new ResponseRouterReturns(result)); var configurationService = new DefaultRazorConfigurationService(languageServer, LoggerFactory); @@ -78,6 +80,79 @@ public async Task GetLatestOptionsAsync_ClientRequestThrows_ReturnsNull() Assert.Null(options); } + [Fact] + public void BuildOptions_ReturnsExpectedOptions() + { + // Arrange - purposely choosing options opposite of default + var expectedOptions = new RazorLSPOptions( + Trace.Verbose, enableFormatting: false, autoClosingTags: false, insertSpaces: false, tabSize: 8); + var razorJsonString = @" +{ + ""trace"": ""Verbose"", + ""format"": { + ""enable"": ""false"" + } +} +".Trim(); + var htmlJsonString = @" +{ + ""format"": ""true"", + ""autoClosingTags"": ""false"" +} +".Trim(); + var vsEditorJsonString = @" +{ + ""IndentSize"": 8, + ""IndentWithTabs"": ""true"" +} +".Trim(); + + // Act + var result = new JObject[] { JObject.Parse(razorJsonString), JObject.Parse(htmlJsonString), JObject.Parse(vsEditorJsonString) }; + var options = DefaultRazorConfigurationService.BuildOptions(result); + + // Assert + Assert.Equal(expectedOptions, options); + } + + [Fact] + public void BuildOptions_MalformedOptions() + { + // This test is purely to ensure we don't crash if the user provides malformed options. + + // Arrange + var defaultOptions = RazorLSPOptions.Default; + var expectedOptions = new RazorLSPOptions( + defaultOptions.Trace, defaultOptions.EnableFormatting, defaultOptions.AutoClosingTags, + insertSpaces: false, defaultOptions.TabSize); + var razorJsonString = @" +{ + ""trace"": 0, + ""format"": { + ""enable"": ""fals"" + } +} +".Trim(); + var htmlJsonString = @" +{ + ""format"": """", +} +".Trim(); + var vsEditorJsonString = @" +{ + ""IndentSize"": ""supposedToBeAnInt"", + ""IndentWithTabs"": 4 +} +".Trim(); + + // Act + var result = new JObject[] { JObject.Parse(razorJsonString), JObject.Parse(htmlJsonString), JObject.Parse(vsEditorJsonString) }; + var options = DefaultRazorConfigurationService.BuildOptions(result); + + // Assert + Assert.Equal(expectedOptions, options); + } + private ClientNotifierServiceBase GetLanguageServer(IResponseRouterReturns result, bool shouldThrow = false) { var languageServer = new Mock(MockBehavior.Strict); diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServerClient.Razor.Test/DefaultRazorLanguageServerCustomMessageTargetTest.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServerClient.Razor.Test/DefaultRazorLanguageServerCustomMessageTargetTest.cs index 555618ba723..3839717d43c 100644 --- a/src/Razor/test/Microsoft.VisualStudio.LanguageServerClient.Razor.Test/DefaultRazorLanguageServerCustomMessageTargetTest.cs +++ b/src/Razor/test/Microsoft.VisualStudio.LanguageServerClient.Razor.Test/DefaultRazorLanguageServerCustomMessageTargetTest.cs @@ -77,8 +77,11 @@ public async Task RazorRangeFormattingAsync_LanguageKindRazor_ReturnsEmpty() var requestInvoker = new Mock(MockBehavior.Strict); var uIContextManager = new Mock(MockBehavior.Strict); var disposable = new Mock(MockBehavior.Strict); + var clientOptionsMonitor = new Mock(MockBehavior.Strict); - var target = new DefaultRazorLanguageServerCustomMessageTarget(documentManager, JoinableTaskContext, requestInvoker.Object, uIContextManager.Object, disposable.Object); + var target = new DefaultRazorLanguageServerCustomMessageTarget( + documentManager, JoinableTaskContext, requestInvoker.Object, + uIContextManager.Object, disposable.Object, clientOptionsMonitor.Object); var request = new RazorDocumentRangeFormattingParams() { @@ -109,8 +112,11 @@ public async Task RazorRangeFormattingAsync_DocumentNotFound_ReturnsEmpty() var requestInvoker = new Mock(MockBehavior.Strict); var uIContextManager = new Mock(MockBehavior.Strict); var disposable = new Mock(MockBehavior.Strict); + var clientOptionsMonitor = new Mock(MockBehavior.Strict); - var target = new DefaultRazorLanguageServerCustomMessageTarget(documentManager, JoinableTaskContext, requestInvoker.Object, uIContextManager.Object, disposable.Object); + var target = new DefaultRazorLanguageServerCustomMessageTarget( + documentManager, JoinableTaskContext, requestInvoker.Object, + uIContextManager.Object, disposable.Object, clientOptionsMonitor.Object); var request = new RazorDocumentRangeFormattingParams() { @@ -156,8 +162,11 @@ public async Task RazorRangeFormattingAsync_ValidRequest_InvokesLanguageServer() var uIContextManager = new Mock(MockBehavior.Strict); var disposable = new Mock(MockBehavior.Strict); + var clientOptionsMonitor = new Mock(MockBehavior.Strict); - var target = new DefaultRazorLanguageServerCustomMessageTarget(documentManager.Object, JoinableTaskContext, requestInvoker.Object, uIContextManager.Object, disposable.Object); + var target = new DefaultRazorLanguageServerCustomMessageTarget( + documentManager.Object, JoinableTaskContext, requestInvoker.Object, + uIContextManager.Object, disposable.Object, clientOptionsMonitor.Object); var request = new RazorDocumentRangeFormattingParams() { @@ -264,8 +273,11 @@ public async Task ProvideCodeActionsAsync_ReturnsCodeActionsAsync() var uIContextManager = new Mock(MockBehavior.Strict); var disposable = new Mock(MockBehavior.Strict); + var clientOptionsMonitor = new Mock(MockBehavior.Strict); - var target = new DefaultRazorLanguageServerCustomMessageTarget(documentManager.Object, JoinableTaskContext, requestInvoker.Object, uIContextManager.Object, disposable.Object); + var target = new DefaultRazorLanguageServerCustomMessageTarget( + documentManager.Object, JoinableTaskContext, requestInvoker.Object, + uIContextManager.Object, disposable.Object, clientOptionsMonitor.Object); var request = new CodeActionParams() { TextDocument = new LanguageServer.Protocol.TextDocumentIdentifier() @@ -312,8 +324,11 @@ public async void ResolveCodeActionsAsync_ReturnsSingleCodeAction() var uIContextManager = new Mock(MockBehavior.Strict); var disposable = new Mock(MockBehavior.Strict); + var clientOptionsMonitor = new Mock(MockBehavior.Strict); - var target = new DefaultRazorLanguageServerCustomMessageTarget(documentManager.Object, JoinableTaskContext, requestInvoker.Object, uIContextManager.Object, disposable.Object); + var target = new DefaultRazorLanguageServerCustomMessageTarget( + documentManager.Object, JoinableTaskContext, requestInvoker.Object, + uIContextManager.Object, disposable.Object, clientOptionsMonitor.Object); var request = new VSCodeAction() { Title = "Something", @@ -405,8 +420,11 @@ public async Task ProvideSemanticTokensAsync_ReturnsSemanticTokensAsync() var uIContextManager = new Mock(MockBehavior.Strict); var disposable = new Mock(MockBehavior.Strict); + var clientOptionsMonitor = new Mock(MockBehavior.Strict); - var target = new DefaultRazorLanguageServerCustomMessageTarget(documentManager.Object, JoinableTaskContext, requestInvoker.Object, uIContextManager.Object, disposable.Object); + var target = new DefaultRazorLanguageServerCustomMessageTarget( + documentManager.Object, JoinableTaskContext, requestInvoker.Object, + uIContextManager.Object, disposable.Object, clientOptionsMonitor.Object); var request = new SemanticTokensParams() { TextDocument = new TextDocumentIdentifier() @@ -456,8 +474,11 @@ public async Task RazorServerReadyAsync_ReportsReadyAsync() disposable .Setup(d => d.Dispose()) .Verifiable(); + var clientOptionsMonitor = new Mock(MockBehavior.Strict); - var target = new DefaultRazorLanguageServerCustomMessageTarget(documentManager.Object, JoinableTaskContext, requestInvoker.Object, uIContextManager.Object, disposable.Object); + var target = new DefaultRazorLanguageServerCustomMessageTarget( + documentManager.Object, JoinableTaskContext, requestInvoker.Object, + uIContextManager.Object, disposable.Object, clientOptionsMonitor.Object); // Act await target.RazorServerReadyAsync(CancellationToken.None); From 43d89478e9e15d2975fc69259cf81a235206aef4 Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Thu, 25 Mar 2021 16:06:38 -0700 Subject: [PATCH 15/21] Fix redundant comment --- .../RazorLSPTextViewConnectionListener.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs index a6bc034bcc5..3ccb78d08b8 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs @@ -155,8 +155,8 @@ private async void RazorOptions_OptionChanged(object sender, EditorOptionChanged // workspace/didChangeConfiguration request. This notifies the server that the user's // settings have changed. The server will then query the client's options monitor (already // updated via the line above) by sending a workspace/configuration request. - // NOTE: This flow uses polyfilling and is used because VS doesn't yet support workspace - // configuration updates. Once they do, we can get rid of this extra logic. + // NOTE: This flow uses polyfilling because VS doesn't yet support workspace configuration + // updates. Once they do, we can get rid of this extra logic. await _requestInvoker.ReinvokeRequestOnServerAsync( Methods.WorkspaceDidChangeConfigurationName, RazorLSPConstants.RazorLSPContentTypeName, From 0fa0197244fe48882d1394bfa77e7b02d04fc087 Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Thu, 25 Mar 2021 21:41:36 -0700 Subject: [PATCH 16/21] Code review feedback pt2 --- .../DefaultRazorConfigurationService.cs | 70 +++++++++---------- .../RazorLSPOptions.cs | 2 + ...moteProjectSnapshotProjectEngineFactory.cs | 2 +- ...tRazorLanguageServerCustomMessageTarget.cs | 3 - .../RazorLSPTextViewConnectionListener.cs | 29 +++++--- .../DefaultRazorConfigurationServiceTest.cs | 8 ++- 6 files changed, 61 insertions(+), 53 deletions(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs index 8c873140d7e..15832f50250 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/DefaultRazorConfigurationService.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Razor.Editor; @@ -42,7 +43,7 @@ public async override Task GetLatestOptionsAsync(CancellationTo var result = await response.Returning(cancellationToken); // LSP spec indicates result should be the same length as the number of ConfigurationItems we pass in. - if (result == null || result.Length < 3 || result[0] == null) + if (result == null || result.Length != request.Items.Count() || result[0] == null) { _logger.LogWarning("Client failed to provide the expected configuration."); return null; @@ -66,35 +67,32 @@ private static ConfigurationParams GenerateConfigParams() { Items = new[] { - new ConfigurationItem() - { - Section = "razor" - }, - new ConfigurationItem() - { - Section = "html" - }, - new ConfigurationItem() - { - Section = "vs.editor.razor" - }, - } + new ConfigurationItem() + { + Section = "razor" + }, + new ConfigurationItem() + { + Section = "html" + }, + new ConfigurationItem() + { + Section = "vs.editor.razor" + }, + } }; } // Internal for testing - internal static RazorLSPOptions BuildOptions(JObject[] result) + internal RazorLSPOptions BuildOptions(JObject[] result) { - var defaultOptions = RazorLSPOptions.Default; - - UpdateVSCodeOptions(defaultOptions, result, out var trace, out var enableFormatting, out var autoClosingTags); - UpdateVSOptions(defaultOptions, result, out var insertSpaces, out var tabSize); + ExtractVSCodeOptions(result, out var trace, out var enableFormatting, out var autoClosingTags); + ExtractVSOptions(result, out var insertSpaces, out var tabSize); return new RazorLSPOptions(trace, enableFormatting, autoClosingTags, insertSpaces, tabSize); } - private static void UpdateVSCodeOptions( - RazorLSPOptions defaultOptions, + private void ExtractVSCodeOptions( JObject[] result, out Trace trace, out bool enableFormatting, @@ -103,60 +101,60 @@ private static void UpdateVSCodeOptions( var razor = result[0]; var html = result[1]; - trace = defaultOptions.Trace; + trace = RazorLSPOptions.Default.Trace; if (razor.TryGetValue("trace", out var parsedTrace)) { - trace = JTokenToObject(parsedTrace, trace); + trace = GetObjectOrDefault(parsedTrace, trace); } - enableFormatting = defaultOptions.EnableFormatting; + enableFormatting = RazorLSPOptions.Default.EnableFormatting; if (razor.TryGetValue("format", out var parsedFormat)) { if (parsedFormat is JObject jObject && jObject.TryGetValue("enable", out var parsedEnableFormatting)) { - enableFormatting = JTokenToObject(parsedEnableFormatting, enableFormatting); + enableFormatting = GetObjectOrDefault(parsedEnableFormatting, enableFormatting); } } - autoClosingTags = defaultOptions.AutoClosingTags; + autoClosingTags = RazorLSPOptions.Default.AutoClosingTags; if (html.TryGetValue("autoClosingTags", out var parsedAutoClosingTags)) { - autoClosingTags = JTokenToObject(parsedAutoClosingTags, autoClosingTags); + autoClosingTags = GetObjectOrDefault(parsedAutoClosingTags, autoClosingTags); } } - private static void UpdateVSOptions( - RazorLSPOptions defaultOptions, + private void ExtractVSOptions( JObject[] result, out bool insertSpaces, out int tabSize) { var vsEditor = result[2]; - insertSpaces = defaultOptions.InsertSpaces; + insertSpaces = RazorLSPOptions.Default.InsertSpaces; if (vsEditor.TryGetValue(nameof(EditorSettings.IndentWithTabs), out var parsedInsertTabs)) { - insertSpaces = !JTokenToObject(parsedInsertTabs, insertSpaces); + insertSpaces = !GetObjectOrDefault(parsedInsertTabs, insertSpaces); } - tabSize = defaultOptions.TabSize; + tabSize = RazorLSPOptions.Default.TabSize; if (vsEditor.TryGetValue(nameof(EditorSettings.IndentSize), out var parsedTabSize)) { - tabSize = JTokenToObject(parsedTabSize, tabSize); + tabSize = GetObjectOrDefault(parsedTabSize, tabSize); } } - private static T JTokenToObject(JToken token, T defaultValue) + private T GetObjectOrDefault(JToken token, T defaultValue) { try { // JToken.ToObject could potentially throw here if the user provides malformed options. // If this occurs, catch the exception and return the default value. - return token.ToObject(); + return token.ToObject() ?? defaultValue; } - catch + catch (Exception ex) { + _logger.LogError(ex, $"Malformed option: Token {token} cannot be converted to type {typeof(T)}."); return defaultValue; } } diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLSPOptions.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLSPOptions.cs index f226448c499..9841823d305 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLSPOptions.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RazorLSPOptions.cs @@ -66,6 +66,8 @@ public override int GetHashCode() hash.Add(Trace); hash.Add(EnableFormatting); hash.Add(AutoClosingTags); + hash.Add(InsertSpaces); + hash.Add(TabSize); return hash; } } diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RemoteProjectSnapshotProjectEngineFactory.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RemoteProjectSnapshotProjectEngineFactory.cs index 2babbdaa801..db5fea2fc1a 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RemoteProjectSnapshotProjectEngineFactory.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/RemoteProjectSnapshotProjectEngineFactory.cs @@ -20,7 +20,7 @@ internal class RemoteProjectSnapshotProjectEngineFactory : DefaultProjectSnapsho public RemoteProjectSnapshotProjectEngineFactory(FilePathNormalizer filePathNormalizer, IOptionsMonitor optionsMonitor) : base(FallbackProjectEngineFactory, ProjectEngineFactories.Factories) { - if (filePathNormalizer == null) + if (filePathNormalizer is null) { throw new ArgumentNullException(nameof(filePathNormalizer)); } diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs index 43dc0c83deb..3aa3bb1ed09 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/DefaultRazorLanguageServerCustomMessageTarget.cs @@ -333,7 +333,6 @@ public override Task WorkspaceConfigurationAsync( } var result = new List(); - var index = 0; foreach (var item in configParams.Items) { // Right now in VS we only care about editor settings, but we should update this logic later if @@ -342,8 +341,6 @@ public override Task WorkspaceConfigurationAsync( ? _clientOptionsMonitor.EditorSettings : new object(); result.Add(setting); - - index++; } return Task.FromResult(result.ToArray()); diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs index 3ccb78d08b8..89508163e53 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs @@ -31,6 +31,8 @@ namespace Microsoft.VisualStudio.LanguageServerClient.Razor [ContentType(RazorLSPConstants.RazorLSPContentTypeName)] internal class RazorLSPTextViewConnectionListener : ITextViewConnectionListener { + private const string RazorEditorOptions = "RazorEditorOptions"; + private readonly IVsEditorAdaptersFactoryService _editorAdaptersFactory; private readonly LSPEditorFeatureDetector _editorFeatureDetector; private readonly IEditorOptionsFactoryService _editorOptionsFactory; @@ -38,7 +40,7 @@ internal class RazorLSPTextViewConnectionListener : ITextViewConnectionListener private readonly RazorLSPClientOptionsMonitor _clientOptionsMonitor; private readonly IVsTextManager2 _textManager; - private IEditorOptions _razorEditorOptions; + private ITextBuffer _textBuffer; [ImportingConstructor] public RazorLSPTextViewConnectionListener( @@ -111,12 +113,16 @@ public void SubjectBuffersConnected(ITextView textView, ConnectionReason reason, // Initialize the user's options and start listening for changes. // We only want to attach the option changed event once so we don't receive multiple // notifications if there is more than one TextView active. - if (_razorEditorOptions == null && textView.TextBuffer.IsRazorLSPBuffer()) + if (!textView.TextBuffer.Properties.ContainsProperty(RazorEditorOptions) && + textView.TextBuffer.IsRazorLSPBuffer()) { - _razorEditorOptions = _editorOptionsFactory.GetOptions(textView); - Assumes.Present(_razorEditorOptions); + var razorEditorOptions = _editorOptionsFactory.GetOptions(textView); + Assumes.Present(razorEditorOptions); + textView.TextBuffer.Properties[RazorEditorOptions] = razorEditorOptions; + _textBuffer = textView.TextBuffer; + RazorOptions_OptionChanged(null, null); - _razorEditorOptions.OptionChanged += RazorOptions_OptionChanged; + razorEditorOptions.OptionChanged += RazorOptions_OptionChanged; } } @@ -124,10 +130,10 @@ public void SubjectBuffersDisconnected(ITextView textView, ConnectionReason reas { // When the TextView goes away so does the filter. No need to do anything more. // However, we do need to detach from listening for option changes to avoid leaking. - if (_razorEditorOptions != null) + if (_textBuffer != null && _textBuffer.Properties.TryGetProperty(RazorEditorOptions, out IEditorOptions editorOptions)) { - _razorEditorOptions.OptionChanged -= RazorOptions_OptionChanged; - _razorEditorOptions = null; + editorOptions.OptionChanged -= RazorOptions_OptionChanged; + _textBuffer.Properties.RemoveProperty(RazorEditorOptions); } } @@ -135,7 +141,8 @@ public void SubjectBuffersDisconnected(ITextView textView, ConnectionReason reas private async void RazorOptions_OptionChanged(object sender, EditorOptionChangedEventArgs e) #pragma warning restore VSTHRD100 // Avoid async void methods { - if (_razorEditorOptions == null) + if (_textBuffer == null || + !_textBuffer.Properties.TryGetProperty(RazorEditorOptions, out IEditorOptions razorEditorOptions)) { return; } @@ -144,8 +151,8 @@ private async void RazorOptions_OptionChanged(object sender, EditorOptionChanged var settings = GetRazorEditorOptions(_textManager); // Update settings in the actual editor. - _razorEditorOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, !settings.IndentWithTabs); - _razorEditorOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, settings.IndentSize); + razorEditorOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, !settings.IndentWithTabs); + razorEditorOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, settings.IndentSize); // Keep track of accurate settings on the client side so we can easily retrieve the // options later when the server sends us a workspace/configuration request. diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorConfigurationServiceTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorConfigurationServiceTest.cs index abfab3934c8..b6bebd3d98d 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorConfigurationServiceTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/DefaultRazorConfigurationServiceTest.cs @@ -109,7 +109,9 @@ public void BuildOptions_ReturnsExpectedOptions() // Act var result = new JObject[] { JObject.Parse(razorJsonString), JObject.Parse(htmlJsonString), JObject.Parse(vsEditorJsonString) }; - var options = DefaultRazorConfigurationService.BuildOptions(result); + var languageServer = GetLanguageServer(new ResponseRouterReturns(result)); + var configurationService = new DefaultRazorConfigurationService(languageServer, LoggerFactory); + var options = configurationService.BuildOptions(result); // Assert Assert.Equal(expectedOptions, options); @@ -147,7 +149,9 @@ public void BuildOptions_MalformedOptions() // Act var result = new JObject[] { JObject.Parse(razorJsonString), JObject.Parse(htmlJsonString), JObject.Parse(vsEditorJsonString) }; - var options = DefaultRazorConfigurationService.BuildOptions(result); + var languageServer = GetLanguageServer(new ResponseRouterReturns(result)); + var configurationService = new DefaultRazorConfigurationService(languageServer, LoggerFactory); + var options = configurationService.BuildOptions(result); // Assert Assert.Equal(expectedOptions, options); From e31ad33e6022d7e3cdc1fd1109a9a33ca2361597 Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Fri, 26 Mar 2021 22:16:08 -0700 Subject: [PATCH 17/21] TextView updates --- .../RazorLSPTextViewConnectionListener.cs | 94 +++++++++++++++---- 1 file changed, 74 insertions(+), 20 deletions(-) diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs index 89508163e53..6d2fa0e817a 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs @@ -31,7 +31,9 @@ namespace Microsoft.VisualStudio.LanguageServerClient.Razor [ContentType(RazorLSPConstants.RazorLSPContentTypeName)] internal class RazorLSPTextViewConnectionListener : ITextViewConnectionListener { - private const string RazorEditorOptions = "RazorEditorOptions"; + private const string RazorEditorTrackedView = "RazorEditorTrackedView"; + private const string RazorEditorViewOptions = "RazorEditorViewOptions"; + private const string RazorEditorBufferOptions = "RazorEditorBufferOptions"; private readonly IVsEditorAdaptersFactoryService _editorAdaptersFactory; private readonly LSPEditorFeatureDetector _editorFeatureDetector; @@ -39,6 +41,8 @@ internal class RazorLSPTextViewConnectionListener : ITextViewConnectionListener private readonly LSPRequestInvoker _requestInvoker; private readonly RazorLSPClientOptionsMonitor _clientOptionsMonitor; private readonly IVsTextManager2 _textManager; + private readonly List _activeTextViews = new(); + private readonly object _lock = new(); private ITextBuffer _textBuffer; @@ -110,19 +114,33 @@ public void SubjectBuffersConnected(ITextView textView, ConnectionReason reason, RazorLSPTextViewFilter.CreateAndRegister(vsTextView); - // Initialize the user's options and start listening for changes. - // We only want to attach the option changed event once so we don't receive multiple - // notifications if there is more than one TextView active. - if (!textView.TextBuffer.Properties.ContainsProperty(RazorEditorOptions) && - textView.TextBuffer.IsRazorLSPBuffer()) + lock (_lock) { - var razorEditorOptions = _editorOptionsFactory.GetOptions(textView); - Assumes.Present(razorEditorOptions); - textView.TextBuffer.Properties[RazorEditorOptions] = razorEditorOptions; - _textBuffer = textView.TextBuffer; - - RazorOptions_OptionChanged(null, null); - razorEditorOptions.OptionChanged += RazorOptions_OptionChanged; + _activeTextViews.Add(textView); + + // Initialize the user's options and start listening for changes. + // We only want to attach the option changed event once so we don't receive multiple + // notifications if there is more than one TextView active. + if (!textView.TextBuffer.Properties.ContainsProperty(RazorEditorTrackedView) && + textView.TextBuffer.IsRazorLSPBuffer()) + { + // We assume there is ever only one TextBuffer at a time and thus all active + // TextViews have the same TextBuffer. + _textBuffer = textView.TextBuffer; + + var bufferOptions = _editorOptionsFactory.GetOptions(_textBuffer); + _textBuffer.Properties[RazorEditorBufferOptions] = bufferOptions; + + // All TextViews share the same options, so we only need to listen to changes for one. + textView.TextBuffer.Properties[RazorEditorTrackedView] = textView; + + var razorEditorTextViewOptions = _editorOptionsFactory.GetOptions(textView); + Assumes.Present(razorEditorTextViewOptions); + textView.TextBuffer.Properties[RazorEditorViewOptions] = razorEditorTextViewOptions; + + RazorOptions_OptionChanged(null, null); + razorEditorTextViewOptions.OptionChanged += RazorOptions_OptionChanged; + } } } @@ -130,10 +148,38 @@ public void SubjectBuffersDisconnected(ITextView textView, ConnectionReason reas { // When the TextView goes away so does the filter. No need to do anything more. // However, we do need to detach from listening for option changes to avoid leaking. - if (_textBuffer != null && _textBuffer.Properties.TryGetProperty(RazorEditorOptions, out IEditorOptions editorOptions)) + // We should switch to listening to a different TextView if the one we're listening + // to is disconnected. + Assumes.NotNull(_textBuffer); + + lock (_lock) { - editorOptions.OptionChanged -= RazorOptions_OptionChanged; - _textBuffer.Properties.RemoveProperty(RazorEditorOptions); + _activeTextViews.Remove(textView); + + // Is the tracked TextView where we listen for option changes the one being disconnected? + // If so, see if another view is available. + if (_textBuffer.Properties.TryGetProperty(RazorEditorTrackedView, out ITextView trackedTextView) && + trackedTextView == textView) + { + Assumes.True(_textBuffer.Properties.TryGetProperty(RazorEditorViewOptions, out IEditorOptions textViewOptions)); + + // If there's another text view we can use to listen for options, start tracking it. + _textBuffer.Properties.RemoveProperty(RazorEditorTrackedView); + _textBuffer.Properties.RemoveProperty(RazorEditorViewOptions); + textViewOptions.OptionChanged -= RazorOptions_OptionChanged; + + if (_activeTextViews.Count != 0) + { + var updatedTextView = _activeTextViews[0]; + _textBuffer.Properties[RazorEditorTrackedView] = updatedTextView; + + var updatedRazorEditorTextViewOptions = _editorOptionsFactory.GetOptions(updatedTextView); + Assumes.Present(updatedRazorEditorTextViewOptions); + _textBuffer.Properties[RazorEditorViewOptions] = updatedRazorEditorTextViewOptions; + + updatedRazorEditorTextViewOptions.OptionChanged += RazorOptions_OptionChanged; + } + } } } @@ -141,8 +187,10 @@ public void SubjectBuffersDisconnected(ITextView textView, ConnectionReason reas private async void RazorOptions_OptionChanged(object sender, EditorOptionChangedEventArgs e) #pragma warning restore VSTHRD100 // Avoid async void methods { - if (_textBuffer == null || - !_textBuffer.Properties.TryGetProperty(RazorEditorOptions, out IEditorOptions razorEditorOptions)) + Assumes.NotNull(_textBuffer); + + if (!_textBuffer.Properties.TryGetProperty(RazorEditorViewOptions, out IEditorOptions razorEditorTextViewOptions) || + !_textBuffer.Properties.TryGetProperty(RazorEditorBufferOptions, out IEditorOptions razorEditorTextBufferOptions)) { return; } @@ -151,8 +199,14 @@ private async void RazorOptions_OptionChanged(object sender, EditorOptionChanged var settings = GetRazorEditorOptions(_textManager); // Update settings in the actual editor. - razorEditorOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, !settings.IndentWithTabs); - razorEditorOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, settings.IndentSize); + // We need to update both the TextView and TextBuffer options. Updating the TextView is necessary + // so 'SPC'/'TABS' in the bottom right corner of the view displays the right setting. Updating the + // TextBuffer is necessary since it's where LSP pulls settings from when sending us requests. + razorEditorTextViewOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, !settings.IndentWithTabs); + razorEditorTextViewOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, settings.IndentSize); + + razorEditorTextBufferOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, !settings.IndentWithTabs); + razorEditorTextBufferOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, settings.IndentSize); // Keep track of accurate settings on the client side so we can easily retrieve the // options later when the server sends us a workspace/configuration request. From 830ec16d08748e324138c700d363b836d5c54a4c Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Fri, 26 Mar 2021 22:29:19 -0700 Subject: [PATCH 18/21] Cleanup --- .../RazorLSPTextViewConnectionListener.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs index 6d2fa0e817a..28203485b38 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs @@ -41,10 +41,18 @@ internal class RazorLSPTextViewConnectionListener : ITextViewConnectionListener private readonly LSPRequestInvoker _requestInvoker; private readonly RazorLSPClientOptionsMonitor _clientOptionsMonitor; private readonly IVsTextManager2 _textManager; - private readonly List _activeTextViews = new(); + + /// + /// Protects concurrent modifications to _activeTextViews and _textBuffer's + /// property bag. + /// private readonly object _lock = new(); + #region protected by _lock + private readonly List _activeTextViews = new(); + private ITextBuffer _textBuffer; + #endregion [ImportingConstructor] public RazorLSPTextViewConnectionListener( @@ -132,11 +140,11 @@ public void SubjectBuffersConnected(ITextView textView, ConnectionReason reason, _textBuffer.Properties[RazorEditorBufferOptions] = bufferOptions; // All TextViews share the same options, so we only need to listen to changes for one. - textView.TextBuffer.Properties[RazorEditorTrackedView] = textView; + _textBuffer.Properties[RazorEditorTrackedView] = textView; var razorEditorTextViewOptions = _editorOptionsFactory.GetOptions(textView); Assumes.Present(razorEditorTextViewOptions); - textView.TextBuffer.Properties[RazorEditorViewOptions] = razorEditorTextViewOptions; + _textBuffer.Properties[RazorEditorViewOptions] = razorEditorTextViewOptions; RazorOptions_OptionChanged(null, null); razorEditorTextViewOptions.OptionChanged += RazorOptions_OptionChanged; From 7249fce068f51522dd47aa29418b38e8e3923cf2 Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Mon, 29 Mar 2021 12:39:41 -0700 Subject: [PATCH 19/21] Code review feedback --- .../RazorLSPTextViewConnectionListener.cs | 125 ++++++++++-------- 1 file changed, 70 insertions(+), 55 deletions(-) diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs index 28203485b38..0fdd78cdd4b 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs @@ -31,10 +31,6 @@ namespace Microsoft.VisualStudio.LanguageServerClient.Razor [ContentType(RazorLSPConstants.RazorLSPContentTypeName)] internal class RazorLSPTextViewConnectionListener : ITextViewConnectionListener { - private const string RazorEditorTrackedView = "RazorEditorTrackedView"; - private const string RazorEditorViewOptions = "RazorEditorViewOptions"; - private const string RazorEditorBufferOptions = "RazorEditorBufferOptions"; - private readonly IVsEditorAdaptersFactoryService _editorAdaptersFactory; private readonly LSPEditorFeatureDetector _editorFeatureDetector; private readonly IEditorOptionsFactoryService _editorOptionsFactory; @@ -122,34 +118,45 @@ public void SubjectBuffersConnected(ITextView textView, ConnectionReason reason, RazorLSPTextViewFilter.CreateAndRegister(vsTextView); - lock (_lock) + if (textView.TextBuffer.IsRazorLSPBuffer()) { - _activeTextViews.Add(textView); - - // Initialize the user's options and start listening for changes. - // We only want to attach the option changed event once so we don't receive multiple - // notifications if there is more than one TextView active. - if (!textView.TextBuffer.Properties.ContainsProperty(RazorEditorTrackedView) && - textView.TextBuffer.IsRazorLSPBuffer()) + lock (_lock) { - // We assume there is ever only one TextBuffer at a time and thus all active - // TextViews have the same TextBuffer. - _textBuffer = textView.TextBuffer; - - var bufferOptions = _editorOptionsFactory.GetOptions(_textBuffer); - _textBuffer.Properties[RazorEditorBufferOptions] = bufferOptions; - - // All TextViews share the same options, so we only need to listen to changes for one. - _textBuffer.Properties[RazorEditorTrackedView] = textView; + _activeTextViews.Add(textView); - var razorEditorTextViewOptions = _editorOptionsFactory.GetOptions(textView); - Assumes.Present(razorEditorTextViewOptions); - _textBuffer.Properties[RazorEditorViewOptions] = razorEditorTextViewOptions; - - RazorOptions_OptionChanged(null, null); - razorEditorTextViewOptions.OptionChanged += RazorOptions_OptionChanged; + // Initialize the user's options and start listening for changes. + // We only want to attach the option changed event once so we don't receive multiple + // notifications if there is more than one TextView active. + if (!textView.TextBuffer.Properties.ContainsProperty(typeof(RazorEditorOptionsTracker))) + { + // We assume there is ever only one TextBuffer at a time and thus all active + // TextViews have the same TextBuffer. + _textBuffer = textView.TextBuffer; + + var bufferOptions = _editorOptionsFactory.GetOptions(_textBuffer); + var viewOptions = _editorOptionsFactory.GetOptions(textView); + + Assumes.Present(bufferOptions); + Assumes.Present(viewOptions); + + // All TextViews share the same options, so we only need to listen to changes for one. + // We need to keep track of and update both the TextView and TextBuffer options. Updating + // the TextView's options is necessary so 'SPC'/'TABS' in the bottom right corner of the + // view displays the right setting. Updating the TextBuffer is necessary since it's where + // LSP pulls settings from when sending us requests. + var optionsTracker = new RazorEditorOptionsTracker(TrackedView: textView, viewOptions, bufferOptions); + _textBuffer.Properties[typeof(RazorEditorOptionsTracker)] = optionsTracker; + + // A change in Tools->Options settings only kicks off an options changed event in the view + // and not the buffer, i.e. even if we listened for TextBuffer option changes, we would never + // be notified. As a workaround, we listen purely for TextView changes, and update the + // TextBuffer options in the TextView listener as well. + RazorOptions_OptionChanged(null, null); + viewOptions.OptionChanged += RazorOptions_OptionChanged; + } } } + } public void SubjectBuffersDisconnected(ITextView textView, ConnectionReason reason, IReadOnlyCollection subjectBuffers) @@ -160,35 +167,40 @@ public void SubjectBuffersDisconnected(ITextView textView, ConnectionReason reas // to is disconnected. Assumes.NotNull(_textBuffer); - lock (_lock) + if (textView.TextBuffer.IsRazorLSPBuffer()) { - _activeTextViews.Remove(textView); - - // Is the tracked TextView where we listen for option changes the one being disconnected? - // If so, see if another view is available. - if (_textBuffer.Properties.TryGetProperty(RazorEditorTrackedView, out ITextView trackedTextView) && - trackedTextView == textView) + lock (_lock) { - Assumes.True(_textBuffer.Properties.TryGetProperty(RazorEditorViewOptions, out IEditorOptions textViewOptions)); - - // If there's another text view we can use to listen for options, start tracking it. - _textBuffer.Properties.RemoveProperty(RazorEditorTrackedView); - _textBuffer.Properties.RemoveProperty(RazorEditorViewOptions); - textViewOptions.OptionChanged -= RazorOptions_OptionChanged; + _activeTextViews.Remove(textView); - if (_activeTextViews.Count != 0) + // Is the tracked TextView where we listen for option changes the one being disconnected? + // If so, see if another view is available. + if (_textBuffer.Properties.TryGetProperty( + typeof(RazorEditorOptionsTracker), out RazorEditorOptionsTracker optionsTracker) && + optionsTracker.TrackedView == textView) { - var updatedTextView = _activeTextViews[0]; - _textBuffer.Properties[RazorEditorTrackedView] = updatedTextView; - - var updatedRazorEditorTextViewOptions = _editorOptionsFactory.GetOptions(updatedTextView); - Assumes.Present(updatedRazorEditorTextViewOptions); - _textBuffer.Properties[RazorEditorViewOptions] = updatedRazorEditorTextViewOptions; - - updatedRazorEditorTextViewOptions.OptionChanged += RazorOptions_OptionChanged; + _textBuffer.Properties.RemoveProperty(typeof(RazorEditorOptionsTracker)); + optionsTracker.ViewOptions.OptionChanged -= RazorOptions_OptionChanged; + + // If there's another text view we can use to listen for options, start tracking it. + if (_activeTextViews.Count != 0) + { + var newTrackedView = _activeTextViews[0]; + var newViewOptions = _editorOptionsFactory.GetOptions(newTrackedView); + Assumes.Present(newViewOptions); + + // We assume the TextViews all have the same TextBuffer, so we can reuse the + // buffer options from the old TextView. + var newOptionsTracker = new RazorEditorOptionsTracker( + newTrackedView, newViewOptions, optionsTracker.BufferOptions); + _textBuffer.Properties[typeof(RazorEditorOptionsTracker)] = newOptionsTracker; + + newViewOptions.OptionChanged += RazorOptions_OptionChanged; + } } } } + } #pragma warning disable VSTHRD100 // Avoid async void methods @@ -197,8 +209,7 @@ private async void RazorOptions_OptionChanged(object sender, EditorOptionChanged { Assumes.NotNull(_textBuffer); - if (!_textBuffer.Properties.TryGetProperty(RazorEditorViewOptions, out IEditorOptions razorEditorTextViewOptions) || - !_textBuffer.Properties.TryGetProperty(RazorEditorBufferOptions, out IEditorOptions razorEditorTextBufferOptions)) + if (!_textBuffer.Properties.TryGetProperty(typeof(RazorEditorOptionsTracker), out RazorEditorOptionsTracker optionsTracker)) { return; } @@ -210,11 +221,11 @@ private async void RazorOptions_OptionChanged(object sender, EditorOptionChanged // We need to update both the TextView and TextBuffer options. Updating the TextView is necessary // so 'SPC'/'TABS' in the bottom right corner of the view displays the right setting. Updating the // TextBuffer is necessary since it's where LSP pulls settings from when sending us requests. - razorEditorTextViewOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, !settings.IndentWithTabs); - razorEditorTextViewOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, settings.IndentSize); + optionsTracker.ViewOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, !settings.IndentWithTabs); + optionsTracker.ViewOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, settings.IndentSize); - razorEditorTextBufferOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, !settings.IndentWithTabs); - razorEditorTextBufferOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, settings.IndentSize); + optionsTracker.BufferOptions.SetOptionValue(DefaultOptions.ConvertTabsToSpacesOptionId, !settings.IndentWithTabs); + optionsTracker.BufferOptions.SetOptionValue(DefaultOptions.TabSizeOptionId, settings.IndentSize); // Keep track of accurate settings on the client side so we can easily retrieve the // options later when the server sends us a workspace/configuration request. @@ -286,5 +297,9 @@ public int GetDataTipText(TextSpan[] pSpan, out string pbstrText) public int GetPairExtents(int iLine, int iIndex, TextSpan[] pSpan) => VSConstants.E_NOTIMPL; } + + private record RazorEditorOptionsTracker(ITextView TrackedView, IEditorOptions ViewOptions, IEditorOptions BufferOptions) + { + } } } From c2e9b61718bfb71a495c48d60409c528569f69ee Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Mon, 29 Mar 2021 12:41:17 -0700 Subject: [PATCH 20/21] Clean up record --- .../RazorLSPTextViewConnectionListener.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs index 0fdd78cdd4b..a3af53e9a29 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs @@ -298,8 +298,6 @@ public int GetDataTipText(TextSpan[] pSpan, out string pbstrText) public int GetPairExtents(int iLine, int iIndex, TextSpan[] pSpan) => VSConstants.E_NOTIMPL; } - private record RazorEditorOptionsTracker(ITextView TrackedView, IEditorOptions ViewOptions, IEditorOptions BufferOptions) - { - } + private record RazorEditorOptionsTracker(ITextView TrackedView, IEditorOptions ViewOptions, IEditorOptions BufferOptions); } } From 9f012b9dbdb4bb7d221e1970172c07a482398b9e Mon Sep 17 00:00:00 2001 From: Allison Chou Date: Mon, 29 Mar 2021 12:52:18 -0700 Subject: [PATCH 21/21] Refactor --- .../RazorLSPTextViewConnectionListener.cs | 126 +++++++++--------- 1 file changed, 64 insertions(+), 62 deletions(-) diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs index a3af53e9a29..784eacc7687 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServerClient.Razor/RazorLSPTextViewConnectionListener.cs @@ -118,45 +118,46 @@ public void SubjectBuffersConnected(ITextView textView, ConnectionReason reason, RazorLSPTextViewFilter.CreateAndRegister(vsTextView); - if (textView.TextBuffer.IsRazorLSPBuffer()) + if (!textView.TextBuffer.IsRazorLSPBuffer()) { - lock (_lock) - { - _activeTextViews.Add(textView); + return; + } - // Initialize the user's options and start listening for changes. - // We only want to attach the option changed event once so we don't receive multiple - // notifications if there is more than one TextView active. - if (!textView.TextBuffer.Properties.ContainsProperty(typeof(RazorEditorOptionsTracker))) - { - // We assume there is ever only one TextBuffer at a time and thus all active - // TextViews have the same TextBuffer. - _textBuffer = textView.TextBuffer; - - var bufferOptions = _editorOptionsFactory.GetOptions(_textBuffer); - var viewOptions = _editorOptionsFactory.GetOptions(textView); - - Assumes.Present(bufferOptions); - Assumes.Present(viewOptions); - - // All TextViews share the same options, so we only need to listen to changes for one. - // We need to keep track of and update both the TextView and TextBuffer options. Updating - // the TextView's options is necessary so 'SPC'/'TABS' in the bottom right corner of the - // view displays the right setting. Updating the TextBuffer is necessary since it's where - // LSP pulls settings from when sending us requests. - var optionsTracker = new RazorEditorOptionsTracker(TrackedView: textView, viewOptions, bufferOptions); - _textBuffer.Properties[typeof(RazorEditorOptionsTracker)] = optionsTracker; - - // A change in Tools->Options settings only kicks off an options changed event in the view - // and not the buffer, i.e. even if we listened for TextBuffer option changes, we would never - // be notified. As a workaround, we listen purely for TextView changes, and update the - // TextBuffer options in the TextView listener as well. - RazorOptions_OptionChanged(null, null); - viewOptions.OptionChanged += RazorOptions_OptionChanged; - } + lock (_lock) + { + _activeTextViews.Add(textView); + + // Initialize the user's options and start listening for changes. + // We only want to attach the option changed event once so we don't receive multiple + // notifications if there is more than one TextView active. + if (!textView.TextBuffer.Properties.ContainsProperty(typeof(RazorEditorOptionsTracker))) + { + // We assume there is ever only one TextBuffer at a time and thus all active + // TextViews have the same TextBuffer. + _textBuffer = textView.TextBuffer; + + var bufferOptions = _editorOptionsFactory.GetOptions(_textBuffer); + var viewOptions = _editorOptionsFactory.GetOptions(textView); + + Assumes.Present(bufferOptions); + Assumes.Present(viewOptions); + + // All TextViews share the same options, so we only need to listen to changes for one. + // We need to keep track of and update both the TextView and TextBuffer options. Updating + // the TextView's options is necessary so 'SPC'/'TABS' in the bottom right corner of the + // view displays the right setting. Updating the TextBuffer is necessary since it's where + // LSP pulls settings from when sending us requests. + var optionsTracker = new RazorEditorOptionsTracker(TrackedView: textView, viewOptions, bufferOptions); + _textBuffer.Properties[typeof(RazorEditorOptionsTracker)] = optionsTracker; + + // A change in Tools->Options settings only kicks off an options changed event in the view + // and not the buffer, i.e. even if we listened for TextBuffer option changes, we would never + // be notified. As a workaround, we listen purely for TextView changes, and update the + // TextBuffer options in the TextView listener as well. + RazorOptions_OptionChanged(null, null); + viewOptions.OptionChanged += RazorOptions_OptionChanged; } } - } public void SubjectBuffersDisconnected(ITextView textView, ConnectionReason reason, IReadOnlyCollection subjectBuffers) @@ -167,40 +168,41 @@ public void SubjectBuffersDisconnected(ITextView textView, ConnectionReason reas // to is disconnected. Assumes.NotNull(_textBuffer); - if (textView.TextBuffer.IsRazorLSPBuffer()) + if (!textView.TextBuffer.IsRazorLSPBuffer()) { - lock (_lock) + return; + } + + lock (_lock) + { + _activeTextViews.Remove(textView); + + // Is the tracked TextView where we listen for option changes the one being disconnected? + // If so, see if another view is available. + if (_textBuffer.Properties.TryGetProperty( + typeof(RazorEditorOptionsTracker), out RazorEditorOptionsTracker optionsTracker) && + optionsTracker.TrackedView == textView) { - _activeTextViews.Remove(textView); + _textBuffer.Properties.RemoveProperty(typeof(RazorEditorOptionsTracker)); + optionsTracker.ViewOptions.OptionChanged -= RazorOptions_OptionChanged; - // Is the tracked TextView where we listen for option changes the one being disconnected? - // If so, see if another view is available. - if (_textBuffer.Properties.TryGetProperty( - typeof(RazorEditorOptionsTracker), out RazorEditorOptionsTracker optionsTracker) && - optionsTracker.TrackedView == textView) + // If there's another text view we can use to listen for options, start tracking it. + if (_activeTextViews.Count != 0) { - _textBuffer.Properties.RemoveProperty(typeof(RazorEditorOptionsTracker)); - optionsTracker.ViewOptions.OptionChanged -= RazorOptions_OptionChanged; - - // If there's another text view we can use to listen for options, start tracking it. - if (_activeTextViews.Count != 0) - { - var newTrackedView = _activeTextViews[0]; - var newViewOptions = _editorOptionsFactory.GetOptions(newTrackedView); - Assumes.Present(newViewOptions); - - // We assume the TextViews all have the same TextBuffer, so we can reuse the - // buffer options from the old TextView. - var newOptionsTracker = new RazorEditorOptionsTracker( - newTrackedView, newViewOptions, optionsTracker.BufferOptions); - _textBuffer.Properties[typeof(RazorEditorOptionsTracker)] = newOptionsTracker; - - newViewOptions.OptionChanged += RazorOptions_OptionChanged; - } + var newTrackedView = _activeTextViews[0]; + var newViewOptions = _editorOptionsFactory.GetOptions(newTrackedView); + Assumes.Present(newViewOptions); + + // We assume the TextViews all have the same TextBuffer, so we can reuse the + // buffer options from the old TextView. + var newOptionsTracker = new RazorEditorOptionsTracker( + newTrackedView, newViewOptions, optionsTracker.BufferOptions); + _textBuffer.Properties[typeof(RazorEditorOptionsTracker)] = newOptionsTracker; + + newViewOptions.OptionChanged += RazorOptions_OptionChanged; } } } - } #pragma warning disable VSTHRD100 // Avoid async void methods