Browse Source

Sync the tree selection to the active document tab

Switching to a document tab is supposed to pull the tree selection over to
what that tab shows, but it didn't reliably -- the gap was masked until the
right-click change stopped moving the selection on its own. Two problems:

The SelectedItem setter replaced the collection with Clear()+Add(), and the
transient empty step made the grid sync defer its completion flag, which then
suppressed the sync for the real new value -- so the tree visual stopped
following tab activation. Route every selection replacement through a single
batched SelectNodes() that fires the selection-changed fan-out once, with the
final set, so consumers never observe the transient empty (or a transient
multi, which would break metadata-tab reuse).

And a tab decompiled from several nodes carries no single SourceNode, so
activating it restored nothing. Restore the tab's full node set from
CurrentNodes (one or many), and mirror a multi-selection into the grid so every
restored node is highlighted, not just the primary.

Assisted-by: Claude:claude-opus-4-8:Claude Code
pull/3755/head
Siegfried Pammer 1 month ago
parent
commit
34e6ef11b6
  1. 165
      ILSpy.Tests/ContextMenus/DecompileInNewViewTests.cs
  2. 88
      ILSpy/AssemblyTree/AssemblyListPane.axaml.cs
  3. 60
      ILSpy/AssemblyTree/AssemblyTreeModel.cs
  4. 15
      ILSpy/Docking/DockWorkspace.cs

165
ILSpy.Tests/ContextMenus/DecompileInNewViewTests.cs

