Browse Source

Leave nothing of the previous list behind when it stops being shown

Switching to another assembly list replaces the whole tree, but nothing
is removed from the outgoing list - the list itself goes away - so no
collection event announces it and the selection, the open tabs and the
navigation history all kept describing a list that was no longer on
screen. The switch now says so with the same Reset that clearing a list
raises, which is the one path that already discards all of it.

Emptying a tab kept its title, because the CurrentNodes setter only
recomputes the cached base title and StartDecompile returns early with
nothing to decompile. A tab showing an empty document went on naming the
member it used to show.

A removal that took the selected node with it left nothing selected,
even with assemblies still loaded: the tree view picks the nearest
survivor for its own Delete gesture, but a removal from anywhere else
did not. The selection is handed over once the tree has caught up.

Assisted-by: Claude:claude-opus-5:Claude Code
pull/4118/head
Siegfried Pammer 1 week ago
parent
commit
5dbf34d0f5
  1. 130
      ILSpy.Tests/AssemblyList/SwitchListAndDeleteAllTests.cs
  2. 36
      ILSpy/AssemblyTree/AssemblyTreeModel.cs
  3. 8
      ILSpy/TextView/DecompilerTabPageModel.cs

130
ILSpy.Tests/AssemblyList/SwitchListAndDeleteAllTests.cs

@ -0,0 +1,130 @@ @@ -0,0 +1,130 @@
// 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;
using System.Linq;
using System.Threading.Tasks;
using Avalonia.Headless;
using Avalonia.Headless.NUnit;
using Avalonia.Input;
using Avalonia.Threading;
using AwesomeAssertions;
using ICSharpCode.ILSpy.AppEnv;
using ICSharpCode.ILSpy.AssemblyTree;
using ICSharpCode.ILSpy.TreeNodes;
using ICSharpCode.ILSpyX;
using NUnit.Framework;
namespace ICSharpCode.ILSpy.Tests;
/// <summary>
/// Switching to another assembly list replaces the whole tree, and deleting every assembly
/// empties the one that is showing. Both leave the selection and the navigation history
/// pointing at nodes that are no longer anywhere in the tree.
/// </summary>
[TestFixture]
public class SwitchListAndDeleteAllTests
{
static async Task GiveTheHistorySomethingToHoldAsync(ViewModels.MainWindowViewModel vm)
{
var typeNode = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
"System.Linq", "System.Linq", "System.Linq.Enumerable");
typeNode.IsExpanded = true;
var methods = typeNode.Children.OfType<MethodTreeNode>().ToList();
vm.AssemblyTreeModel.SelectNode(methods.First(m => m.MethodDefinition.Name == "AsEnumerable"));
await vm.DockWorkspace.WaitForDecompiledTextAsync();
// NavigationHistory collapses selections inside 0.5s into one entry.
await Task.Delay(600);
vm.AssemblyTreeModel.SelectNode(methods.First(m => m.MethodDefinition.Name == "Empty"));
await vm.DockWorkspace.WaitForDecompiledTextAsync();
vm.DockWorkspace.BackHistory.Should().NotBeEmpty("the test needs history to lose");
}
[AvaloniaTest]
public async Task Switching_To_Another_Assembly_List_Drops_The_Selection_And_History()
{
// The whole tree is replaced, so every history entry and every selected node belongs to
// a tree that is no longer on screen.
// Arrange - boot, build up history in the default list.
var (_, vm) = await TestHarness.BootAsync(3);
await GiveTheHistorySomethingToHoldAsync(vm);
// Act - create another list and switch to it, as the assembly-list dropdown does.
var listManager = AppComposition.Current.GetExport<SettingsService>().AssemblyListManager;
listManager.CreateList("test-list");
vm.AssemblyTreeModel.ActiveListName = "test-list";
await Waiters.WaitForIdleAsync();
// Assert - nothing from the previous list is still held on to.
vm.AssemblyTreeModel.SelectedItems.Should().BeEmpty("the nodes belong to the previous tree");
vm.DockWorkspace.BackHistory.Should().BeEmpty("every entry points into the previous tree");
vm.AssemblyTreeModel.Root!.Children.Should().BeEmpty("a freshly created list holds no assemblies");
var tab = vm.DockWorkspace.ActiveDecompilerTab;
tab.Should().NotBeNull();
tab!.Text.Should().BeEmpty("the member it showed belongs to the previous list");
tab.Title.Should().Be("(unnamed)", "a tab that shows nothing must not still name what it showed");
}
[AvaloniaTest]
public async Task Ctrl_A_Then_Delete_Drops_The_Selection_History_And_Tab()
{
// The real gestures: SharpTreeView handles Ctrl+A (SelectAll) and Delete
// (DeleteSelection) itself, so driving the model's SelectNodes/DeleteCore instead would
// skip whatever the view contributes to clearing up.
// Arrange - boot, decompile something so there is a tab and history to lose, then give
// the tree keyboard focus.
var (window, vm) = await TestHarness.BootAsync(3);
await GiveTheHistorySomethingToHoldAsync(vm);
// Collapse again: Ctrl+A selects every visible row, and the tree is normally collapsed
// to its assemblies when a user reaches for it.
foreach (var node in vm.AssemblyTreeModel.Root!.Children)
node.IsExpanded = false;
await Waiters.WaitForIdleAsync();
var pane = await window.WaitForComponent<AssemblyListPane>();
var tree = await pane.WaitForComponent<ICSharpCode.ILSpy.Controls.TreeView.SharpTreeView>();
tree.Focus();
Dispatcher.UIThread.RunJobs();
// Act - Ctrl+A, then Delete.
int selectedBefore = vm.AssemblyTreeModel.SelectedItems.Count;
HeadlessWindowExtensions.KeyPress(window, Key.A, RawInputModifiers.Control, PhysicalKey.A, null);
Waiters.PumpUI();
vm.AssemblyTreeModel.SelectedItems.Count.Should().BeGreaterThan(selectedBefore,
"Ctrl+A must reach the tree and select more than the one row that was selected");
vm.DockWorkspace.BackHistory.Should().NotBeEmpty("the history has to survive up to the delete");
HeadlessWindowExtensions.KeyPress(window, Key.Delete, RawInputModifiers.None, PhysicalKey.Delete, null);
Waiters.PumpUI();
await Waiters.WaitForAsync(() => vm.AssemblyTreeModel.AssemblyList!.GetAssemblies().Length == 0,
timeout: TimeSpan.FromSeconds(20));
await Waiters.WaitForIdleAsync();
// Assert - nothing that described the deleted assemblies is left behind.
vm.AssemblyTreeModel.SelectedItems.Should().BeEmpty("the selected rows were deleted");
vm.DockWorkspace.BackHistory.Should().BeEmpty("every entry pointed into a deleted assembly");
var tab = vm.DockWorkspace.ActiveDecompilerTab;
tab.Should().NotBeNull();
tab!.Text.Should().BeEmpty("the member it showed is gone");
tab.Title.Should().Be("(unnamed)", "a tab that shows nothing must not still name what it showed");
}
}

