diff --git a/ICSharpCode.ILSpyX/AssemblyList.cs b/ICSharpCode.ILSpyX/AssemblyList.cs index d53f91a19..49d9fbae3 100644 --- a/ICSharpCode.ILSpyX/AssemblyList.cs +++ b/ICSharpCode.ILSpyX/AssemblyList.cs @@ -451,8 +451,24 @@ namespace ICSharpCode.ILSpyX { List list = new List(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; + } + } + } } } diff --git a/ILSpy.Tests/AssemblyList/AssemblyListResetTests.cs b/ILSpy.Tests/AssemblyList/AssemblyListResetTests.cs new file mode 100644 index 000000000..42cd7ce19 --- /dev/null +++ b/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; + +/// +/// 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. +/// +[TestFixture] +public class AssemblyListResetTests +{ + static async Task DecompileAMethodAsync(ViewModels.MainWindowViewModel vm) + { + var typeNode = vm.AssemblyTreeModel.FindNode( + "System.Linq", "System.Linq", "System.Linq.Enumerable"); + typeNode.IsExpanded = true; + var method = typeNode.Children.OfType() + .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( + "System.Linq", "System.Linq", "System.Linq.Enumerable"); + typeNode.IsExpanded = true; + var first = typeNode.Children.OfType() + .Single(m => m.MethodDefinition.Name == "AsEnumerable"); + var second = typeNode.Children.OfType() + .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"); + } +} diff --git a/ILSpy/AssemblyTree/AssemblyTreeModel.cs b/ILSpy/AssemblyTree/AssemblyTreeModel.cs index 1a77c1987..8eefbae53 100644 --- a/ILSpy/AssemblyTree/AssemblyTreeModel.cs +++ b/ILSpy/AssemblyTree/AssemblyTreeModel.cs @@ -969,6 +969,10 @@ namespace ICSharpCode.ILSpy.AssemblyTree /// 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 diff --git a/ILSpy/Docking/DockWorkspace.cs b/ILSpy/Docking/DockWorkspace.cs index 3e32ee93f..ccb673ded 100644 --- a/ILSpy/Docking/DockWorkspace.cs +++ b/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( - oldItems.OfType()); - 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? removed = null; + if (inner.Action != System.Collections.Specialized.NotifyCollectionChangedAction.Reset) + { + if (inner.OldItems is not { Count: > 0 } oldItems) + return; + removed = new HashSet( + oldItems.OfType()); + if (removed.Count == 0) + return; + } PruneHistoryAfterAssemblyListChange(removed); @@ -277,7 +279,7 @@ namespace ICSharpCode.ILSpy.Docking var owner = n.AncestorsAndSelf().OfType().LastOrDefault(); if (owner is null) continue; - if (removed.Contains(owner.LoadedAssembly)) + if (removed == null || removed.Contains(owner.LoadedAssembly)) anyTouchesRemoved = true; else anyAlive = true;