@ -283,7 +283,172 @@ public class DecompileInNewViewTests @@ -283,7 +283,172 @@ public class DecompileInNewViewTests
"B's highlight must be gone");
}
[AvaloniaTest]
public async Task Opening_A_Node_In_A_New_Tab_Syncs_The_Tree_Selection_To_It()
{
// Activating the new tab must pull the tree selection over to its node -- both the model
// selection AND the grid's visual selection. Previously masked because the right-click
// already moved the selection; now that right-click leaves it put, the tab-activation
// sync has to do the work.
var window = AppComposition.Current.GetExport<MainWindow>();
window.Show();
var vm = (MainWindowViewModel)window.DataContext!;
await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3);
var pane = await window.WaitForComponent<AssemblyListPane>();
var grid = await pane.WaitForComponent<DataGrid>();
var nodeA = vm.AssemblyTreeModel.FindNode<AssemblyTreeNode>("System.Linq");
var nodeB = vm.AssemblyTreeModel.FindNode<AssemblyTreeNode>(TreeNavigation.CoreLibName);
vm.AssemblyTreeModel.SelectNode(nodeA);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
for (int i = 0; i < 6; i++)
{
Dispatcher.UIThread.RunJobs();
grid.UpdateLayout();
await Task.Delay(20);
}
var registry = AppComposition.Current.GetExport<ContextMenuEntryRegistry>();
var menu = pane.BuildContextMenuForCurrentState(registry.Entries, rightClickedNode: nodeB);
menu!.ClickItem(Resources.DecompileToNewPanel);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
for (int i = 0; i < 6; i++)
{
Dispatcher.UIThread.RunJobs();
grid.UpdateLayout();
await Task.Delay(20);
}
// The model selection follows the new active tab...
ReferenceEquals(vm.AssemblyTreeModel.SelectedItem, nodeB).Should().BeTrue(
"the tree model selection must follow the newly-activated tab");
// ...and so does the grid's visual selection (the grid stores either the raw node or its
// HierarchicalNode wrapper depending on the path that set it).
ReferenceEquals(GridSelectedNode(grid), nodeB).Should().BeTrue(
"the grid's visual selection must follow the newly-activated tab");
}
[AvaloniaTest]
public async Task Activating_A_Single_Node_Tab_Syncs_The_Tree_After_A_Multi_Node_Selection()
{
// Regression: once the tree has held a multi-selection (e.g. a tab decompiled from
// several nodes), activating a single-node tab must still pull the tree selection over.
// The SelectedItem setter replaces the collection via Clear()+Add(); for the count>1 case
// that exposed a transient empty selection, whose grid sync deferred its done-flag and
// then suppressed the sync for the real value -- so the tree stopped following the tab.
var window = AppComposition.Current.GetExport<MainWindow>();
window.Show();
var vm = (MainWindowViewModel)window.DataContext!;
await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3);
var pane = await window.WaitForComponent<AssemblyListPane>();
var grid = await pane.WaitForComponent<DataGrid>();
var assemblies = vm.AssemblyTreeModel.Root!.Children.OfType<AssemblyTreeNode>().Take(3).ToArray();
assemblies.Length.Should().BeGreaterThanOrEqualTo(3, "need three top-level rows");
var nodeA = assemblies[0];
var nodeB = assemblies[1];
var nodeC = assemblies[2];
// Hold a multi-selection (A + B), as a multi-node decompile tab would.
vm.AssemblyTreeModel.SelectedItems.Clear();
vm.AssemblyTreeModel.SelectedItems.Add(nodeA);
vm.AssemblyTreeModel.SelectedItems.Add(nodeB);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
for (int i = 0; i < 6; i++)
{
Dispatcher.UIThread.RunJobs();
grid.UpdateLayout();
await Task.Delay(20);
}
vm.AssemblyTreeModel.SelectedItems.Count.Should().Be(2, "precondition: a multi-selection is held");
// Open C in a new tab -> activates it -> the tree must follow to C.
var registry = AppComposition.Current.GetExport<ContextMenuEntryRegistry>();
var menu = pane.BuildContextMenuForCurrentState(registry.Entries, rightClickedNode: nodeC);
menu!.ClickItem(Resources.DecompileToNewPanel);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
for (int i = 0; i < 6; i++)
{
Dispatcher.UIThread.RunJobs();
grid.UpdateLayout();
await Task.Delay(20);
}
ReferenceEquals(vm.AssemblyTreeModel.SelectedItem, nodeC).Should().BeTrue(
"the tree model selection must follow the newly-activated single-node tab");
vm.AssemblyTreeModel.SelectedItems.Count.Should().Be(1,
"activating the single-node tab must collapse the multi-selection to that one node");
ReferenceEquals(GridSelectedNode(grid), nodeC).Should().BeTrue(
"the grid's visual selection must follow even after a prior multi-selection");
}
[AvaloniaTest]
public async Task Activating_A_Multi_Node_Tab_Restores_Its_Multi_Selection_In_The_Tree()
{
// A tab decompiled from several nodes carries no single SourceNode, so activating it has
// to restore the WHOLE set in the tree -- not nothing, and not just one. Scenario: select
// two nodes (multi-node preview tab), open a third in a new tab, then re-activate the
// multi-node tab; the tree selection must come back to both original nodes.
var window = AppComposition.Current.GetExport<MainWindow>();
window.Show();
var vm = (MainWindowViewModel)window.DataContext!;
await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3);
var pane = await window.WaitForComponent<AssemblyListPane>();
var grid = await pane.WaitForComponent<DataGrid>();
var documents = ((ILSpyDockFactory)vm.DockWorkspace.Factory).Documents!;
var assemblies = vm.AssemblyTreeModel.Root!.Children.OfType<AssemblyTreeNode>().Take(3).ToArray();
assemblies.Length.Should().BeGreaterThanOrEqualTo(3, "need three top-level rows");
var nodeA = assemblies[0];
var nodeB = assemblies[1];
var nodeC = assemblies[2];
// 1) Multi-select A + B -> the active (preview) tab decompiles both.
vm.AssemblyTreeModel.SelectedItems.Clear();
vm.AssemblyTreeModel.SelectedItems.Add(nodeA);
vm.AssemblyTreeModel.SelectedItems.Add(nodeB);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
var multiTab = (ContentTabPage)documents.ActiveDockable!;
// 2) Open C in a new (single-node) tab -> it becomes active, tree -> C.
var registry = AppComposition.Current.GetExport<ContextMenuEntryRegistry>();
var menu = pane.BuildContextMenuForCurrentState(registry.Entries, rightClickedNode: nodeC);
menu!.ClickItem(Resources.DecompileToNewPanel);
await vm.DockWorkspace.WaitForDecompiledTextAsync();
ReferenceEquals(multiTab, documents.ActiveDockable).Should().BeFalse(
"precondition: the new single-node tab is active, not the multi-node one");
// 3) Re-activate the multi-node tab.
vm.DockWorkspace.Factory.SetActiveDockable(multiTab);
for (int i = 0; i < 8; i++)
{
Dispatcher.UIThread.RunJobs();
grid.UpdateLayout();
await Task.Delay(20);
}
// 4) The tree selection must contain BOTH original nodes again -- in the model...
vm.AssemblyTreeModel.SelectedItems.Should().Contain(nodeA)
.And.Contain(nodeB);
vm.AssemblyTreeModel.SelectedItems.Count.Should().Be(2,
"re-activating the multi-node tab restores exactly its two nodes");
// ...and visually in the grid (both rows highlighted, not just the primary).
GridSelectedNodes(grid).Should().Contain(nodeA).And.Contain(nodeB);
GridSelectedNodes(grid).Should().HaveCount(2,
"the grid must visually highlight every restored node");
}
static bool RowNodeEquals(DataGridRow row, SharpTreeNode node)
=> row.DataContext is HierarchicalNode hn && ReferenceEquals(hn.Item, node);
static SharpTreeNode? GridSelectedNode(DataGrid grid)
=> grid.SelectedItem is HierarchicalNode hn ? hn.Item as SharpTreeNode : grid.SelectedItem as SharpTreeNode;
static System.Collections.Generic.List<SharpTreeNode> GridSelectedNodes(DataGrid grid)
=> grid.SelectedItems.Cast<object?>()
.Select(o => o is HierarchicalNode hn ? hn.Item as SharpTreeNode : o as SharpTreeNode)
.Where(n => n != null)
.Select(n => n!)
.ToList();
}

