Fix some minor design flaws in new code.

icsharpcode · Sep 14, 2024 · da01176 · da01176
1 parent 0b477f7
commit da01176
Show file tree

Hide file tree

Showing 19 changed files with 53 additions and 46 deletions.
diff --git a/ILSpy.ReadyToRun/ReadyToRunOptionPage.xaml.cs b/ILSpy.ReadyToRun/ReadyToRunOptionPage.xaml.cs
@@ -56,7 +56,7 @@ public void Load(SettingsSnapshot snapshot)
 
 		public void LoadDefaults()
 		{
-			Options.LoadFromSection(new("empty"));
+			Options.LoadFromXml(new("empty"));
 		}
 	}
 }
diff --git a/ILSpy.ReadyToRun/ReadyToRunOptions.cs b/ILSpy.ReadyToRun/ReadyToRunOptions.cs
@@ -64,7 +64,7 @@ public string DisassemblyFormat {
 
 		public XName SectionName { get; } = ns + "ReadyToRunOptions";
 
-		public void LoadFromSection(XElement e)
+		public void LoadFromXml(XElement e)
 		{
 			XAttribute format = e.Attribute("DisassemblyFormat");
 			DisassemblyFormat = format == null ? intel : (string)format;
@@ -79,12 +79,16 @@ public void LoadFromSection(XElement e)
 			IsShowGCInfo = showGc != null && (bool)showGc;
 		}
 
-		public void SaveToSection(XElement section)
+		public XElement SaveToXml()
 		{
+			var section = new XElement(SectionName);
+
 			section.SetAttributeValue("DisassemblyFormat", disassemblyFormat);
 			section.SetAttributeValue("IsShowUnwindInfo", isShowUnwindInfo);
 			section.SetAttributeValue("IsShowDebugInfo", isShowDebugInfo);
 			section.SetAttributeValue("IsShowGCInfo", isShowGCInfo);
+
+			return section;
 		}
 	}
 }
