Merge pull request #34212 from JoeRobich/fix-naming-options

Fix saving naming styles options page

src/Tools/ILAsm/IlAsmDeploy.csproj

@@ -12,9 +12,6 @@
     <TargetLatestRuntimePatch>false</TargetLatestRuntimePatch>
     <SelfContained>true</SelfContained>
 
-    <!-- Do not build again when publishing -->
-    <NoBuild>true</NoBuild>
-
     <PublishDir>$(ArtifactsDir)tools\ILAsm\$(Configuration)\</PublishDir>
   </PropertyGroup>
 

src/VisualStudio/Core/Impl/Options/AbstractOptionPage.cs

@@ -12,7 +12,9 @@ internal abstract class AbstractOptionPage : UIElementDialogPage
     {
         private static IOptionService s_optionService;
         private static OptionStore s_optionStore;
-        private static bool s_needsLoadOnNextActivate = true;
+        private static bool s_needsToUpdateOptionStore = true;
+
+        private bool _needsLoadOnNextActivate = true;
 
         protected abstract AbstractOptionPageControl CreateOptionPage(IServiceProvider serviceProvider, OptionStore optionStore);
 
@@ -50,16 +52,22 @@ protected override void OnActivate(System.ComponentModel.CancelEventArgs e)
         {
             EnsureOptionPageCreated();
 
-            if (s_needsLoadOnNextActivate)
+            if (s_needsToUpdateOptionStore)
             {
                 // Reset the option store to the current state of the options.
                 s_optionStore.SetOptions(s_optionService.GetOptions());
                 s_optionStore.SetRegisteredOptions(s_optionService.GetRegisteredOptions());
 
-                s_needsLoadOnNextActivate = false;
+                s_needsToUpdateOptionStore = false;
             }
 
-            pageControl.LoadSettings();
+            if (_needsLoadOnNextActivate)
+            {
+                // For pages that don't use option bindings we need to load setting changes.
+                pageControl.OnLoad();
+
+                _needsLoadOnNextActivate = false;
+            }
         }
 
         public override void LoadSettingsFromStorage()
@@ -81,19 +89,36 @@ public override void LoadSettingsFromStorage()
             // they may have been changed programmatically. Therefore, we'll set a flag so we load
             // next time
 
-            s_needsLoadOnNextActivate = pageControl != null;
+            // When pageControl is null we know that Activation has not happened for this page.
+            // We only need to update the OptionStore after a cancel or close click (2).
+            s_needsToUpdateOptionStore = pageControl != null;
+
+            // For pages that don't use option bindings we need to load settings when it is
+            // activated next.
+            _needsLoadOnNextActivate = true;
         }
 
         public override void SaveSettingsToStorage()
         {
             EnsureOptionPageCreated();
 
+            // Allow page controls to perist their settings to the option store before updating the
+            // option service.
+            pageControl.OnSave();
+
             // Save the changes that were accumulated in the option store.
-            s_optionService.SetOptions(s_optionStore.GetOptions());
+            var oldOptions = s_optionService.GetOptions();
+            var newOptions = s_optionStore.GetOptions();
+            s_optionService.SetOptions(newOptions);
+            OptionLogger.Log(oldOptions, newOptions);
 
             // Make sure we load the next time a page is activated, in case that options changed
             // programmatically between now and the next time the page is activated
-            s_needsLoadOnNextActivate = true;
+            s_needsToUpdateOptionStore = true;
+
+            // For pages that don't use option bindings we need to load settings when it is
+            // activated next.
+            _needsLoadOnNextActivate = true;
         }
 
         protected override void OnClosed(EventArgs e)

src/VisualStudio/Core/Impl/Options/AbstractOptionPageControl.cs

@@ -148,7 +148,7 @@ protected void BindToFullSolutionAnalysisOption(CheckBox checkbox, string langua
             _bindingExpressions.Add(bindingExpression);
         }
 
-        internal virtual void LoadSettings()
+        internal virtual void OnLoad()
         {
             foreach (var bindingExpression in _bindingExpressions)
             {
@@ -156,17 +156,8 @@ internal virtual void LoadSettings()
             }
         }
 