88
ILSpy/AssemblyTree/AssemblyListPane.axaml.cs

@ -383,40 +383,90 @@ namespace ILSpy.AssemblyTree @@ -383,40 +383,90 @@ namespace ILSpy.AssemblyTree
if (TreeGrid.HierarchicalModel is not IHierarchicalModel hm)
return;
// Everything that can touch the grid -- including the ancestor Expand calls, which
// realise rows and can fire a SelectionChanged -- must run inside BeginSync so that
// echo doesn't bounce back into the model and disturb the in-flight navigation.
BeginSync();
try
{
var primaryWrapper = ExpandAncestors(hm, target);
if (primaryWrapper == null)
return;
if (!ReferenceEquals(TreeGrid.SelectedItem, target))
{
TreeGrid.SelectedItem = target;
// Defer past Expand's pending child-realization notifications — synchronously
// the wrapper isn't in the visible list yet.
var scrollTarget = primaryWrapper;
Dispatcher.UIThread.Post(
() => CenterRowInView(scrollTarget),
DispatcherPriority.Background);
}
// Multi-selection: mirror every OTHER selected node into the grid so a tab
// restored from several nodes highlights all of them, not just the primary set
// above. Single selection makes this a no-op.
if (DataContext is AssemblyTreeModel model && model.SelectedItems.Count > 1)
MirrorMultiSelectionToGrid(hm, model, target);
}
finally
{
EndSync();
}
}
// Expands every ancestor of <paramref name="node"/> so its row is realised, returning the
// node's own wrapper (or null if any level can't be located yet).
static HierarchicalNode? ExpandAncestors(IHierarchicalModel hm, SharpTreeNode node)
{
var path = new System.Collections.Generic.List<SharpTreeNode>();
for (var n = target; n.Parent != null; n = n.Parent)
for (var n = node; n.Parent != null; n = n.Parent)
path.Add(n);
path.Reverse();
// Each ancestor must be expanded before FindNode can locate the next level's wrapper.
BeginSync();
HierarchicalNode? hNode = null;
for (int i = 0; i < path.Count; i++)
{
hNode = hm.FindNode(path[i]);
if (hNode == null)
{
EndSync();
return;
}
return null;
if (i < path.Count - 1 && !hNode.IsExpanded)
hm.Expand(hNode);
}
return hNode;
}
if (ReferenceEquals(TreeGrid.SelectedItem, target))
// Brings the grid's selected set in line with the model's multi-selection: adds each
// selected node's wrapper that isn't highlighted yet (expanding its ancestors first) and
// drops any grid row no longer selected in the model. The primary is the caller's job.
void MirrorMultiSelectionToGrid(IHierarchicalModel hm, AssemblyTreeModel model, SharpTreeNode primary)
{
EndSync();
return;
foreach (var node in model.SelectedItems)
{
if (ReferenceEquals(node, primary))
continue;
var wrapper = ExpandAncestors(hm, node);
if (wrapper != null && !GridSelectionContains(node))
TreeGrid.SelectedItems.Add(wrapper);
}
TreeGrid.SelectedItem = target;
// Pass the HierarchicalNode wrapper (DataGrid.IndexOf is against the wrappers, not
// the underlying SharpTreeNodes), and defer past Expand's pending child-realization
// notifications — synchronously the wrapper isn't in the visible list yet.
var scrollTarget = hNode!;
Dispatcher.UIThread.Post(
() => CenterRowInView(scrollTarget),
DispatcherPriority.Background);
EndSync();
for (int i = TreeGrid.SelectedItems.Count - 1; i >= 0; i--)
{
var node = GridItemNode(TreeGrid.SelectedItems[i]);
if (node != null && !model.SelectedItems.Contains(node))
TreeGrid.SelectedItems.RemoveAt(i);
}
}
static SharpTreeNode? GridItemNode(object? item)
=> item is HierarchicalNode hn ? hn.Item as SharpTreeNode : item as SharpTreeNode;
bool GridSelectionContains(SharpTreeNode node)
{
foreach (var item in TreeGrid.SelectedItems)
{
if (ReferenceEquals(GridItemNode(item), node))
return true;
}
return false;
}
// DataGrid.ScrollIntoView only brings the row to the nearest viewport edge; we want