36
ILSpy/AssemblyTree/AssemblyTreeModel.cs

@ -29,6 +29,8 @@ using System.Runtime.Serialization; @@ -29,6 +29,8 @@ using System.Runtime.Serialization;
using System.Threading;
using System.Threading.Tasks;
using Avalonia.Threading;
using CommunityToolkit.Mvvm.ComponentModel;
using ICSharpCode.Decompiler;
@ -527,7 +529,17 @@ namespace ICSharpCode.ILSpy.AssemblyTree @@ -527,7 +529,17 @@ namespace ICSharpCode.ILSpy.AssemblyTree
// that subscribe to CurrentAssemblyListChangedEventArgs see add/remove events
// from the live list.
if (AssemblyList is { } previous)
{
previous.CollectionChanged -= OnActiveAssemblyListCollectionChanged;
// Everything below builds a new tree, so the selection, the open tabs and the
// navigation history all describe a list that is about to stop being shown.
// Nothing is removed from that list - it is the list itself that goes away - so
// no collection event announces it, and the panes are told with the same Reset
// that clearing a list raises, which is the one path that discards all of it.
SelectedItems.Clear();
Util.MessageBus.Send(this, new Util.CurrentAssemblyListChangedEventArgs(
new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset)));
}
AssemblyList = list;
list.CollectionChanged += OnActiveAssemblyListCollectionChanged;
if (list.GetAssemblies().Length == 0 && list.ListName == AssemblyListManager.DefaultListName)
@ -1027,11 +1039,35 @@ namespace ICSharpCode.ILSpy.AssemblyTree @@ -1027,11 +1039,35 @@ namespace ICSharpCode.ILSpy.AssemblyTree
}
Util.MessageBus.Send(this, new Util.CurrentAssemblyListChangedEventArgs(e));
// The removed assemblies took the selected node with them. Hand the selection to the
// nearest survivor so the tree does not come back empty-handed while it still has
// something to show; the tree view does this for its own Delete gesture, but a
// removal from anywhere else (the context menu, a command, a reload) would not.
// With nothing left there is nothing to select, and the panes stay empty.
if (e.OldItems is { Count: > 0 })
SelectSurvivorAfterRemoval(e.OldStartingIndex);
// List-dependent menu commands (Clear assembly list, Remove assemblies with load errors)
// re-evaluate CanExecute now that the list gained or lost entries.
Commands.CommandManager.InvalidateRequerySuggested();
}
/// <summary>
/// Puts the selection back on the row nearest to where the removed ones were, once the
/// tree has caught up with the removal. Does nothing while something is still selected -
/// only some of the removed assemblies held the selection, or none did.
/// </summary>
void SelectSurvivorAfterRemoval(int removedIndex)
{
Dispatcher.UIThread.Post(() => {
if (SelectedItems.Count > 0)
return;
if (Root is not { } root || root.Children.Count == 0)
return;
SelectNode(root.Children[Math.Clamp(removedIndex, 0, root.Children.Count - 1)]);
}, DispatcherPriority.Background);
}
// Coalesces burst F5 / programmatic Refresh() calls into a single async pipeline.
// Without the gate, two Refresh() in quick succession would run two parallel
// ShowAssemblyList + GetMetadataFileAsync cycles, doubling the work and producing

8
ILSpy/TextView/DecompilerTabPageModel.cs

@ -346,6 +346,14 @@ namespace ICSharpCode.ILSpy.TextView @@ -346,6 +346,14 @@ namespace ICSharpCode.ILSpy.TextView
n.PropertyChanged -= OnCurrentNodePropertyChanged;
currentNodes = value.ToArray();
cachedBaseTitle = ComposeBaseTitle();
if (currentNodes.Count == 0)
{
// Emptying the tab has to take the title with it. Nothing below assigns one
// for an empty tab - StartDecompile has nothing to decompile and returns -
// so the tab would keep naming the member it no longer shows. ComposeBaseTitle
// already names the empty case.
Title = cachedBaseTitle;
}
foreach (var n in currentNodes)
n.PropertyChanged += OnCurrentNodePropertyChanged;
// Let host chrome (the omnibar breadcrumb) react to the tab re-targeting a node.

Loading…
Cancel
Save