From 6e57b2435ae86dd5376c422cfb92fc60e1070334 Mon Sep 17 00:00:00 2001 From: tmat Date: Thu, 6 Jun 2024 13:03:27 -0700 Subject: [PATCH 1/3] Change global option change notification to batch all option changes --- ...mbeddedClassificationViewTaggerProvider.cs | 4 +-- .../InlineHintsDataTaggerProvider.cs | 25 +++++++++--------- ...erEventSources.OptionChangedEventSource.cs | 12 ++++----- .../EventSources/TaggerEventSources.cs | 5 ++-- .../AbstractStructureTaggerProvider.cs | 17 ++++++------ ...actAsynchronousTaggerProvider.TagSource.cs | 13 ++++------ .../Diagnostics/DiagnosticAnalyzerService.cs | 3 ++- .../Handler/CodeLens/CodeLensRefreshQueue.cs | 4 +-- .../InlayHint/InlayHintRefreshQueue.cs | 25 +++++++++--------- .../InheritanceMarginTaggerProvider.cs | 11 ++++---- .../InheritanceMarginViewMargin.cs | 5 ++-- ...ctLanguageService`2.VsCodeWindowManager.cs | 3 +-- .../StackTraceExplorerCommandHandler.cs | 19 ++++++++------ .../Core/Def/Telemetry/FileLogger.cs | 11 +++++--- .../OptionPages/ForceLowMemoryMode.cs | 2 +- .../Portable/Options/GlobalOptionService.cs | 26 ++++++++----------- .../Options/OptionChangedEventArgs.cs | 16 ++++-------- .../GlobalOptionServiceTests.cs | 6 ++--- 18 files changed, 102 insertions(+), 105 deletions(-) diff --git a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs index c49b5083fd4ba..b26cd16e6f34a 100644 --- a/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs +++ b/src/EditorFeatures/Core/Classification/Semantic/AbstractSemanticOrEmbeddedClassificationViewTaggerProvider.cs @@ -77,8 +77,8 @@ protected sealed override ITaggerEventSource CreateEventSource(ITextView textVie TaggerEventSources.OnViewSpanChanged(ThreadingContext, textView), TaggerEventSources.OnWorkspaceChanged(subjectBuffer, AsyncListener), TaggerEventSources.OnDocumentActiveContextChanged(subjectBuffer), - TaggerEventSources.OnGlobalOptionChanged(this.GlobalOptions, ClassificationOptionsStorage.ClassifyReassignedVariables), - TaggerEventSources.OnGlobalOptionChanged(this.GlobalOptions, ClassificationOptionsStorage.ClassifyObsoleteSymbols)); + TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, + static option => option.Equals(ClassificationOptionsStorage.ClassifyReassignedVariables) || option.Equals(ClassificationOptionsStorage.ClassifyObsoleteSymbols))); } protected sealed override async Task ProduceTagsAsync( diff --git a/src/EditorFeatures/Core/InlineHints/InlineHintsDataTaggerProvider.cs b/src/EditorFeatures/Core/InlineHints/InlineHintsDataTaggerProvider.cs index c2698831c8cb9..113e4e37f2d5c 100644 --- a/src/EditorFeatures/Core/InlineHints/InlineHintsDataTaggerProvider.cs +++ b/src/EditorFeatures/Core/InlineHints/InlineHintsDataTaggerProvider.cs @@ -58,18 +58,19 @@ protected override ITaggerEventSource CreateEventSource(ITextView textView, ITex TaggerEventSources.OnViewSpanChanged(this.ThreadingContext, textView), TaggerEventSources.OnWorkspaceChanged(subjectBuffer, this.AsyncListener), new InlineHintKeyProcessorEventSource(_inlineHintKeyProcessor), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InlineHintsOptionsStorage.EnabledForParameters), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InlineHintsOptionsStorage.ForLiteralParameters), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InlineHintsOptionsStorage.ForIndexerParameters), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InlineHintsOptionsStorage.ForObjectCreationParameters), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InlineHintsOptionsStorage.ForOtherParameters), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InlineHintsOptionsStorage.SuppressForParametersThatMatchMethodIntent), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InlineHintsOptionsStorage.SuppressForParametersThatDifferOnlyBySuffix), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InlineHintsOptionsStorage.SuppressForParametersThatMatchArgumentName), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InlineHintsOptionsStorage.EnabledForTypes), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InlineHintsOptionsStorage.ForImplicitVariableTypes), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InlineHintsOptionsStorage.ForLambdaParameterTypes), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InlineHintsOptionsStorage.ForImplicitObjectCreation)); + TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, static option => + option.Equals(InlineHintsOptionsStorage.EnabledForParameters) || + option.Equals(InlineHintsOptionsStorage.ForLiteralParameters) || + option.Equals(InlineHintsOptionsStorage.ForIndexerParameters) || + option.Equals(InlineHintsOptionsStorage.ForObjectCreationParameters) || + option.Equals(InlineHintsOptionsStorage.ForOtherParameters) || + option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatMatchMethodIntent) || + option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatDifferOnlyBySuffix) || + option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatMatchArgumentName) || + option.Equals(InlineHintsOptionsStorage.EnabledForTypes) || + option.Equals(InlineHintsOptionsStorage.ForImplicitVariableTypes) || + option.Equals(InlineHintsOptionsStorage.ForLambdaParameterTypes) || + option.Equals(InlineHintsOptionsStorage.ForImplicitObjectCreation))); } protected override void AddSpansToTag(ITextView? textView, ITextBuffer subjectBuffer, ref TemporaryArray result) diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.OptionChangedEventSource.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.OptionChangedEventSource.cs index 168096760c723..5cf4fdc09cbd7 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.OptionChangedEventSource.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.OptionChangedEventSource.cs @@ -2,30 +2,28 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using Microsoft.CodeAnalysis.Options; namespace Microsoft.CodeAnalysis.Editor.Shared.Tagging; internal partial class TaggerEventSources { - private sealed class GlobalOptionChangedEventSource(IGlobalOptionService globalOptions, IOption2 globalOption) : AbstractTaggerEventSource + private sealed class GlobalOptionChangedEventSource(IGlobalOptionService globalOptions, Func predicate) : AbstractTaggerEventSource { - private readonly IOption2 _globalOption = globalOption; - private readonly IGlobalOptionService _globalOptions = globalOptions; - public override void Connect() { - _globalOptions.AddOptionChangedHandler(this, OnGlobalOptionChanged); + globalOptions.AddOptionChangedHandler(this, OnGlobalOptionChanged); } public override void Disconnect() { - _globalOptions.RemoveOptionChangedHandler(this, OnGlobalOptionChanged); + globalOptions.RemoveOptionChangedHandler(this, OnGlobalOptionChanged); } private void OnGlobalOptionChanged(object? sender, OptionChangedEventArgs e) { - if (e.Option == _globalOption) + if (e.HasOption(predicate)) { RaiseChanged(); } diff --git a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.cs b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.cs index 8e5c1a870a0c5..eb7393b38a54d 100644 --- a/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.cs +++ b/src/EditorFeatures/Core/Shared/Tagging/EventSources/TaggerEventSources.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System; using System.Collections.Generic; using System.Linq; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; @@ -45,8 +46,8 @@ public static ITaggerEventSource OnSelectionChanged(ITextView textView) public static ITaggerEventSource OnReadOnlyRegionsChanged(ITextBuffer subjectBuffer) => new ReadOnlyRegionsChangedEventSource(subjectBuffer); - public static ITaggerEventSource OnGlobalOptionChanged(IGlobalOptionService globalOptions, IOption2 globalOption) - => new GlobalOptionChangedEventSource(globalOptions, globalOption); + public static ITaggerEventSource OnGlobalOptionChanged(IGlobalOptionService globalOptions, Func predicate) + => new GlobalOptionChangedEventSource(globalOptions, predicate); public static ITaggerEventSource OnParseOptionChanged(ITextBuffer subjectBuffer) => new ParseOptionChangedEventSource(subjectBuffer); diff --git a/src/EditorFeatures/Core/Structure/AbstractStructureTaggerProvider.cs b/src/EditorFeatures/Core/Structure/AbstractStructureTaggerProvider.cs index 5aa0029ef42e0..0244ed956c537 100644 --- a/src/EditorFeatures/Core/Structure/AbstractStructureTaggerProvider.cs +++ b/src/EditorFeatures/Core/Structure/AbstractStructureTaggerProvider.cs @@ -157,14 +157,15 @@ protected sealed override ITaggerEventSource CreateEventSource(ITextView? textVi TaggerEventSources.OnTextChanged(subjectBuffer), TaggerEventSources.OnParseOptionChanged(subjectBuffer), TaggerEventSources.OnWorkspaceRegistrationChanged(subjectBuffer), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, BlockStructureOptionsStorage.ShowBlockStructureGuidesForCodeLevelConstructs), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, BlockStructureOptionsStorage.ShowBlockStructureGuidesForDeclarationLevelConstructs), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, BlockStructureOptionsStorage.ShowBlockStructureGuidesForCommentsAndPreprocessorRegions), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, BlockStructureOptionsStorage.ShowOutliningForCodeLevelConstructs), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, BlockStructureOptionsStorage.ShowOutliningForDeclarationLevelConstructs), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, BlockStructureOptionsStorage.ShowOutliningForCommentsAndPreprocessorRegions), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, BlockStructureOptionsStorage.CollapseRegionsWhenCollapsingToDefinitions), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, BlockStructureOptionsStorage.CollapseLocalFunctionsWhenCollapsingToDefinitions)); + TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, static option => + option.Equals(BlockStructureOptionsStorage.ShowBlockStructureGuidesForCodeLevelConstructs) || + option.Equals(BlockStructureOptionsStorage.ShowBlockStructureGuidesForDeclarationLevelConstructs) || + option.Equals(BlockStructureOptionsStorage.ShowBlockStructureGuidesForCommentsAndPreprocessorRegions) || + option.Equals(BlockStructureOptionsStorage.ShowOutliningForCodeLevelConstructs) || + option.Equals(BlockStructureOptionsStorage.ShowOutliningForDeclarationLevelConstructs) || + option.Equals(BlockStructureOptionsStorage.ShowOutliningForCommentsAndPreprocessorRegions) || + option.Equals(BlockStructureOptionsStorage.CollapseRegionsWhenCollapsingToDefinitions) || + option.Equals(BlockStructureOptionsStorage.CollapseLocalFunctionsWhenCollapsingToDefinitions))); } protected sealed override async Task ProduceTagsAsync( diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs index 45e7798431059..cd42e5a18c093 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource.cs @@ -324,18 +324,15 @@ private ITaggerEventSource CreateEventSource() // If there are any options specified for this tagger, then also hook up event // notifications for when those options change. - var optionChangedEventSources = _dataSource.Options.Concat(_dataSource.FeatureOptions) - .Select(globalOption => TaggerEventSources.OnGlobalOptionChanged(_dataSource.GlobalOptions, globalOption)) - .ToList(); - - if (optionChangedEventSources.Count == 0) + if (_dataSource.Options.IsEmpty && _dataSource.FeatureOptions.IsEmpty) { - // No options specified for this tagger. So just keep the event source as is. return eventSource; } - optionChangedEventSources.Add(eventSource); - return TaggerEventSources.Compose(optionChangedEventSources); + return TaggerEventSources.Compose( + eventSource, + TaggerEventSources.OnGlobalOptionChanged(_dataSource.GlobalOptions, option => + _dataSource.Options.Contains(option) || _dataSource.FeatureOptions.Contains(option))); } private void RaiseTagsChanged(ITextBuffer buffer, DiffResult difference) diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs b/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs index e183dba40f013..b850e46ce9367 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/DiagnosticAnalyzerService.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; +using System.Linq; using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; @@ -51,7 +52,7 @@ public DiagnosticAnalyzerService( globalOptions.AddOptionChangedHandler(this, (_, e) => { - if (IsGlobalOptionAffectingDiagnostics(e.Option)) + if (e.HasOption(IsGlobalOptionAffectingDiagnostics)) { RequestDiagnosticRefresh(); } diff --git a/src/LanguageServer/Protocol/Handler/CodeLens/CodeLensRefreshQueue.cs b/src/LanguageServer/Protocol/Handler/CodeLens/CodeLensRefreshQueue.cs index 0557fab1300f0..668d2b4cb0ec0 100644 --- a/src/LanguageServer/Protocol/Handler/CodeLens/CodeLensRefreshQueue.cs +++ b/src/LanguageServer/Protocol/Handler/CodeLens/CodeLensRefreshQueue.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Linq; using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Shared.TestHooks; using Roslyn.LanguageServer.Protocol; @@ -39,8 +40,7 @@ protected override string GetWorkspaceRefreshName() private void OnOptionChanged(object? sender, OptionChangedEventArgs e) { - if (e.Option.Equals(LspOptionsStorage.LspEnableReferencesCodeLens) || - e.Option.Equals(LspOptionsStorage.LspEnableTestsCodeLens)) + if (e.HasOption(static option => option.Equals(LspOptionsStorage.LspEnableReferencesCodeLens) || option.Equals(LspOptionsStorage.LspEnableTestsCodeLens))) { EnqueueRefreshNotification(documentUri: null); } diff --git a/src/LanguageServer/Protocol/Handler/InlayHint/InlayHintRefreshQueue.cs b/src/LanguageServer/Protocol/Handler/InlayHint/InlayHintRefreshQueue.cs index eef77631f06db..03f9112ec4224 100644 --- a/src/LanguageServer/Protocol/Handler/InlayHint/InlayHintRefreshQueue.cs +++ b/src/LanguageServer/Protocol/Handler/InlayHint/InlayHintRefreshQueue.cs @@ -33,18 +33,19 @@ public override void Dispose() private void OnOptionChanged(object? sender, OptionChangedEventArgs e) { - if (e.Option.Equals(InlineHintsOptionsStorage.EnabledForParameters) || - e.Option.Equals(InlineHintsOptionsStorage.ForIndexerParameters) || - e.Option.Equals(InlineHintsOptionsStorage.ForLiteralParameters) || - e.Option.Equals(InlineHintsOptionsStorage.ForOtherParameters) || - e.Option.Equals(InlineHintsOptionsStorage.ForObjectCreationParameters) || - e.Option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatDifferOnlyBySuffix) || - e.Option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatMatchArgumentName) || - e.Option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatMatchMethodIntent) || - e.Option.Equals(InlineHintsOptionsStorage.EnabledForTypes) || - e.Option.Equals(InlineHintsOptionsStorage.ForImplicitVariableTypes) || - e.Option.Equals(InlineHintsOptionsStorage.ForLambdaParameterTypes) || - e.Option.Equals(InlineHintsOptionsStorage.ForImplicitObjectCreation)) + if (e.HasOption(static option => + option.Equals(InlineHintsOptionsStorage.EnabledForParameters) || + option.Equals(InlineHintsOptionsStorage.ForIndexerParameters) || + option.Equals(InlineHintsOptionsStorage.ForLiteralParameters) || + option.Equals(InlineHintsOptionsStorage.ForOtherParameters) || + option.Equals(InlineHintsOptionsStorage.ForObjectCreationParameters) || + option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatDifferOnlyBySuffix) || + option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatMatchArgumentName) || + option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatMatchMethodIntent) || + option.Equals(InlineHintsOptionsStorage.EnabledForTypes) || + option.Equals(InlineHintsOptionsStorage.ForImplicitVariableTypes) || + option.Equals(InlineHintsOptionsStorage.ForLambdaParameterTypes) || + option.Equals(InlineHintsOptionsStorage.ForImplicitObjectCreation))) { EnqueueRefreshNotification(documentUri: null); } diff --git a/src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginTaggerProvider.cs b/src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginTaggerProvider.cs index 7bc41a0566f16..3280ed09b4711 100644 --- a/src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginTaggerProvider.cs +++ b/src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginTaggerProvider.cs @@ -59,11 +59,12 @@ protected override ITaggerEventSource CreateEventSource(ITextView textView, ITex // Note: Also generate tags when InheritanceMarginOptions.InheritanceMarginCombinedWithIndicatorMargin is changed, // because we want to refresh the glyphs in indicator margin. return TaggerEventSources.Compose( - TaggerEventSources.OnWorkspaceChanged(subjectBuffer, AsyncListener), - TaggerEventSources.OnViewSpanChanged(ThreadingContext, textView), - TaggerEventSources.OnDocumentActiveContextChanged(subjectBuffer), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InheritanceMarginOptionsStorage.ShowInheritanceMargin), - TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, InheritanceMarginOptionsStorage.InheritanceMarginCombinedWithIndicatorMargin)); + TaggerEventSources.OnWorkspaceChanged(subjectBuffer, AsyncListener), + TaggerEventSources.OnViewSpanChanged(ThreadingContext, textView), + TaggerEventSources.OnDocumentActiveContextChanged(subjectBuffer), + TaggerEventSources.OnGlobalOptionChanged(GlobalOptions, static option => + option.Equals(InheritanceMarginOptionsStorage.ShowInheritanceMargin) || + option.Equals(InheritanceMarginOptionsStorage.InheritanceMarginCombinedWithIndicatorMargin))); } protected override async Task ProduceTagsAsync( diff --git a/src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginViewMargin.cs b/src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginViewMargin.cs index ce2aa66a4f75e..71e3b7665d28c 100644 --- a/src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginViewMargin.cs +++ b/src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginViewMargin.cs @@ -124,8 +124,9 @@ private void OnLayoutChanged(object sender, TextViewLayoutChangedEventArgs e) private void OnGlobalOptionChanged(object sender, OptionChangedEventArgs e) { - if (e.Option.Equals(InheritanceMarginOptionsStorage.ShowInheritanceMargin) || - e.Option.Equals(InheritanceMarginOptionsStorage.InheritanceMarginCombinedWithIndicatorMargin)) + if (e.HasOption(option => + option.Equals(InheritanceMarginOptionsStorage.ShowInheritanceMargin) || + option.Equals(InheritanceMarginOptionsStorage.InheritanceMarginCombinedWithIndicatorMargin))) { UpdateMarginVisibility(); } diff --git a/src/VisualStudio/Core/Def/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs b/src/VisualStudio/Core/Def/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs index b1448c16788a1..5156731843ab5 100644 --- a/src/VisualStudio/Core/Def/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs +++ b/src/VisualStudio/Core/Def/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs @@ -59,8 +59,7 @@ private void SetupView(IVsTextView view) private void GlobalOptionChanged(object sender, OptionChangedEventArgs e) { - if (e.Language != _languageService.RoslynLanguageName || - e.Option != NavigationBarViewOptionsStorage.ShowNavigationBar) + if (e.ChangedOptions.Any(item => item.key.Language == _languageService.RoslynLanguageName && item.key.Option == NavigationBarViewOptionsStorage.ShowNavigationBar)) { return; } diff --git a/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerCommandHandler.cs b/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerCommandHandler.cs index 3736d31906504..cf646956fb9a4 100644 --- a/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerCommandHandler.cs +++ b/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerCommandHandler.cs @@ -4,6 +4,7 @@ using System; using System.ComponentModel.Design; +using System.Linq; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.Options; @@ -107,16 +108,18 @@ private void UnadviseBroadcastMessages() private void GlobalOptionChanged(object sender, OptionChangedEventArgs e) { - if (e.Option == StackTraceExplorerOptionsStorage.OpenOnFocus && e.Value is not null) + foreach (var (key, newValue) in e.ChangedOptions) { - var enabled = (bool)e.Value; - if (enabled) + if (key.Option.Equals(StackTraceExplorerOptionsStorage.OpenOnFocus) && newValue is bool enabled) { - AdviseBroadcastMessages(); - } - else - { - UnadviseBroadcastMessages(); + if (enabled) + { + AdviseBroadcastMessages(); + } + else + { + UnadviseBroadcastMessages(); + } } } } diff --git a/src/VisualStudio/Core/Def/Telemetry/FileLogger.cs b/src/VisualStudio/Core/Def/Telemetry/FileLogger.cs index a731aa1c059cb..d4c30090efa66 100644 --- a/src/VisualStudio/Core/Def/Telemetry/FileLogger.cs +++ b/src/VisualStudio/Core/Def/Telemetry/FileLogger.cs @@ -5,6 +5,7 @@ using System; using System.Globalization; using System.IO; +using System.Linq; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -52,11 +53,13 @@ private static string GetLogFileName() private void OptionService_OptionChanged(object? sender, OptionChangedEventArgs e) { - if (e.Option == VisualStudioLoggingOptionsStorage.EnableFileLoggingForDiagnostics) + foreach (var (key, newValue) in e.ChangedOptions) { - Contract.ThrowIfNull(e.Value); - - _enabled = (bool)e.Value; + if (key.Option.Equals(VisualStudioLoggingOptionsStorage.EnableFileLoggingForDiagnostics)) + { + Contract.ThrowIfNull(newValue); + _enabled = (bool)newValue; + } } } diff --git a/src/VisualStudio/VisualStudioDiagnosticsToolWindow/OptionPages/ForceLowMemoryMode.cs b/src/VisualStudio/VisualStudioDiagnosticsToolWindow/OptionPages/ForceLowMemoryMode.cs index 3f3ec91de193a..57e0625acd832 100644 --- a/src/VisualStudio/VisualStudioDiagnosticsToolWindow/OptionPages/ForceLowMemoryMode.cs +++ b/src/VisualStudio/VisualStudioDiagnosticsToolWindow/OptionPages/ForceLowMemoryMode.cs @@ -29,7 +29,7 @@ public ForceLowMemoryMode(IGlobalOptionService globalOptions) private void Options_OptionChanged(object sender, OptionChangedEventArgs e) { - if (e.Option == Enabled || e.Option == SizeInMegabytes) + if (e.HasOption(static option => option.Equals(Enabled) || option.Equals(SizeInMegabytes))) { RefreshFromSettings(); } diff --git a/src/Workspaces/Core/Portable/Options/GlobalOptionService.cs b/src/Workspaces/Core/Portable/Options/GlobalOptionService.cs index 5e498abdb06c7..09e32f60735a6 100644 --- a/src/Workspaces/Core/Portable/Options/GlobalOptionService.cs +++ b/src/Workspaces/Core/Portable/Options/GlobalOptionService.cs @@ -10,6 +10,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host.Mef; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.Utilities; using Roslyn.Utilities; @@ -187,7 +188,7 @@ public bool SetGlobalOptions(ImmutableArray> o private bool SetGlobalOptions(OneOrMany> options) { - var changedOptions = new List(); + var changedOptions = new ArrayBuilder<(OptionKey2, object?)>(options.Count); var persisters = GetOptionPersisters(); lock (_gate) @@ -202,23 +203,23 @@ private bool SetGlobalOptions(OneOrMany> optio } currentValues = currentValues.SetItem(optionKey, value); - changedOptions.Add(new OptionChangedEventArgs(optionKey, value)); + changedOptions.Add((optionKey, value)); } _currentValues = currentValues; } - if (changedOptions.Count == 0) + if (changedOptions.IsEmpty) { return false; } - foreach (var changedOption in changedOptions) + foreach (var (key, value) in changedOptions) { - PersistOption(persisters, changedOption.OptionKey, changedOption.Value); + PersistOption(persisters, key, value); } - RaiseOptionChangedEvent(changedOptions); + RaiseOptionChangedEvent(changedOptions.ToImmutable()); return true; } @@ -249,8 +250,7 @@ public bool RefreshOption(OptionKey2 optionKey, object? newValue) _currentValues = _currentValues.SetItem(optionKey, newValue); } - var changedOptions = new List { new OptionChangedEventArgs(optionKey, newValue) }; - RaiseOptionChangedEvent(changedOptions); + RaiseOptionChangedEvent([(optionKey, newValue)]); return true; } @@ -264,14 +264,10 @@ public void RemoveOptionChangedHandler(object target, EventHandler changedOptions) + private void RaiseOptionChangedEvent(ImmutableArray<(OptionKey2, object?)> changedOptions) { - Debug.Assert(changedOptions.Count > 0); - - foreach (var changedOption in changedOptions) - { - _optionChanged.RaiseEvent(this, changedOption); - } + Debug.Assert(!changedOptions.IsEmpty); + _optionChanged.RaiseEvent(this, new OptionChangedEventArgs(changedOptions)); } internal TestAccessor GetTestAccessor() diff --git a/src/Workspaces/Core/Portable/Options/OptionChangedEventArgs.cs b/src/Workspaces/Core/Portable/Options/OptionChangedEventArgs.cs index 9d7857ac89243..3d2381312b72a 100644 --- a/src/Workspaces/Core/Portable/Options/OptionChangedEventArgs.cs +++ b/src/Workspaces/Core/Portable/Options/OptionChangedEventArgs.cs @@ -3,20 +3,14 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Immutable; namespace Microsoft.CodeAnalysis.Options; -internal sealed class OptionChangedEventArgs : EventArgs +internal sealed class OptionChangedEventArgs(ImmutableArray<(OptionKey2 key, object? newValue)> changedOptions) : EventArgs { - public OptionKey2 OptionKey { get; } - public object? Value { get; } + public ImmutableArray<(OptionKey2 key, object? newValue)> ChangedOptions => changedOptions; - internal OptionChangedEventArgs(OptionKey2 optionKey, object? value) - { - OptionKey = optionKey; - Value = value; - } - - public IOption2 Option => OptionKey.Option; - public string? Language => OptionKey.Language; + public bool HasOption(Func predicate) + => changedOptions.Any(static (option, predicate) => predicate(option.key.Option), predicate); } diff --git a/src/Workspaces/CoreTest/WorkspaceServiceTests/GlobalOptionServiceTests.cs b/src/Workspaces/CoreTest/WorkspaceServiceTests/GlobalOptionServiceTests.cs index c34e8b5fa2ff3..ba3600ff18fda 100644 --- a/src/Workspaces/CoreTest/WorkspaceServiceTests/GlobalOptionServiceTests.cs +++ b/src/Workspaces/CoreTest/WorkspaceServiceTests/GlobalOptionServiceTests.cs @@ -170,9 +170,9 @@ public void GlobalOptions() var option2 = new Option2("test_option2", defaultValue: 2); var option3 = new Option2("test_option3", defaultValue: 3); - var changedOptions = new List(); + var events = new List(); - var handler = new EventHandler((_, e) => changedOptions.Add(e)); + var handler = new EventHandler((_, e) => events.Add(e)); globalOptions.AddOptionChangedHandler(this, handler); var values = globalOptions.GetOptions([new OptionKey2(option1), new OptionKey2(option2)]); @@ -190,7 +190,7 @@ public void GlobalOptions() { "test_option1=5", "test_option2=6", - }, changedOptions.Select(e => $"{e.Option.Definition.ConfigName}={e.Value}")); + }, events.Single().ChangedOptions.Select(e => $"{e.key.Option.Definition.ConfigName}={e.newValue}")); values = globalOptions.GetOptions([new OptionKey2(option1), new OptionKey2(option2), new OptionKey2(option3)]); Assert.Equal(5, values[0]); From 524a277eb11a859da55f22592873192e640c5099 Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Thu, 6 Jun 2024 16:54:16 -0700 Subject: [PATCH 2/3] Fix and feedback --- .../InlayHint/InlayHintRefreshQueue.cs | 24 +++++++++--------- .../InheritanceMarginViewMargin.cs | 4 +-- ...ctLanguageService`2.VsCodeWindowManager.cs | 6 ++--- .../StackTraceExplorerCommandHandler.cs | 25 ++++++++++++------- 4 files changed, 32 insertions(+), 27 deletions(-) diff --git a/src/LanguageServer/Protocol/Handler/InlayHint/InlayHintRefreshQueue.cs b/src/LanguageServer/Protocol/Handler/InlayHint/InlayHintRefreshQueue.cs index 03f9112ec4224..887de0f1b7321 100644 --- a/src/LanguageServer/Protocol/Handler/InlayHint/InlayHintRefreshQueue.cs +++ b/src/LanguageServer/Protocol/Handler/InlayHint/InlayHintRefreshQueue.cs @@ -34,18 +34,18 @@ public override void Dispose() private void OnOptionChanged(object? sender, OptionChangedEventArgs e) { if (e.HasOption(static option => - option.Equals(InlineHintsOptionsStorage.EnabledForParameters) || - option.Equals(InlineHintsOptionsStorage.ForIndexerParameters) || - option.Equals(InlineHintsOptionsStorage.ForLiteralParameters) || - option.Equals(InlineHintsOptionsStorage.ForOtherParameters) || - option.Equals(InlineHintsOptionsStorage.ForObjectCreationParameters) || - option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatDifferOnlyBySuffix) || - option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatMatchArgumentName) || - option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatMatchMethodIntent) || - option.Equals(InlineHintsOptionsStorage.EnabledForTypes) || - option.Equals(InlineHintsOptionsStorage.ForImplicitVariableTypes) || - option.Equals(InlineHintsOptionsStorage.ForLambdaParameterTypes) || - option.Equals(InlineHintsOptionsStorage.ForImplicitObjectCreation))) + option.Equals(InlineHintsOptionsStorage.EnabledForParameters) || + option.Equals(InlineHintsOptionsStorage.ForIndexerParameters) || + option.Equals(InlineHintsOptionsStorage.ForLiteralParameters) || + option.Equals(InlineHintsOptionsStorage.ForOtherParameters) || + option.Equals(InlineHintsOptionsStorage.ForObjectCreationParameters) || + option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatDifferOnlyBySuffix) || + option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatMatchArgumentName) || + option.Equals(InlineHintsOptionsStorage.SuppressForParametersThatMatchMethodIntent) || + option.Equals(InlineHintsOptionsStorage.EnabledForTypes) || + option.Equals(InlineHintsOptionsStorage.ForImplicitVariableTypes) || + option.Equals(InlineHintsOptionsStorage.ForLambdaParameterTypes) || + option.Equals(InlineHintsOptionsStorage.ForImplicitObjectCreation))) { EnqueueRefreshNotification(documentUri: null); } diff --git a/src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginViewMargin.cs b/src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginViewMargin.cs index 71e3b7665d28c..fcf176a54600f 100644 --- a/src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginViewMargin.cs +++ b/src/VisualStudio/Core/Def/InheritanceMargin/InheritanceMarginViewMargin.cs @@ -125,8 +125,8 @@ private void OnLayoutChanged(object sender, TextViewLayoutChangedEventArgs e) private void OnGlobalOptionChanged(object sender, OptionChangedEventArgs e) { if (e.HasOption(option => - option.Equals(InheritanceMarginOptionsStorage.ShowInheritanceMargin) || - option.Equals(InheritanceMarginOptionsStorage.InheritanceMarginCombinedWithIndicatorMargin))) + option.Equals(InheritanceMarginOptionsStorage.ShowInheritanceMargin) || + option.Equals(InheritanceMarginOptionsStorage.InheritanceMarginCombinedWithIndicatorMargin))) { UpdateMarginVisibility(); } diff --git a/src/VisualStudio/Core/Def/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs b/src/VisualStudio/Core/Def/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs index 5156731843ab5..1eb6b09afedfa 100644 --- a/src/VisualStudio/Core/Def/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs +++ b/src/VisualStudio/Core/Def/LanguageService/AbstractLanguageService`2.VsCodeWindowManager.cs @@ -59,12 +59,10 @@ private void SetupView(IVsTextView view) private void GlobalOptionChanged(object sender, OptionChangedEventArgs e) { - if (e.ChangedOptions.Any(item => item.key.Language == _languageService.RoslynLanguageName && item.key.Option == NavigationBarViewOptionsStorage.ShowNavigationBar)) + if (e.ChangedOptions.Any(item => item.key.Language == _languageService.RoslynLanguageName && item.key.Option.Equals(NavigationBarViewOptionsStorage.ShowNavigationBar))) { - return; + AddOrRemoveDropdown(); } - - AddOrRemoveDropdown(); } private void AddOrRemoveDropdown() diff --git a/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerCommandHandler.cs b/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerCommandHandler.cs index cf646956fb9a4..278cf673f7066 100644 --- a/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerCommandHandler.cs +++ b/src/VisualStudio/Core/Def/StackTraceExplorer/StackTraceExplorerCommandHandler.cs @@ -108,18 +108,25 @@ private void UnadviseBroadcastMessages() private void GlobalOptionChanged(object sender, OptionChangedEventArgs e) { + bool? enabled = null; foreach (var (key, newValue) in e.ChangedOptions) { - if (key.Option.Equals(StackTraceExplorerOptionsStorage.OpenOnFocus) && newValue is bool enabled) + if (key.Option.Equals(StackTraceExplorerOptionsStorage.OpenOnFocus)) { - if (enabled) - { - AdviseBroadcastMessages(); - } - else - { - UnadviseBroadcastMessages(); - } + Contract.ThrowIfNull(newValue); + enabled = (bool)newValue; + } + } + + if (enabled.HasValue) + { + if (enabled.Value) + { + AdviseBroadcastMessages(); + } + else + { + UnadviseBroadcastMessages(); } } } From 853b06888d34672c48f2a1f5bc5a81d06b6f0511 Mon Sep 17 00:00:00 2001 From: Tomas Matousek Date: Thu, 6 Jun 2024 17:47:49 -0700 Subject: [PATCH 3/3] Batch option changes in LSP --- ...dChangeConfigurationNotificationHandler.cs | 33 ++++++++----------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/src/LanguageServer/Protocol/Handler/Configuration/DidChangeConfigurationNotificationHandler.cs b/src/LanguageServer/Protocol/Handler/Configuration/DidChangeConfigurationNotificationHandler.cs index f87923246fbbf..60137a3e1ac92 100644 --- a/src/LanguageServer/Protocol/Handler/Configuration/DidChangeConfigurationNotificationHandler.cs +++ b/src/LanguageServer/Protocol/Handler/Configuration/DidChangeConfigurationNotificationHandler.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics; using System.Text.Json.Nodes; using System.Threading; using System.Threading.Tasks; @@ -84,34 +85,28 @@ private async Task RefreshOptionsAsync(CancellationToken cancellationToken) RoslynDebug.Assert(configurationsFromClient.Length == SupportedOptions.Sum(option => option is IPerLanguageValuedOption ? 2 : 1)); // LSP ensures the order of result from client should match the order we sent from server. + var optionsToUpdate = new ArrayBuilder>(); + for (var i = 0; i < configurationsFromClient.Length; i++) { var valueFromClient = configurationsFromClient[i]; var (option, languageName) = _optionsAndLanguageNamesToRefresh[i]; + // If option doesn't exist in the client, don't try to update the option. if (!string.IsNullOrEmpty(valueFromClient)) - SetOption(option, valueFromClient, languageName); - } - } - - private void SetOption(IOption2 option, string valueFromClient, string? languageName = null) - { - if (option.Definition.Serializer.TryParse(valueFromClient, out var result)) - { - if (option is IPerLanguageValuedOption && languageName != null) { - _globalOptionService.SetGlobalOption(new OptionKey2(option, language: languageName), result); - } - else - { - RoslynDebug.Assert(languageName == null); - _globalOptionService.SetGlobalOption(new OptionKey2(option, language: null), result); + if (option.Definition.Serializer.TryParse(valueFromClient, out var parsedValue)) + { + optionsToUpdate.Add(KeyValuePairUtil.Create(new OptionKey2(option, language: languageName), parsedValue)); + } + else + { + _lspLogger.LogWarning($"Failed to parse '{valueFromClient}' to type: '{option.Type.Name}'. '{option.Name}' would not be updated."); + } } } - else - { - _lspLogger.LogWarning($"Failed to parse {valueFromClient} to type: {option.Type.Name}. {option.Name} would not be updated."); - } + + _globalOptionService.SetGlobalOptions(optionsToUpdate.ToImmutable()); } private async Task> GetConfigurationsAsync(CancellationToken cancellationToken)