Merge pull request #4117 from icsharpcode/fix/stale-view-on-clear

Empty the panes when the assembly list is cleared

ICSharpCode.ILSpyX/AssemblyList.cs

@@ -451,8 +451,24 @@ namespace ICSharpCode.ILSpyX
 			{
 				List<LoadedAssembly> list = new List<LoadedAssembly>(assemblies);
 				list.Sort(index, Math.Min(count, list.Count - index), comparer);
-				assemblies.Clear();
-				assemblies.AddRange(list);
+				// Reorder in place. Rebuilding the collection through Clear() would raise a Reset,
+				// which says every entry went away - and consumers that hold on to what the list
+				// contained (the navigation history, the open tabs) would throw it all away for a
+				// change that removes nothing.
+				for (int i = 0; i < list.Count; i++)
+				{
+					if (ReferenceEquals(assemblies[i], list[i]))
+						continue;
+					// Both hold the same entries, so the item is somewhere after i.
+					for (int j = i + 1; j < assemblies.Count; j++)
+					{
+						if (ReferenceEquals(assemblies[j], list[i]))
+						{
+							assemblies.Move(j, i);
+							break;
+						}
+					}
+				}
 			}
 		}
 

ILSpy.Tests/AssemblyList/AssemblyListResetTests.cs

@@ -0,0 +1,110 @@
+// Copyright (c) 2026 Siegfried Pammer
+//
+// Permission is hereby granted, free of charge, to any person obtaining a copy of this
+// software and associated documentation files (the "Software"), to deal in the Software
+// without restriction, including without limitation the rights to use, copy, modify, merge,
+// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons
+// to whom the Software is furnished to do so, subject to the following conditions:
+//
+// The above copyright notice and this permission notice shall be included in all copies or
+// substantial portions of the Software.
+//
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
+// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR
+// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE
+// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
+// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+// DEALINGS IN THE SOFTWARE.
+
+using System.Linq;
+using System.Threading.Tasks;
+
+using Avalonia.Headless.NUnit;
+
+using AwesomeAssertions;
+
+using ICSharpCode.ILSpy.TreeNodes;
+
+using NUnit.Framework;
+
+namespace ICSharpCode.ILSpy.Tests;
+
+/// <summary>
+/// What a wholesale change to the assembly list does to the panes that show its content.
+/// Clearing the list and sorting it both raise a Reset on the collection, but they mean
+/// opposite things: after a clear nothing that was on screen exists any more, while a sort
+/// only reorders and everything on screen is still valid.
+/// </summary>
+[TestFixture]
+public class AssemblyListResetTests
+{
+	static async Task<MethodTreeNode> DecompileAMethodAsync(ViewModels.MainWindowViewModel vm)
+	{
+		var typeNode = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
+			"System.Linq", "System.Linq", "System.Linq.Enumerable");
+		typeNode.IsExpanded = true;
+		var method = typeNode.Children.OfType<MethodTreeNode>()
+			.Single(m => m.MethodDefinition.Name == "AsEnumerable");
+		vm.AssemblyTreeModel.SelectNode(method);
+		var tab = await vm.DockWorkspace.WaitForDecompiledTextAsync();
+		tab.Text.Should().Contain("AsEnumerable", "the pane must hold something before it can go stale");
+		return method;
+	}
+
+	[AvaloniaTest]
+	public async Task Clearing_The_Assembly_List_Empties_The_Decompiled_View()
+	{
+		// Clear() raises Reset, whose OldItems is null. The pane used to keep showing the last
+		// decompiled member over an empty list, while removing the same assemblies one at a
+		// time cleaned it up correctly.
+
+		// Arrange - boot and decompile something.
+		var (_, vm) = await TestHarness.BootAsync(3);
+		await DecompileAMethodAsync(vm);
+
+		// Act - clear the whole list, as the "Clear assembly list" command does.
+		vm.AssemblyTreeModel.AssemblyList!.Clear();
+		await Waiters.WaitForIdleAsync();
+
+		// Assert - nothing from the cleared assemblies is left on screen.
+		vm.AssemblyTreeModel.AssemblyList!.GetAssemblies().Should().BeEmpty();
+		var tab = vm.DockWorkspace.ActiveDecompilerTab;
+		if (tab != null)
+			tab.Text.Should().NotContain("AsEnumerable", "the assembly it came from is gone");
+	}
+
+	[AvaloniaTest]
+	public async Task Sorting_The_Assembly_List_Keeps_The_Navigation_History()
+	{
+		// Sort() rebuilt the collection through Clear() + AddRange, so it raised the same Reset
+		// a wholesale clear does and every history entry was dropped with it - even though
+		// sorting removes nothing.
+
+		// Arrange - boot, decompile two members so there is a back entry to lose.
+		var (_, vm) = await TestHarness.BootAsync(3);
+		var typeNode = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
+			"System.Linq", "System.Linq", "System.Linq.Enumerable");
+		typeNode.IsExpanded = true;
+		var first = typeNode.Children.OfType<MethodTreeNode>()
+			.Single(m => m.MethodDefinition.Name == "AsEnumerable");
+		var second = typeNode.Children.OfType<MethodTreeNode>()
+			.First(m => m.MethodDefinition.Name == "Empty");
+		vm.AssemblyTreeModel.SelectNode(first);
+		await vm.DockWorkspace.WaitForDecompiledTextAsync();
+		// NavigationHistory collapses selections inside 0.5s into one entry.
+		await Task.Delay(600);
+		vm.AssemblyTreeModel.SelectNode(second);
+		await Waiters.WaitForAsync(() => ReferenceEquals(vm.AssemblyTreeModel.SelectedItem, second));
+		await vm.DockWorkspace.WaitForDecompiledTextAsync();
+		vm.DockWorkspace.BackHistory.Should().NotBeEmpty("the test needs history to survive");
+		int assemblyCount = vm.AssemblyTreeModel.AssemblyList!.GetAssemblies().Length;
+
+		// Act - sort the list.
+		vm.AssemblyTreeModel.SortAssemblyList();
+		await Waiters.WaitForIdleAsync();
+
+		// Assert - the list is only reordered, so nothing it held became invalid.
+		vm.AssemblyTreeModel.AssemblyList!.GetAssemblies().Should().HaveCount(assemblyCount);
+		vm.DockWorkspace.BackHistory.Should().NotBeEmpty("sorting reorders the list, it removes nothing");
+	}
+}