-        internal virtual void SaveSettings()
+        internal virtual void OnSave()
         {
-            foreach (var bindingExpression in _bindingExpressions)
-            {
-                if (!bindingExpression.IsDirty)
-                {
-                    continue;
-                }
-
-                bindingExpression.UpdateSource();
-            }
         }
 
         internal virtual void Close()

src/VisualStudio/Core/Impl/Options/GridOptionPreviewControl.xaml.cs

@@ -86,7 +86,7 @@ private void Options_PreviewKeyDown(object sender, KeyEventArgs e)
             }
         }
 
-        internal override void LoadSettings()
+        internal override void OnLoad()
         {
             this.ViewModel = _createViewModel(OptionStore, _serviceProvider);
 

src/VisualStudio/Core/Impl/Options/OptionPreviewControl.xaml.cs

@@ -87,7 +87,7 @@ private void Options_PreviewKeyDown(object sender, KeyEventArgs e)
             }
         }
 
-        internal override void LoadSettings()
+        internal override void OnLoad()
         {
             this.ViewModel = _createViewModel(this.OptionStore, _serviceProvider);
 

src/VisualStudio/Core/Impl/Options/OptionStore.cs

-using System;
+// Copyright (c) Microsoft.  All Rights Reserved.  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 Microsoft.CodeAnalysis.Options;
 
@@ -28,27 +30,21 @@ public OptionStore(OptionSet optionSet, IEnumerable<IOption> registeredOptions)
 
         public void SetOption(OptionKey optionKey, object value)
         {
-            var oldOptions = _optionSet;
             _optionSet = _optionSet.WithChangedOption(optionKey, value);
-            OptionLogger.Log(oldOptions, _optionSet);
 
             OnOptionChanged(optionKey);
         }
 
         public void SetOption<T>(Option<T> option, T value)
         {
-            var oldOptions = _optionSet;
             _optionSet = _optionSet.WithChangedOption(option, value);
-            OptionLogger.Log(oldOptions, _optionSet);
 
             OnOptionChanged(new OptionKey(option));
         }
 
         public void SetOption<T>(PerLanguageOption<T> option, string language, T value)
         {
-            var oldOptionSet = _optionSet;
             _optionSet = _optionSet.WithChangedOption(option, language, value);
-            OptionLogger.Log(oldOptionSet, _optionSet);
 
             OnOptionChanged(new OptionKey(option, language));
         }

src/VisualStudio/Core/Impl/Options/Style/NamingPreferences/NamingStyleOptionPageControl.xaml.cs

@@ -50,7 +50,7 @@ internal NamingStyleOptionPageControl(OptionStore optionStore, INotificationServ
             _notificationService = notificationService;
 
             InitializeComponent();
-            LoadSettings();
+            OnLoad();
         }
 
         private NamingStyleOptionPageViewModel.NamingRuleViewModel CreateItemWithNoSelections()
@@ -139,7 +139,7 @@ private void SetFocusToSelectedRow()
             }
         }
 
-        internal override void SaveSettings()
+        internal override void OnSave()
         {
             var symbolSpecifications = ArrayBuilder<SymbolSpecification>.GetInstance();
             var namingRules = ArrayBuilder<SerializableNamingRule>.GetInstance();
@@ -177,15 +177,12 @@ internal override void SaveSettings()
                 namingStyles.ToImmutableAndFree(),
                 namingRules.ToImmutableAndFree());
 
-            var oldOptions = OptionStore.GetOptions();
-            var newOptions = oldOptions.WithChangedOption(SimplificationOptions.NamingPreferences, _languageName, info);
-            OptionStore.SetOptions(newOptions);
-            OptionLogger.Log(oldOptions, newOptions);
+            OptionStore.SetOption(SimplificationOptions.NamingPreferences, _languageName, info);
         }
 
-        internal override void LoadSettings()
+        internal override void OnLoad()
         {
-            base.LoadSettings();
+            base.OnLoad();
 
             var preferences = OptionStore.GetOption(SimplificationOptions.NamingPreferences, _languageName);
             if (preferences == null)