Fix update settings to finally remove the need for settings service singleton

ILSpy/AboutPage.cs

@@ -23,6 +23,7 @@ using System.IO;
 using System.Text.RegularExpressions;
 using System.Windows;
 using System.Windows.Controls;
+using System.Windows.Controls.Primitives;
 using System.Windows.Data;
 using System.Windows.Input;
 using System.Windows.Navigation;
@@ -34,7 +35,6 @@ using ICSharpCode.ILSpy.Properties;
 using ICSharpCode.ILSpy.TextView;
 using ICSharpCode.ILSpy.Themes;
 using ICSharpCode.ILSpy.Updates;
-using ICSharpCode.ILSpyX.Settings;
 
 namespace ICSharpCode.ILSpy
 {
@@ -53,7 +53,7 @@ namespace ICSharpCode.ILSpy
 
 	[Export]
 	[Shared]
-	public sealed class AboutPage(IEnumerable<IAboutPageAddition> aboutPageAdditions)
+	public sealed class AboutPage(IEnumerable<IAboutPageAddition> aboutPageAdditions, SettingsService settingsService)
 	{
 		public void Display(DecompilerTextView textView)
 		{
@@ -68,9 +68,10 @@ namespace ICSharpCode.ILSpy
 
 			output.AddUIElement(
 			delegate {
-				StackPanel stackPanel = new StackPanel();
-				stackPanel.HorizontalAlignment = HorizontalAlignment.Center;
-				stackPanel.Orientation = Orientation.Horizontal;
+				StackPanel stackPanel = new() {
+					HorizontalAlignment = HorizontalAlignment.Center,
+					Orientation = Orientation.Horizontal
+				};
 				if (NotifyOfUpdatesStrategy.LatestAvailableVersion == null)
 				{
 					AddUpdateCheckButton(stackPanel, textView);
@@ -80,11 +81,13 @@ namespace ICSharpCode.ILSpy
 					// we already retrieved the latest version sometime earlier
 					ShowAvailableVersion(NotifyOfUpdatesStrategy.LatestAvailableVersion, stackPanel);
 				}
-				CheckBox checkBox = new CheckBox();
-				checkBox.Margin = new Thickness(4);
-				checkBox.Content = Resources.AutomaticallyCheckUpdatesEveryWeek;
-				UpdateSettings settings = new UpdateSettings(SettingsService.Instance.SpySettings);
-				checkBox.SetBinding(CheckBox.IsCheckedProperty, new Binding("AutomaticUpdateCheckEnabled") { Source = settings });
+				CheckBox checkBox = new() {
+					Margin = new Thickness(4),
+					Content = Resources.AutomaticallyCheckUpdatesEveryWeek
+				};
+
+				var settings = settingsService.GetSettings<UpdateSettings>();
+				checkBox.SetBinding(ToggleButton.IsCheckedProperty, new Binding("AutomaticUpdateCheckEnabled") { Source = settings });
 				return new StackPanel {
 					Margin = new Thickness(0, 4, 0, 0),
 					Cursor = Cursors.Arrow,

ILSpy/AssemblyTree/AssemblyTreeModel.cs

@@ -42,6 +42,7 @@ using ICSharpCode.ILSpy.Properties;
 using ICSharpCode.ILSpy.Search;
 using ICSharpCode.ILSpy.TextView;
 using ICSharpCode.ILSpy.TreeNodes;
+using ICSharpCode.ILSpy.Updates;
 using ICSharpCode.ILSpy.ViewModels;
 using ICSharpCode.ILSpyX;
 using ICSharpCode.ILSpyX.Settings;
@@ -169,16 +170,16 @@ namespace ICSharpCode.ILSpy.AssemblyTree
 
 		/// <summary>
 		/// Called on startup or when passed arguments via WndProc from a second instance.
-		/// In the format case, spySettings is non-null; in the latter it is null.
+		/// In the format case, updateSettings is non-null; in the latter it is null.
 		/// </summary>
-		private async Task HandleCommandLineArgumentsAfterShowList(CommandLineArguments args, ISettingsProvider? spySettings = null)
+		private async Task HandleCommandLineArgumentsAfterShowList(CommandLineArguments args, UpdateSettings? updateSettings = null)
 		{
 			var sessionSettings = settingsService.SessionSettings;
 
 			var relevantAssemblies = commandLineLoadedAssemblies.ToList();
 			commandLineLoadedAssemblies.Clear(); // clear references once we don't need them anymore
 
-			await NavigateOnLaunch(args.NavigateTo, sessionSettings.ActiveTreeViewPath, spySettings, relevantAssemblies);
+			await NavigateOnLaunch(args.NavigateTo, sessionSettings.ActiveTreeViewPath, updateSettings, relevantAssemblies);
 
 			if (args.Search != null)
 			{
@@ -207,7 +208,7 @@ namespace ICSharpCode.ILSpy.AssemblyTree
 			});
 		}
 
-		private async Task NavigateOnLaunch(string? navigateTo, string[]? activeTreeViewPath, ISettingsProvider? spySettings, List<LoadedAssembly> relevantAssemblies)
+		private async Task NavigateOnLaunch(string? navigateTo, string[]? activeTreeViewPath, UpdateSettings? updateSettings, List<LoadedAssembly> relevantAssemblies)
 		{
 			var initialSelection = SelectedItem;
 			if (navigateTo != null)
@@ -277,7 +278,7 @@ namespace ICSharpCode.ILSpy.AssemblyTree
 					SelectNode(asmNode);
 				}
 			}
-			else if (spySettings != null)
+			else if (updateSettings != null)
 			{
 				SharpTreeNode? node = null;
 				if (activeTreeViewPath?.Length > 0)
@@ -300,7 +301,7 @@ namespace ICSharpCode.ILSpy.AssemblyTree
 						SelectNode(node);
 
 						// only if not showing the about page, perform the update check:
-						await App.Current.MainWindow.ShowMessageIfUpdatesAvailableAsync(spySettings);
+						await App.Current.MainWindow.ShowMessageIfUpdatesAvailableAsync(updateSettings);
 					}
 					else
 					{
@@ -399,7 +400,7 @@ namespace ICSharpCode.ILSpy.AssemblyTree
 
 		private async Task OpenAssemblies()
 		{
-			await HandleCommandLineArgumentsAfterShowList(App.CommandLineArguments, settingsService.SpySettings);
+			await HandleCommandLineArgumentsAfterShowList(App.CommandLineArguments, settingsService.GetSettings<UpdateSettings>());
 
 			if (FormatExceptions(App.StartupExceptions.ToArray(), out var output))
 			{

ILSpy/Commands/CheckForUpdatesCommand.cs

@@ -20,6 +20,7 @@
 using System.Composition;
 
 using ICSharpCode.ILSpy.Properties;
+using ICSharpCode.ILSpy.Updates;
 
 namespace ICSharpCode.ILSpy
 {
@@ -29,7 +30,7 @@ namespace ICSharpCode.ILSpy
 	{
 		public override async void Execute(object parameter)
 		{
-			await App.Current.MainWindow.ShowMessageIfUpdatesAvailableAsync(settingsService.SpySettings, forceCheck: true);
+			await App.Current.MainWindow.ShowMessageIfUpdatesAvailableAsync(settingsService.GetSettings<UpdateSettings>(), forceCheck: true);
 		}
 	}
 }

ILSpy/MainWindow.xaml.cs

@@ -152,16 +152,16 @@ namespace ICSharpCode.ILSpy
 
 		string updateAvailableDownloadUrl;
 
-		public async Task ShowMessageIfUpdatesAvailableAsync(ISettingsProvider spySettings, bool forceCheck = false)
+		public async Task ShowMessageIfUpdatesAvailableAsync(UpdateSettings settings, bool forceCheck = false)
 		{
 			string downloadUrl;
 			if (forceCheck)
 			{
-				downloadUrl = await NotifyOfUpdatesStrategy.CheckForUpdatesAsync(spySettings);
+				downloadUrl = await NotifyOfUpdatesStrategy.CheckForUpdatesAsync(settings);
 			}
 			else
 			{
-				downloadUrl = await NotifyOfUpdatesStrategy.CheckForUpdatesIfEnabledAsync(spySettings);
+				downloadUrl = await NotifyOfUpdatesStrategy.CheckForUpdatesIfEnabledAsync(settings);
 			}
 
 			// The Update Panel is only available for NotifyOfUpdatesStrategy, AutoUpdate will have differing UI requirements
@@ -182,7 +182,7 @@ namespace ICSharpCode.ILSpy
 			else
 			{
 				updatePanel.Visibility = Visibility.Collapsed;
-				string downloadUrl = await NotifyOfUpdatesStrategy.CheckForUpdatesAsync(settingsService.SpySettings);
+				string downloadUrl = await NotifyOfUpdatesStrategy.CheckForUpdatesAsync(settingsService.GetSettings<UpdateSettings>());
 				AdjustUpdateUIAfterCheck(downloadUrl, true);
 			}
 		}

ILSpy/Themes/WindowStyleManagerBehavior.cs

@@ -43,7 +43,7 @@ namespace ICSharpCode.ILSpy.Themes
 		{
 			base.OnAttached();
 
-			MessageBus<SettingsChangedEventArgs>.Subscribers += (sender, e) => DisplaySettings_PropertyChanged(sender, e);
+			MessageBus<SettingsChangedEventArgs>.Subscribers += (sender, e) => Settings_PropertyChanged(sender, e);
 
 			_foreground = AssociatedObject.Track(Control.ForegroundProperty);
 			_background = AssociatedObject.Track(Control.BackgroundProperty);
@@ -83,7 +83,7 @@ namespace ICSharpCode.ILSpy.Themes
 			MessageBox.Show(Properties.Resources.SettingsChangeRestartRequired);
 		}
 
-		private void DisplaySettings_PropertyChanged(object sender, PropertyChangedEventArgs e)
+		private void Settings_PropertyChanged(object sender, PropertyChangedEventArgs e)
 		{
 			if (sender is not DisplaySettings displaySettings)
 				return;

ILSpy/Updates/NotifyOfUpdatesStrategy.cs

@@ -54,51 +54,40 @@ namespace ICSharpCode.ILSpy.Updates
 			return LatestAvailableVersion;
 		}
 
-
-
 		/// <summary>
 		/// If automatic update checking is enabled, checks if there are any updates available.
 		/// Returns the download URL if an update is available.
 		/// Returns null if no update is available, or if no check was performed.
 		/// </summary>
-		public static async Task<string> CheckForUpdatesIfEnabledAsync(ISettingsProvider spySettings)
+		public static async Task<string> CheckForUpdatesIfEnabledAsync(UpdateSettings settings)
 		{
-			UpdateSettings s = new UpdateSettings(spySettings);
-
 			// If we're in an MSIX package, updates work differently
-			if (s.AutomaticUpdateCheckEnabled)
-			{
+			if (!settings.AutomaticUpdateCheckEnabled)
+				return null;
+
 			// perform update check if we never did one before;
 			// or if the last check wasn't in the past 7 days
-				if (s.LastSuccessfulUpdateCheck == null
-					|| s.LastSuccessfulUpdateCheck < DateTime.UtcNow.AddDays(-7)
-					|| s.LastSuccessfulUpdateCheck > DateTime.UtcNow)
+			if (settings.LastSuccessfulUpdateCheck == null
+			    || settings.LastSuccessfulUpdateCheck < DateTime.UtcNow.AddDays(-7)
+			    || settings.LastSuccessfulUpdateCheck > DateTime.UtcNow)
 			{
-					return await CheckForUpdateInternal(s).ConfigureAwait(false);
+				return await CheckForUpdateInternal(settings).ConfigureAwait(false);
 			}
-				else
-				{
-					return null;
-				}
-			}
-			else
-			{
+
 			return null;
 		}
-		}
 
-		public static Task<string> CheckForUpdatesAsync(ISettingsProvider spySettings)
+		public static Task<string> CheckForUpdatesAsync(UpdateSettings settings)
 		{
-			UpdateSettings s = new UpdateSettings(spySettings);
-			return CheckForUpdateInternal(s);
+			return CheckForUpdateInternal(settings);
 		}
 
-		static async Task<string> CheckForUpdateInternal(UpdateSettings s)
+		static async Task<string> CheckForUpdateInternal(UpdateSettings settings)
 		{
 			try
 			{
 				var v = await GetLatestVersionAsync().ConfigureAwait(false);
-				s.LastSuccessfulUpdateCheck = DateTime.UtcNow;
+				settings.LastSuccessfulUpdateCheck = DateTime.UtcNow;
 				if (v.Version > AppUpdateService.CurrentVersion)
 					return v.DownloadUrl;
 				else

ILSpy/Updates/UpdateSettings.cs

@@ -17,73 +17,56 @@
 // DEALINGS IN THE SOFTWARE.
 
 using System;
-using System.ComponentModel;
 using System.Xml.Linq;
 
-using ICSharpCode.ILSpyX.Settings;
+using TomsToolbox.Wpf;
 
 namespace ICSharpCode.ILSpy.Updates
 {
-	sealed class UpdateSettings : INotifyPropertyChanged
+	public sealed class UpdateSettings : ObservableObjectBase, ISettingsSection
 	{
-		public UpdateSettings(ISettingsProvider spySettings)
-		{
-			XElement s = spySettings["UpdateSettings"];
-			this.automaticUpdateCheckEnabled = (bool?)s.Element("AutomaticUpdateCheckEnabled") ?? true;
-			try
-			{
-				this.lastSuccessfulUpdateCheck = (DateTime?)s.Element("LastSuccessfulUpdateCheck");
-			}
-			catch (FormatException)
-			{
-				// avoid crashing on settings files invalid due to
-				// https://github.com/icsharpcode/ILSpy/issues/closed/#issue/2
-			}
-		}
+		public XName SectionName => "UpdateSettings";
 
 		bool automaticUpdateCheckEnabled;
 
 		public bool AutomaticUpdateCheckEnabled {
-			get { return automaticUpdateCheckEnabled; }
-			set {
-				if (automaticUpdateCheckEnabled != value)
-				{
-					automaticUpdateCheckEnabled = value;
-					Save();
-					OnPropertyChanged(nameof(AutomaticUpdateCheckEnabled));
-				}
-			}
+			get => automaticUpdateCheckEnabled;
+			set => SetProperty(ref automaticUpdateCheckEnabled, value);
 		}
 
 		DateTime? lastSuccessfulUpdateCheck;
 
 		public DateTime? LastSuccessfulUpdateCheck {
-			get { return lastSuccessfulUpdateCheck; }
-			set {
-				if (lastSuccessfulUpdateCheck != value)
+			get => lastSuccessfulUpdateCheck;
+			set => SetProperty(ref lastSuccessfulUpdateCheck, value);
+		}
+
+		public void LoadFromXml(XElement section)
+		{
+			AutomaticUpdateCheckEnabled = (bool?)section.Element("AutomaticUpdateCheckEnabled") ?? true;
+			try
 			{
-					lastSuccessfulUpdateCheck = value;
-					Save();
-					OnPropertyChanged(nameof(LastSuccessfulUpdateCheck));
+				LastSuccessfulUpdateCheck = (DateTime?)section.Element("LastSuccessfulUpdateCheck");
 			}
+			catch (FormatException)
+			{
+				// avoid crashing on settings files invalid due to
+				// https://github.com/icsharpcode/ILSpy/issues/closed/#issue/2
 			}
 		}
 
-		public void Save()
+		public XElement SaveToXml()
 		{
-			XElement updateSettings = new XElement("UpdateSettings");
-			updateSettings.Add(new XElement("AutomaticUpdateCheckEnabled", automaticUpdateCheckEnabled));
-			if (lastSuccessfulUpdateCheck != null)
-				updateSettings.Add(new XElement("LastSuccessfulUpdateCheck", lastSuccessfulUpdateCheck));
-			SettingsService.Instance.SpySettings.SaveSettings(updateSettings);
-		}
+			var section = new XElement(SectionName);
 
-		public event PropertyChangedEventHandler PropertyChanged;
+			section.Add(new XElement("AutomaticUpdateCheckEnabled", AutomaticUpdateCheckEnabled));
 
-		void OnPropertyChanged(string propertyName)
+			if (LastSuccessfulUpdateCheck != null)
 			{
-			PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
-		}
+				section.Add(new XElement("LastSuccessfulUpdateCheck", LastSuccessfulUpdateCheck));
 			}
 
+			return section;
+		}
+	}
 }

ILSpy/Util/SettingsService.cs

@@ -19,12 +19,9 @@
 using System;
 using System.Collections.Concurrent;
 using System.ComponentModel;
-using System.Composition;
 using System.Xml.Linq;
 
-using ICSharpCode.Decompiler;
 using ICSharpCode.ILSpy.Options;
-using ICSharpCode.ILSpy.ViewModels;
 using ICSharpCode.ILSpyX;
 using ICSharpCode.ILSpyX.Settings;
 
@@ -48,16 +45,11 @@ namespace ICSharpCode.ILSpy.Util
 		XElement SaveToXml();
 	}
 
-	public abstract class SettingsServiceBase
+	public abstract class SettingsServiceBase(ISettingsProvider spySettings)
 	{
 		protected readonly ConcurrentDictionary<Type, ISettingsSection> sections = new();
 
-		public ISettingsProvider SpySettings;
-
-		protected SettingsServiceBase(ISettingsProvider spySettings)
-		{
-			SpySettings = spySettings;
-		}
+		protected ISettingsProvider SpySettings { get; set; } = spySettings;
 
 		public T GetSettings<T>() where T : ISettingsSection, new()
 		{
@@ -73,7 +65,7 @@ namespace ICSharpCode.ILSpy.Util
 			});
 		}
 
-		protected void SaveSection(ISettingsSection section, XElement root)
+		protected static void SaveSection(ISettingsSection section, XElement root)
 		{
 			var element = section.SaveToXml();
 
@@ -89,15 +81,8 @@ namespace ICSharpCode.ILSpy.Util
 		}
 	}
 
-	public class SettingsSnapshot : SettingsServiceBase
-	{
-		private readonly SettingsService parent;
-
-		public SettingsSnapshot(SettingsService parent) : base(parent.SpySettings)
+	public class SettingsSnapshot(SettingsService parent, ISettingsProvider spySettings) : SettingsServiceBase(spySettings)
 	{
-			this.parent = parent;
-		}
-
 		public void Save()
 		{
 			SpySettings.Update(root => {
@@ -113,8 +98,6 @@ namespace ICSharpCode.ILSpy.Util
 
 	public class SettingsService() : SettingsServiceBase(LoadSettings())
 	{
-		public static readonly SettingsService Instance = App.ExportProvider.GetExportedValue<SettingsService>();
-
 		public SessionSettings SessionSettings => GetSettings<SessionSettings>();
 
 		public DecompilerSettings DecompilerSettings => GetSettings<DecompilerSettings>();
@@ -174,7 +157,7 @@ namespace ICSharpCode.ILSpy.Util
 
 		public SettingsSnapshot CreateSnapshot()
 		{
-			return new(this);
+			return new(this, SpySettings);
 		}
 
 		protected override void Section_PropertyChanged(object? sender, PropertyChangedEventArgs e)