ILSpy/AssemblyTree/AssemblyTreeModel.cs

@@ -969,6 +969,10 @@ namespace ICSharpCode.ILSpy.AssemblyTree
 		/// </summary>
 		void OnActiveAssemblyListCollectionChanged(object? sender, NotifyCollectionChangedEventArgs e)
 		{
+			// A Move carries the moved entry in OldItems, but nothing left the list: sorting must
+			// not look like removal to anything downstream.
+			if (e.Action == NotifyCollectionChangedAction.Move)
+				return;
 			// Prune navigation-history entries that pointed at tree nodes inside removed
 			// assemblies BEFORE re-publishing — Back/Forward consumers (the toolbar
 			// commands + dropdowns) re-evaluate their CanExecute when the bus fires, so

ILSpy/Docking/DockWorkspace.cs

@@ -243,20 +243,22 @@ namespace ICSharpCode.ILSpy.Docking
 		{
 			var inner = e.Inner;
 
-			// On Reset (assembly list wholesale-cleared), drop ALL history — every entry is
-			// stale by definition.
-			if (inner.Action == System.Collections.Specialized.NotifyCollectionChangedAction.Reset)
-			{
-				PruneHistoryAfterAssemblyListChange(removed: null);
+			// A Move carries the moved entry in OldItems, but the list only got reordered.
+			if (inner.Action == System.Collections.Specialized.NotifyCollectionChangedAction.Move)
 				return;
-			}
 
-			if (inner.OldItems is not { Count: > 0 } oldItems)
-				return;
-			var removed = new HashSet<ICSharpCode.ILSpyX.LoadedAssembly>(
-				oldItems.OfType<ICSharpCode.ILSpyX.LoadedAssembly>());
-			if (removed.Count == 0)
-				return;
+			// On Reset the list was cleared wholesale: every entry is stale by definition, and
+			// `removed == null` below stands for "all of them".
+			HashSet<ICSharpCode.ILSpyX.LoadedAssembly>? removed = null;
+			if (inner.Action != System.Collections.Specialized.NotifyCollectionChangedAction.Reset)
+			{
+				if (inner.OldItems is not { Count: > 0 } oldItems)
+					return;
+				removed = new HashSet<ICSharpCode.ILSpyX.LoadedAssembly>(
+					oldItems.OfType<ICSharpCode.ILSpyX.LoadedAssembly>());
+				if (removed.Count == 0)
+					return;
+			}
 
 			PruneHistoryAfterAssemblyListChange(removed);
 
@@ -277,7 +279,7 @@ namespace ICSharpCode.ILSpy.Docking
 					var owner = n.AncestorsAndSelf().OfType<TreeNodes.AssemblyTreeNode>().LastOrDefault();
 					if (owner is null)
 						continue;
-					if (removed.Contains(owner.LoadedAssembly))
+					if (removed == null || removed.Contains(owner.LoadedAssembly))
 						anyTouchesRemoved = true;
 					else
 						anyAlive = true;