Browse Source

Empty the panes when the assembly list is cleared

Removing assemblies one at a time cleaned up the tabs that showed them,
but clearing the whole list did not: Clear() raises a Reset whose
OldItems is null, and the handler pruned the history and returned before
the loop that empties the main tab and closes the orphaned ones. The
last decompiled member stayed on screen over an empty list.

Sorting had to be fixed first. It rebuilt the collection through Clear()
plus AddRange, so it raised the same Reset and dropped the whole
navigation history as a side effect - and once Reset closes tabs, it
would have closed all of those too. It now reorders in place, and a Move
is ignored where a removal would be handled, because a sort removes
nothing.

Assisted-by: Claude:claude-opus-5:Claude Code
pull/4117/head
Siegfried Pammer 1 week ago
parent
commit
42022a3a9e
  1. 20
      ICSharpCode.ILSpyX/AssemblyList.cs
  2. 110
      ILSpy.Tests/AssemblyList/AssemblyListResetTests.cs
  3. 4
      ILSpy/AssemblyTree/AssemblyTreeModel.cs
  4. 18
      ILSpy/Docking/DockWorkspace.cs

20
ICSharpCode.ILSpyX/AssemblyList.cs

@ -451,8 +451,24 @@ namespace ICSharpCode.ILSpyX @@ -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;
}
}
}
}
}

110
ILSpy.Tests/AssemblyList/AssemblyListResetTests.cs

@ -0,0 +1,110 @@ @@ -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");
}
}

4
ILSpy/AssemblyTree/AssemblyTreeModel.cs

@ -969,6 +969,10 @@ namespace ICSharpCode.ILSpy.AssemblyTree @@ -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

18
ILSpy/Docking/DockWorkspace.cs

@ -243,20 +243,22 @@ namespace ICSharpCode.ILSpy.Docking @@ -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;
}
// 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;
var removed = new HashSet<ICSharpCode.ILSpyX.LoadedAssembly>(
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 @@ -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;

Loading…
Cancel
Save