From aea59ee881ec5a682dd3a51a0dcf868ba47e8eaf Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 4 Jun 2026 21:25:39 +0200 Subject: [PATCH] Keep tree selection in sync after Delete-unload Pressing Delete unloaded the assembly but left the grid showing a row selected while model.SelectedItems went empty -- so a second Delete read an empty selection and no-op'd ("spamming Delete breaks"). Record the deleted node's flattened index and, once the visible tree rebuilds, re-select the node that now occupies that slot (clamped to the new end) via SelectNode, which re-syncs grid and model. Selects nothing when the list empties. --- ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs | 38 ++++++++++++++ ILSpy/AssemblyTree/AssemblyListPane.axaml.cs | 49 ++++++++++++++++--- 2 files changed, 81 insertions(+), 6 deletions(-) diff --git a/ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs b/ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs index 85ddb085c..ded727b45 100644 --- a/ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs +++ b/ILSpy.Tests/AssemblyList/AssemblyTreeTests.cs @@ -818,6 +818,44 @@ public class AssemblyTreeTests description: "DataGrid item count should drop after the assembly is unloaded"); } + [AvaloniaTest] + public async Task Delete_Reselects_The_Next_Node_So_Repeated_Delete_Keeps_Working() + { + // Regression for the "spamming Delete breaks" bug: Unload cleared the model selection but + // the grid kept showing a row selected, so model.SelectedItems went empty and the next + // Delete read an empty selection and no-op'd. Delete must re-select the nearest remaining + // node (grid + model in sync) so repeated Delete keeps unloading. + var (window, vm) = await TestHarness.BootAsync(3); + var model = vm.AssemblyTreeModel; + + var firstAssembly = model.FindNode("System.Linq"); + model.SelectNode(firstAssembly); + await Waiters.WaitForAsync(() => ReferenceEquals(model.SelectedItem, firstAssembly)); + + var pane = await window.WaitForComponent(); + var grid = await pane.WaitForComponent(); + grid.Focus(); + Dispatcher.UIThread.RunJobs(); + + int loadedBefore = model.AssemblyList!.GetAssemblies().Count(); + + window.KeyPress(Key.Delete, RawInputModifiers.None, PhysicalKey.Delete, null); + await Waiters.WaitForAsync(() => model.AssemblyList!.GetAssemblies().Count() == loadedBefore - 1); + Dispatcher.UIThread.RunJobs(); + + // The bug: after the first Delete the selection must still point at a (different) node. + model.SelectedItems.Should().NotBeEmpty( + "after Delete the selection must move to the nearest remaining node, not unset"); + ((object?)model.SelectedItem).Should().NotBeNull(); + ReferenceEquals(model.SelectedItem, firstAssembly).Should().BeFalse( + "the deleted node must not stay selected"); + + // Spamming Delete: a second press must unload another assembly (it couldn't before). + window.KeyPress(Key.Delete, RawInputModifiers.None, PhysicalKey.Delete, null); + await Waiters.WaitForAsync(() => model.AssemblyList!.GetAssemblies().Count() == loadedBefore - 2, + description: "a second Delete must unload another assembly; the selection didn't desync"); + } + [AvaloniaTest] public async Task Clear_Assembly_List_Command_Empties_The_Active_List() { diff --git a/ILSpy/AssemblyTree/AssemblyListPane.axaml.cs b/ILSpy/AssemblyTree/AssemblyListPane.axaml.cs index e1e9c88f6..bcb5d280c 100644 --- a/ILSpy/AssemblyTree/AssemblyListPane.axaml.cs +++ b/ILSpy/AssemblyTree/AssemblyListPane.axaml.cs @@ -295,14 +295,19 @@ namespace ILSpy.AssemblyTree { // Snapshot before mutation — Unload mutates the list and indirectly the // model's selection. - var assemblies = model.SelectedItems.OfType() - .Select(n => n.LoadedAssembly) - .ToList(); - if (assemblies.Count == 0) + var selectedAssemblyNodes = model.SelectedItems.OfType().ToList(); + if (selectedAssemblyNodes.Count == 0) return; - foreach (var asm in assemblies) - list.Unload(asm); + // Remember where the topmost selected assembly sits in the visible (flattened) tree + // so we can re-select its neighbour after the unload. Without this, Unload clears the + // model selection while the grid keeps a row visually selected -- the two desync and + // the next Delete reads an empty selection and no-ops ("spamming Delete breaks"). + int reselectIndex = FlattenedIndexOf(selectedAssemblyNodes[0]); + foreach (var node in selectedAssemblyNodes) + list.Unload(node.LoadedAssembly); e.Handled = true; + // The flattened list rebuilds in response to the list change; re-select after it settles. + Dispatcher.UIThread.Post(() => ReselectAfterDelete(reselectIndex), DispatcherPriority.Background); return; } if (e.Key == Key.R && e.KeyModifiers == KeyModifiers.Control) @@ -322,6 +327,38 @@ namespace ILSpy.AssemblyTree } } + // Flattened (visible) index of a tree node, or -1 if not currently realized/visible. + int FlattenedIndexOf(SharpTreeNode node) + { + if (TreeGrid.HierarchicalModel is not IHierarchicalModel hm || hm.FindNode(node) is not { } hNode) + return -1; + var flattened = hm.Flattened; + for (int i = 0; i < flattened.Count; i++) + { + if (ReferenceEquals(flattened[i], hNode)) + return i; + } + return -1; + } + + // After a Delete-unload, put the selection back on the node that now occupies the deleted + // node's slot (clamped to the new end), so grid and model stay in sync and repeated Delete + // keeps working. Selects nothing when the list is empty. + void ReselectAfterDelete(int index) + { + if (DataContext is not AssemblyTreeModel model) + return; + if (TreeGrid.HierarchicalModel is not IHierarchicalModel hm) + return; + var flattened = hm.Flattened; + if (flattened.Count == 0 || index < 0) + { + model.SelectNode(null); + return; + } + model.SelectNode(flattened[System.Math.Clamp(index, 0, flattened.Count - 1)].Item as SharpTreeNode); + } + static ILSpy.Analyzers.AnalyzerTreeViewModel? TryGetAnalyzerTreeViewModel() { try