diff --git a/ILSpy/Commands/BrowseBackCommand.cs b/ILSpy/Commands/BrowseBackCommand.cs
@@ -47,8 +47,6 @@ protected override void OnCanExecute(object sender, CanExecuteRoutedEventArgs e)
 
 		protected override void OnExecute(object sender, ExecutedRoutedEventArgs e)
 		{
-			base.OnExecute(sender, e);
-
 			if (assemblyTreeModel.CanNavigateBack)
 			{
 				e.Handled = true;

diff --git a/ILSpy/Commands/BrowseForwardCommand.cs b/ILSpy/Commands/BrowseForwardCommand.cs
@@ -47,8 +47,6 @@ protected override void OnCanExecute(object sender, CanExecuteRoutedEventArgs e)
 
 		protected override void OnExecute(object sender, ExecutedRoutedEventArgs e)
 		{
-			base.OnExecute(sender, e);
-
 			if (assemblyTreeModel.CanNavigateForward)
 			{
 				e.Handled = true;

diff --git a/ILSpy/Commands/CommandWrapper.cs b/ILSpy/Commands/CommandWrapper.cs
@@ -22,11 +22,11 @@
 
 namespace ICSharpCode.ILSpy
 {
-	class CommandWrapper : ICommand
+	abstract class CommandWrapper : ICommand
 	{
 		private readonly ICommand wrappedCommand;
 
-		public CommandWrapper(ICommand wrappedCommand)
+		protected CommandWrapper(ICommand wrappedCommand)
 		{
 			this.wrappedCommand = wrappedCommand;
 
@@ -56,9 +56,7 @@ public bool CanExecute(object parameter)
 			return wrappedCommand.CanExecute(parameter);
 		}
 
-		protected virtual void OnExecute(object sender, ExecutedRoutedEventArgs e)
-		{
-		}
+		protected abstract void OnExecute(object sender, ExecutedRoutedEventArgs e);
 
 		protected virtual void OnCanExecute(object sender, CanExecuteRoutedEventArgs e)
 		{

diff --git a/ILSpy/Commands/OpenCommand.cs b/ILSpy/Commands/OpenCommand.cs
@@ -42,8 +42,6 @@ public OpenCommand(AssemblyTreeModel assemblyTreeModel)
 
 		protected override void OnExecute(object sender, ExecutedRoutedEventArgs e)
 		{
-			base.OnExecute(sender, e);
-
 			e.Handled = true;
 			OpenFileDialog dlg = new OpenFileDialog {
 				Filter = ".NET assemblies|*.dll;*.exe;*.winmd;*.wasm|Nuget Packages (*.nupkg)|*.nupkg|Portable Program Database (*.pdb)|*.pdb|All files|*.*",

diff --git a/ILSpy/Commands/RefreshCommand.cs b/ILSpy/Commands/RefreshCommand.cs
@@ -40,8 +40,6 @@ public RefreshCommand(AssemblyTreeModel assemblyTreeModel)
 
 		protected override void OnExecute(object sender, ExecutedRoutedEventArgs e)
 		{
-			base.OnExecute(sender, e);
-
 			assemblyTreeModel.Refresh();
 		}
 	}

diff --git a/ILSpy/Commands/SaveCommand.cs b/ILSpy/Commands/SaveCommand.cs
@@ -47,8 +47,6 @@ protected override void OnCanExecute(object sender, CanExecuteRoutedEventArgs e)
 
 		protected override void OnExecute(object sender, ExecutedRoutedEventArgs e)
 		{
-			base.OnExecute(sender, e);
-
 			SaveCodeContextMenuEntry.Execute(assemblyTreeModel.SelectedNodes.ToList());
 		}
 	}

diff --git a/ILSpy/Languages/Languages.cs b/ILSpy/Languages/Languages.cs
@@ -54,7 +54,7 @@ public static Language GetLanguage(string name)
 			return AllLanguages.FirstOrDefault(l => l.Name == name) ?? AllLanguages.First();
 		}
 
-		static ILLanguage ilLanguage;
+		static ILLanguage? ilLanguage;
 
 		public static ILLanguage ILLanguage {
 			get {

diff --git a/ILSpy/Options/DecompilerSettings.cs b/ILSpy/Options/DecompilerSettings.cs
@@ -17,15 +17,19 @@ public class DecompilerSettings : Decompiler.DecompilerSettings, ISettingsSectio
 
 		public XName SectionName => "DecompilerSettings";
 
-		public void SaveToSection(XElement section)
+		public XElement SaveToXml()
 		{
+			var section = new XElement(SectionName);
+
 			foreach (var p in properties)
 			{
 				section.SetAttributeValue(p.Name, p.GetValue(this));
 			}
+
+			return section;
 		}
 
-		public void LoadFromSection(XElement section)
+		public void LoadFromXml(XElement section)
 		{
 			foreach (var p in properties)
 			{
@@ -37,10 +41,10 @@ public void LoadFromSection(XElement section)
 
 		public new DecompilerSettings Clone()
 		{
-			var section = new XElement("DecompilerSettings");
-			SaveToSection(section);
+			var section = SaveToXml();
+
 			var newSettings = new DecompilerSettings();
-			newSettings.LoadFromSection(section);
+			newSettings.LoadFromXml(section);
 
 			return newSettings;
 		}

diff --git a/ILSpy/Options/DecompilerSettingsViewModel.cs b/ILSpy/Options/DecompilerSettingsViewModel.cs
@@ -138,7 +138,7 @@ public sealed class DecompilerSettingsItemViewModel(PropertyInfo property, Decom
 	{
 		private bool isEnabled = property.GetValue(decompilerSettings) is true;
 
-		public PropertyInfo Property { get; } = property;
+		public PropertyInfo Property => property;
 
 		public bool IsEnabled {
 			get => isEnabled;

diff --git a/ILSpy/Options/DisplaySettings.cs b/ILSpy/Options/DisplaySettings.cs
@@ -150,7 +150,7 @@ public bool ShowRawOffsetsAndBytesBeforeInstruction {
 
 		public XName SectionName => "DisplaySettings";
 
-		public void LoadFromSection(XElement section)
+		public void LoadFromXml(XElement section)
 		{
 			SelectedFont = new FontFamily((string)section.Attribute("Font") ?? "Consolas");
 			SelectedFontSize = (double?)section.Attribute("FontSize") ?? 10.0 * 4 / 3;
@@ -174,8 +174,10 @@ public void LoadFromSection(XElement section)
 			StyleWindowTitleBar = (bool?)section.Attribute("StyleWindowTitleBar") ?? false;
 		}
 
-		public void SaveToSection(XElement section)
+		public XElement SaveToXml()
 		{
+			var section = new XElement(SectionName);
+
 			section.SetAttributeValue("Font", SelectedFont.Source);
 			section.SetAttributeValue("FontSize", SelectedFontSize);
 			section.SetAttributeValue("ShowLineNumbers", ShowLineNumbers);
@@ -196,6 +198,8 @@ public void SaveToSection(XElement section)
 			section.SetAttributeValue("UseNestedNamespaceNodes", UseNestedNamespaceNodes);
 			section.SetAttributeValue("ShowRawOffsetsAndBytesBeforeInstruction", ShowRawOffsetsAndBytesBeforeInstruction);
 			section.SetAttributeValue("StyleWindowTitleBar", StyleWindowTitleBar);
+
+			return section;
 		}
 	}
 }
diff --git a/ILSpy/Options/DisplaySettingsViewModel.cs b/ILSpy/Options/DisplaySettingsViewModel.cs
@@ -87,7 +87,7 @@ static FontFamily[] FontLoader()
 
 		public void LoadDefaults()
 		{
-			Settings.LoadFromSection(new XElement("empty"));
+			Settings.LoadFromXml(new XElement("empty"));
 			SessionSettings.Theme = ThemeManager.Current.DefaultTheme;
 		}
 	}

diff --git a/ILSpy/Options/MiscSettings.cs b/ILSpy/Options/MiscSettings.cs
@@ -40,16 +40,20 @@ public bool LoadPreviousAssemblies {
 
 		public XName SectionName => "MiscSettings";
 
-		public void LoadFromSection(XElement e)
+		public void LoadFromXml(XElement e)
 		{
 			AllowMultipleInstances = (bool?)e.Attribute(nameof(AllowMultipleInstances)) ?? false;
 			LoadPreviousAssemblies = (bool?)e.Attribute(nameof(LoadPreviousAssemblies)) ?? true;
 		}
 
-		public void SaveToSection(XElement section)
+		public XElement SaveToXml()
 		{
+			var section = new XElement(SectionName);
+
 			section.SetAttributeValue(nameof(AllowMultipleInstances), AllowMultipleInstances);
 			section.SetAttributeValue(nameof(LoadPreviousAssemblies), LoadPreviousAssemblies);
+
+			return section;
 		}
 	}
 }

diff --git a/ILSpy/Options/MiscSettingsViewModel.cs b/ILSpy/Options/MiscSettingsViewModel.cs
@@ -98,7 +98,7 @@ public void Load(SettingsSnapshot settings)
 
 		public void LoadDefaults()
 		{
-			Settings.LoadFromSection(new XElement("dummy"));
+			Settings.LoadFromXml(new XElement("dummy"));
 		}
 	}
 }
diff --git a/ILSpy/Search/SearchPane.xaml.cs b/ILSpy/Search/SearchPane.xaml.cs
@@ -550,8 +550,6 @@ public ShowSearchCommand()
 
 		protected override void OnExecute(object sender, ExecutedRoutedEventArgs e)
 		{
-			base.OnExecute(sender, e);
-
 			DockWorkspace.Instance.ShowToolPane(SearchPaneModel.PaneContentId);
 		}
 	}

diff --git a/ILSpy/SessionSettings.cs b/ILSpy/SessionSettings.cs
@@ -41,7 +41,7 @@ public sealed class SessionSettings : ISettingsSection
 	{
 		public XName SectionName => "SessionSettings";
 
-		public void LoadFromSection(XElement section)
+		public void LoadFromXml(XElement section)
 		{
 			XElement filterSettings = section.Element("FilterSettings") ?? new XElement("FilterSettings");
 
@@ -103,9 +103,9 @@ public string ActiveAssemblyList {
 
 		public DockLayoutSettings DockLayout { get; set; }
 
-		public void SaveToSection(XElement section)
+		public XElement SaveToXml()
 		{
-			section.RemoveAll();
+			var section = new XElement(SectionName);
 
 			section.Add(this.LanguageSettings.SaveAsXml());
 			if (this.ActiveAssemblyList != null)
@@ -134,6 +134,8 @@ public void SaveToSection(XElement section)
 				dockLayoutElement.Add(DockLayout.SaveAsXml());
 			}
 			section.Add(dockLayoutElement);
+
+			return section;
 		}
 
 		static Regex regex = new("\\\\x(?<num>[0-9A-f]{4})");

diff --git a/ILSpy/Util/SettingsService.cs b/ILSpy/Util/SettingsService.cs
@@ -24,9 +24,9 @@ public interface ISettingsSection : INotifyPropertyChanged
 	{
 		XName SectionName { get; }
 
-		void LoadFromSection(XElement section);
+		void LoadFromXml(XElement section);
 
-		void SaveToSection(XElement section);
+		XElement SaveToXml();
 	}
 
 	public abstract class SettingsServiceBase
@@ -47,7 +47,7 @@ protected SettingsServiceBase(ISettingsProvider spySettings)
 
 				var sectionElement = SpySettings[section.SectionName];
 
-				section.LoadFromSection(sectionElement);
+				section.LoadFromXml(sectionElement);
 				section.PropertyChanged += Section_PropertyChanged;
 
 				return section;
@@ -56,9 +56,7 @@ protected SettingsServiceBase(ISettingsProvider spySettings)
 
 		protected void SaveSection(ISettingsSection section, XElement root)
 		{
-			var element = SpySettings[section.SectionName];
-
-			section.SaveToSection(element);
+			var element = section.SaveToXml();
 
 			var existingElement = root.Element(section.SectionName);
 			if (existingElement != null)
@@ -150,7 +148,7 @@ public void Reload()
 				{
 					var element = SpySettings[section.SectionName];
 
-					section.LoadFromSection(element);
+					section.LoadFromXml(element);
 				}
 			}
 			finally

diff --git a/TestPlugin/CustomOptionPage.xaml.cs b/TestPlugin/CustomOptionPage.xaml.cs
@@ -23,6 +23,7 @@ public CustomOptionPage()
 	}
 
 	[ExportOptionPage(Order = 0)]
+	[PartCreationPolicy(CreationPolicy.NonShared)]
 	class CustomOptionsViewModel : ObservableObject, IOptionPage
 	{
 		private Options options;
@@ -41,7 +42,7 @@ public void Load(SettingsSnapshot snapshot)
 
 		public void LoadDefaults()
 		{
-			Options.LoadFromSection(new XElement("dummy"));
+			Options.LoadFromXml(new XElement("dummy"));
 		}
 	}
 
@@ -65,16 +66,20 @@ public double UselessOption2 {
 
 		public XName SectionName { get; } = ns + "CustomOptions";
 
-		public void LoadFromSection(XElement e)
+		public void LoadFromXml(XElement e)
 		{
 			UselessOption1 = (bool?)e.Attribute("useless1") ?? false;
 			UselessOption2 = (double?)e.Attribute("useless2") ?? 50.0;
 		}
 
-		public void SaveToSection(XElement section)
+		public XElement SaveToXml()
 		{
+			var section = new XElement(SectionName);
+
 			section.SetAttributeValue("useless1", UselessOption1);
 			section.SetAttributeValue("useless2", UselessOption2);
+
+			return section;
 		}
 	}
 }