60
ILSpy/AssemblyTree/AssemblyTreeModel.cs

@ -88,10 +88,52 @@ namespace ILSpy.AssemblyTree @@ -88,10 +88,52 @@ namespace ILSpy.AssemblyTree
set {
if (SelectedItem == value)
return;
SelectNodes(value == null ? System.Array.Empty<SharpTreeNode>() : new[] { value });
}
}
/// <summary>
/// Replaces the whole selection with <paramref name="nodes"/> in ONE logical change. The
/// collection can't be swapped atomically (Clear()+Add() passes through a transient empty,
/// add-before-remove through a transient multi), so the selection-changed fan-out is
/// batched: per-element IsSelected toggling still happens, but the PropertyChanged / path /
/// message-bus notifications fire once, AFTER, with the final set. Without this a transient
/// empty poisons the grid sync's deferred guard (tree stops following tab activation) and a
/// transient multi confuses count-sensitive consumers (metadata-tab reuse). Drives both the
/// single-node <see cref="SelectedItem"/> setter and multi-node tab-activation restore.
/// </summary>
public void SelectNodes(IReadOnlyList<SharpTreeNode> nodes)
{
ArgumentNullException.ThrowIfNull(nodes);
if (SelectionMatches(nodes))
return;
batchingSelectionChange = true;
try
{
SelectedItems.Clear();
if (value != null)
SelectedItems.Add(value);
foreach (var node in nodes)
{
if (node != null && !SelectedItems.Contains(node))
SelectedItems.Add(node);
}
}
finally
{
batchingSelectionChange = false;
}
RaiseSelectionChanged();
}
bool SelectionMatches(IReadOnlyList<SharpTreeNode> nodes)
{
if (SelectedItems.Count != nodes.Count)
return false;
for (int i = 0; i < nodes.Count; i++)
{
if (!SelectedItems.Contains(nodes[i]))
return false;
}
return true;
}
[ObservableProperty]
@ -155,6 +197,11 @@ namespace ILSpy.AssemblyTree @@ -155,6 +197,11 @@ namespace ILSpy.AssemblyTree
catch { return null; }
}
// True while the SelectedItem setter is replacing the collection via Clear()+Add();
// suppresses the selection-changed fan-out until the final state is in place so consumers
// never observe the transient empty/multi mid-replace.
bool batchingSelectionChange;
void OnSelectedItemsChanged(object? sender, NotifyCollectionChangedEventArgs e)
{
if (e.NewItems != null)
@ -164,6 +211,15 @@ namespace ILSpy.AssemblyTree @@ -164,6 +211,15 @@ namespace ILSpy.AssemblyTree
foreach (SharpTreeNode n in e.OldItems)
n.IsSelected = false;
// During a SelectedItem-setter batch the fan-out is deferred to the single
// RaiseSelectionChanged() the setter issues once the final selection is in place.
if (batchingSelectionChange)
return;
RaiseSelectionChanged();
}
void RaiseSelectionChanged()
{
// SelectedItem is a wrapper over this collection — anyone bound to it must be
// notified, and the saved path must follow the new primary.
OnPropertyChanged(nameof(SelectedItem));

15
ILSpy/Docking/DockWorkspace.cs

@ -451,14 +451,23 @@ namespace ILSpy.Docking @@ -451,14 +451,23 @@ namespace ILSpy.Docking
// Carve-out tab → active: pull the tree's selection over to whatever entity the
// new active tab is showing. Fires on user-driven tab clicks (via the dock model's
// ActiveDockable setter) and on programmatic SetActiveDockable calls.
if (factory.Documents?.ActiveDockable is not ContentTabPage tab || tab.SourceNode is not { } node)
if (factory.Documents?.ActiveDockable is not ContentTabPage tab)
return;
if (ReferenceEquals(assemblyTreeModel.SelectedItem, node))
// Restore the tab's FULL selection. A decompiler tab carries its node(s) in
// CurrentNodes (one or many) — a multi-node tab has no single SourceNode, so reading
// CurrentNodes is what lets the tree restore the whole multi-selection. Other content
// (metadata, compare) carries a single SourceNode.
System.Collections.Generic.IReadOnlyList<SharpTreeNode> nodes;
if (UnwrapDecompilerTab(tab) is { CurrentNodes.Count: > 0 } dec)
nodes = dec.CurrentNodes;
else if (tab.SourceNode is { } single)
nodes = new[] { single };
else
return;
syncingTreeFromActiveTab = true;
try
{
assemblyTreeModel.SelectedItem = node;
assemblyTreeModel.SelectNodes(nodes);
}
finally
{

Loading…
Cancel
Save