From fa464968e1bb7ab84939d31d4dcd73eff3f44588 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Mon, 14 Sep 2026 05:59:35 +0200 Subject: [PATCH] Reorder the tree rows when the assembly list is sorted Sorting reorders the assembly list in place and reports it as a Move, which nothing downstream could act on: the tree node's handler had cases for Add, Remove and Reset only, and neither the child collection nor the flattener had a move at all. The rows therefore kept their pre-sort order until something else forced a rebuild. Moving the node rather than removing and re-inserting it keeps its identity, so an expanded subtree stays expanded and its row is not rebuilt. A run longer than one row has to be reported the way the consumer reads it: Avalonia's VirtualizingStackPanel applies a ranged move by removing OldItems.Count rows and re-inserting them at NewStartingIndex - (Count - 1), so a run reported by its final start index lands short by its own length. For a single row - a collapsed node, and every move a sort makes - the two readings coincide. Assisted-by: Claude:claude-opus-5[1m]:Claude Code --- ICSharpCode.ILSpyX/TreeView/SharpTreeNode.cs | 34 ++++++ .../TreeView/SharpTreeNodeCollection.cs | 22 ++++ ICSharpCode.ILSpyX/TreeView/TreeFlattener.cs | 23 ++++ ILSpy.Tests/Controls/FlatListTreeNodeTests.cs | 112 ++++++++++++++++++ ILSpy.Tests/Controls/SharpTreeViewTests.cs | 28 +++++ ILSpy/TreeNodes/AssemblyListTreeNode.cs | 6 + 6 files changed, 225 insertions(+) diff --git a/ICSharpCode.ILSpyX/TreeView/SharpTreeNode.cs b/ICSharpCode.ILSpyX/TreeView/SharpTreeNode.cs index 8f22c7801..b7ff6f716 100644 --- a/ICSharpCode.ILSpyX/TreeView/SharpTreeNode.cs +++ b/ICSharpCode.ILSpyX/TreeView/SharpTreeNode.cs @@ -201,6 +201,15 @@ namespace ICSharpCode.ILSpyX.TreeView public virtual void OnChildrenChanged(NotifyCollectionChangedEventArgs e) { + if (e.Action == NotifyCollectionChangedAction.Move) + { + // A move keeps the node and its parent, so none of the attach/detach work below + // applies: only the node's position in the flat list changes, and it takes its + // visible descendants with it as one run. + MoveChild((SharpTreeNode)e.OldItems![0]!, e.NewStartingIndex); + RaiseIsLastChangedIfNeeded(e); + return; + } if (e.OldItems != null) { foreach (SharpTreeNode node in e.OldItems) @@ -270,6 +279,31 @@ namespace ICSharpCode.ILSpyX.TreeView RaisePropertyChanged(nameof(ShowExpander)); RaiseIsLastChangedIfNeeded(e); } + + void MoveChild(SharpTreeNode node, int newIndex) + { + Debug.Assert(node.modelParent == this); + if (!node.isVisible) + { + // Not part of the flat list, so the reorder of modelChildren is all there is to do. + return; + } + int oldVisibleIndex = GetVisibleIndexForNode(node); + List movedNodes = node.VisibleDescendantsAndSelf().ToList(); + SharpTreeNode moveEnd = node; + while (moveEnd.modelChildren != null && moveEnd.modelChildren.Count > 0) + moveEnd = moveEnd.modelChildren.Last(); + RemoveNodes(node, moveEnd); + + // Same rule as insertion: the node goes after its predecessor's last descendant, or + // directly after this parent when it becomes the first child. + SharpTreeNode? insertionPos = newIndex == 0 ? null : modelChildren?[newIndex - 1]; + while (insertionPos != null && insertionPos.modelChildren != null && insertionPos.modelChildren.Count > 0) + insertionPos = insertionPos.modelChildren.Last(); + InsertNodeAfter(insertionPos ?? this, node); + + GetListRoot().treeFlattener?.NodesMoved(oldVisibleIndex, GetVisibleIndexForNode(node), movedNodes); + } #endregion #region Expanding / LazyLoading diff --git a/ICSharpCode.ILSpyX/TreeView/SharpTreeNodeCollection.cs b/ICSharpCode.ILSpyX/TreeView/SharpTreeNodeCollection.cs index a34e22915..970981e16 100644 --- a/ICSharpCode.ILSpyX/TreeView/SharpTreeNodeCollection.cs +++ b/ICSharpCode.ILSpyX/TreeView/SharpTreeNodeCollection.cs @@ -146,6 +146,28 @@ namespace ICSharpCode.ILSpyX.TreeView OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, newNodes, index)); } + /// + /// Moves the node at to , where + /// is the position the node ends up at in the reordered + /// collection. Raised as a single , so the + /// node keeps its identity: a consumer that would throw away per-item state on a + /// remove/insert pair - selection, an expanded subtree, a container - keeps it. + /// + public void Move(int oldIndex, int newIndex) + { + ThrowOnReentrancy(); + if ((uint)oldIndex >= (uint)list.Count) + throw new ArgumentOutOfRangeException(nameof(oldIndex)); + if ((uint)newIndex >= (uint)list.Count) + throw new ArgumentOutOfRangeException(nameof(newIndex)); + if (oldIndex == newIndex) + return; + var node = list[oldIndex]; + list.RemoveAt(oldIndex); + list.Insert(newIndex, node); + OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Move, node, newIndex, oldIndex)); + } + public void RemoveAt(int index) { ThrowOnReentrancy(); diff --git a/ICSharpCode.ILSpyX/TreeView/TreeFlattener.cs b/ICSharpCode.ILSpyX/TreeView/TreeFlattener.cs index 67ae9200d..afb37fe90 100644 --- a/ICSharpCode.ILSpyX/TreeView/TreeFlattener.cs +++ b/ICSharpCode.ILSpyX/TreeView/TreeFlattener.cs @@ -76,6 +76,29 @@ namespace ICSharpCode.ILSpyX.TreeView RaiseCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Remove, list, index)); } + // The moved node's run keeps its identity here as well: one ranged Move instead of a + // Remove/Add pair, so a consumer that tracks items rather than indices (selection, realized + // containers) survives a reorder. Both indices are positions in the list as it stands before + // the move, which is what NotifyCollectionChangedEventArgs specifies for a move. + public void NodesMoved(int oldIndex, int newIndex, IEnumerable nodes) + { + if (!includeRoot) + { + oldIndex--; + newIndex--; + } + IList list = nodes as IList ?? new List(nodes); + if (list.Count == 0 || oldIndex == newIndex) + return; + // A forward move reports where the run ENDS up, not where it starts. The consumer + // (Avalonia's VirtualizingStackPanel) applies a ranged move by removing OldItems.Count + // rows at OldStartingIndex and re-inserting them at NewStartingIndex - (Count - 1), so a + // run reported by its final start index lands short by its own length. For a single row - + // a collapsed node, and every move the assembly list makes - the two readings coincide. + int reportedNewIndex = newIndex > oldIndex ? newIndex + list.Count - 1 : newIndex; + RaiseCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Move, list, reportedNewIndex, oldIndex)); + } + public void Stop() { Debug.Assert(root.treeFlattener == this); diff --git a/ILSpy.Tests/Controls/FlatListTreeNodeTests.cs b/ILSpy.Tests/Controls/FlatListTreeNodeTests.cs index d3f4492d0..c70d9403b 100644 --- a/ILSpy.Tests/Controls/FlatListTreeNodeTests.cs +++ b/ILSpy.Tests/Controls/FlatListTreeNodeTests.cs @@ -17,6 +17,9 @@ // DEALINGS IN THE SOFTWARE. using System; +using System.Collections.Generic; +using System.Collections.Specialized; +using System.Linq; using AwesomeAssertions; @@ -69,4 +72,113 @@ public class FlatListTreeNodeTests Assert.That(SharpTreeNode.GetNodeByVisibleIndex(listRoot, 0), Is.SameAs(root)); Assert.That(SharpTreeNode.GetNodeByVisibleIndex(listRoot, 1), Is.SameAs(child)); } + + [Test] + public void Move_ReordersChildrenAndRaisesOneMoveEvent() + { + var root = new TestNode("root"); + var a = new TestNode("a"); + var b = new TestNode("b"); + var c = new TestNode("c"); + root.Children.AddRange(new[] { a, b, c }); + var events = new List(); + root.Children.CollectionChanged += (_, e) => events.Add(e); + + root.Children.Move(2, 0); + + root.Children.Should().Equal(c, a, b); + events.Should().ContainSingle(); + events[0].Action.Should().Be(NotifyCollectionChangedAction.Move); + events[0].OldStartingIndex.Should().Be(2); + events[0].NewStartingIndex.Should().Be(0); + events[0].OldItems!.Cast().Should().Equal(c); + events[0].NewItems!.Cast().Should().Equal(c); + } + + [Test] + public void Move_ToSameIndex_DoesNothing() + { + var root = new TestNode("root"); + var a = new TestNode("a"); + var b = new TestNode("b"); + root.Children.AddRange(new[] { a, b }); + var events = new List(); + root.Children.CollectionChanged += (_, e) => events.Add(e); + + root.Children.Move(1, 1); + + root.Children.Should().Equal(a, b); + events.Should().BeEmpty(); + } + + [Test] + public void Move_UpdatesTheFlattenedOrder() + { + var root = new TestNode("root"); + var a = new TestNode("a"); + var b = new TestNode("b"); + var c = new TestNode("c"); + root.Children.AddRange(new[] { a, b, c }); + root.IsExpanded = true; + var flattener = new TreeFlattener(root, includeRoot: true); + + root.Children.Move(2, 0); + + Flatten(flattener).Should().Equal(root, c, a, b); + } + + [Test] + public void Move_OfAnExpandedNode_MovesItsWholeRun() + { + var root = new TestNode("root"); + var a = new TestNode("a"); + var a1 = new TestNode("a1"); + var a2 = new TestNode("a2"); + var b = new TestNode("b"); + a.Children.AddRange(new[] { a1, a2 }); + root.Children.AddRange(new[] { a, b }); + root.IsExpanded = true; + a.IsExpanded = true; + var flattener = new TreeFlattener(root, includeRoot: true); + Flatten(flattener).Should().Equal(root, a, a1, a2, b); + + root.Children.Move(0, 1); + + Flatten(flattener).Should().Equal(root, b, a, a1, a2); + } + + [Test] + public void Move_RaisesOneRangedMoveOnTheFlattener() + { + var root = new TestNode("root"); + var a = new TestNode("a"); + var a1 = new TestNode("a1"); + var b = new TestNode("b"); + a.Children.Add(a1); + root.Children.AddRange(new[] { a, b }); + root.IsExpanded = true; + a.IsExpanded = true; + var flattener = new TreeFlattener(root, includeRoot: true); + var events = new List(); + flattener.CollectionChanged += (_, e) => events.Add(e); + + // root, a, a1, b -> root, b, a, a1: the run [a, a1] moves from index 1 to index 2. + root.Children.Move(0, 1); + + events.Should().ContainSingle(); + events[0].Action.Should().Be(NotifyCollectionChangedAction.Move); + events[0].OldItems!.Cast().Should().Equal(a, a1); + events[0].OldStartingIndex.Should().Be(1); + // A forward move of a multi-row run reports the row the run ends on, which is how the + // consumer re-inserts it; a1 is the last row of [a, a1] and ends up at index 3. + events[0].NewStartingIndex.Should().Be(3); + } + + static List Flatten(TreeFlattener flattener) + { + var result = new List(); + for (int i = 0; i < flattener.Count; i++) + result.Add(flattener[i]); + return result; + } } diff --git a/ILSpy.Tests/Controls/SharpTreeViewTests.cs b/ILSpy.Tests/Controls/SharpTreeViewTests.cs index ba87155e1..a8bcee715 100644 --- a/ILSpy.Tests/Controls/SharpTreeViewTests.cs +++ b/ILSpy.Tests/Controls/SharpTreeViewTests.cs @@ -16,6 +16,7 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. +using System.Collections.Generic; using System.Linq; using Avalonia.Controls; @@ -272,4 +273,31 @@ public class SharpTreeViewTests Children.Add(new TestNode($"{text}_{i}")); } } + + [AvaloniaTest] + public void Moving_An_Expanded_Node_Reorders_The_Rendered_Rows() + { + var (_, tree, root) = Host(); + var b = (TestNode)root.Children[1]; + b.IsExpanded = true; + Dispatcher.UIThread.RunJobs(); + RenderedRows(tree).Should().Equal("A", "B", "B1", "C"); + + // B carries B1 with it: the run [B, B1] moves past C. + root.Children.Move(1, 2); + Dispatcher.UIThread.RunJobs(); + + RenderedRows(tree).Should().Equal("A", "C", "B", "B1"); + } + + static List RenderedRows(SharpTreeView tree) + { + var rows = new List(); + for (int i = 0; i < tree.ItemCount; i++) + { + var container = tree.ContainerFromIndex(i); + rows.Add((container?.DataContext as SharpTreeNode)?.Text?.ToString() ?? ""); + } + return rows; + } } diff --git a/ILSpy/TreeNodes/AssemblyListTreeNode.cs b/ILSpy/TreeNodes/AssemblyListTreeNode.cs index c8ded3b17..e47c8846b 100644 --- a/ILSpy/TreeNodes/AssemblyListTreeNode.cs +++ b/ILSpy/TreeNodes/AssemblyListTreeNode.cs @@ -48,6 +48,12 @@ namespace ICSharpCode.ILSpy.TreeNodes case NotifyCollectionChangedAction.Remove: Children.RemoveRange(e.OldStartingIndex, e.OldItems!.Count); break; + case NotifyCollectionChangedAction.Move: + // Sorting the list reorders it in place. Mirror that as a move rather than a + // remove/insert pair, so the node - and with it the selection, the expanded + // subtree below it and its row - survives the reorder. + Children.Move(e.OldStartingIndex, e.NewStartingIndex); + break; case NotifyCollectionChangedAction.Reset: Children.Clear(); Children.AddRange(assemblyList.GetAssemblies().Select(a => new AssemblyTreeNode(a)));