Browse Source

Pin tab without spawning a new preview tab

VS-style preview-tab semantics, refined per user spec: pinning a tab
should freeze its contents in place but NOT immediately spawn a fresh
preview tab beside it. The new preview tab opens lazily, on the next
tree-node selection that finds the active tab frozen — at which point
the new content lands in a brand-new tab and the pinned one is left
untouched.

Assisted-by: Claude:claude-opus-4-7:Claude Code
pull/3755/head
Siegfried Pammer 2 months ago
parent
commit
5e4146c13a
  1. 137
      ILSpy.Tests/Docking/PreviewTabPromotionTests.cs
  2. 109
      ILSpy/Docking/DockWorkspace.cs
  3. 34
      ILSpy/Docking/ILSpyDockFactory.cs

137
ILSpy.Tests/Docking/PreviewTabPromotionTests.cs

@ -75,8 +75,12 @@ public class PreviewTabPromotionTests @@ -75,8 +75,12 @@ public class PreviewTabPromotionTests
}
[AvaloniaTest]
public async Task PinCurrentTab_Promotes_MainTab_And_Spawns_A_New_Preview_MainTab()
public async Task PinCurrentTab_Flips_IsPreview_Without_Spawning_A_New_Tab()
{
// New semantics: Pin only flips IsPreview=false on the current MainTab. No new
// preview tab spawns at pin time. A fresh preview tab opens lazily later, when a
// tree-selection change finds the active tab frozen — see
// Selecting_A_Different_Node_After_Pin_Opens_A_New_Preview_Tab below.
var window = AppComposition.Current.GetExport<MainWindow>();
window.Show();
var vm = (MainWindowViewModel)window.DataContext!;
@ -90,34 +94,36 @@ public class PreviewTabPromotionTests @@ -90,34 +94,36 @@ public class PreviewTabPromotionTests
var typeNode = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
"System.Linq", "System.Linq", "System.Linq.Enumerable");
vm.AssemblyTreeModel.SelectNode(typeNode);
await vm.DockWorkspace.WaitForDecompiledTextAsync(System.TimeSpan.FromSeconds(30));
await vm.DockWorkspace.WaitForDecompiledTextAsync(System.TimeSpan.FromSeconds(60));
ReferenceEquals(previousMainTab.SourceNode, typeNode).Should().BeTrue(
"baseline: tree selection populated MainTab with the chosen node");
var tabCountBefore = vm.DockWorkspace.Documents!.VisibleDockables!.Count;
// Pin.
vm.DockWorkspace.PinCurrentTab();
// Promoted tab keeps its content and identity, just loses preview status.
// Tab is now pinned, content preserved.
previousMainTab.IsPreview.Should().BeFalse(
"after pin, the previously-preview MainTab becomes a regular pinned tab");
ReferenceEquals(previousMainTab.SourceNode, typeNode).Should().BeTrue(
"pin must not throw away the tab's content");
// A new preview MainTab takes its place.
factory.MainTab.Should().NotBeSameAs(previousMainTab,
"factory.MainTab must rotate to a fresh preview tab after pin");
factory.MainTab!.IsPreview.Should().BeTrue(
"the new MainTab is the next preview slot");
// factory.MainTab still points at the same (now-pinned) tab — no rotation.
factory.MainTab.Should().BeSameAs(previousMainTab,
"pin alone must not rotate factory.MainTab — the pinned tab keeps the slot until a selection change spawns a new preview");
// Both tabs live in the documents dock.
var docTabs = vm.DockWorkspace.Documents!.VisibleDockables!.OfType<ContentTabPage>().ToList();
docTabs.Should().Contain(previousMainTab).And.Contain(factory.MainTab,
"the documents dock must hold both the promoted tab and the fresh MainTab");
// And critically: no new tab spawned.
vm.DockWorkspace.Documents!.VisibleDockables!.Count.Should().Be(tabCountBefore,
"pinning alone must not open a new tab; new tabs open lazily on the next selection change");
}
[AvaloniaTest]
public async Task After_Pin_Tree_Selection_Writes_To_New_MainTab_Not_Promoted_One()
public async Task Selecting_A_Different_Node_After_Pin_Opens_A_New_Preview_Tab()
{
// Pin holds the current tab's content. Selecting a different tree node while
// the (now-pinned) tab is active must spawn a fresh preview tab beside it and
// route the new content there — never overwrite the pinned tab.
var window = AppComposition.Current.GetExport<MainWindow>();
window.Show();
var vm = (MainWindowViewModel)window.DataContext!;
@ -129,23 +135,34 @@ public class PreviewTabPromotionTests @@ -129,23 +135,34 @@ public class PreviewTabPromotionTests
var typeA = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
"System.Linq", "System.Linq", "System.Linq.Enumerable");
vm.AssemblyTreeModel.SelectNode(typeA);
await vm.DockWorkspace.WaitForDecompiledTextAsync(System.TimeSpan.FromSeconds(30));
await vm.DockWorkspace.WaitForDecompiledTextAsync(System.TimeSpan.FromSeconds(60));
var pinnedTab = factory.MainTab!;
var pinnedContent = pinnedTab.Content;
// Phase 2: pin.
// Phase 2: pin. factory.MainTab still points at the same (now-pinned) tab —
// no spawn yet (covered by PinCurrentTab_Flips_IsPreview_Without_Spawning_A_New_Tab).
var tabCountBeforeSelection = vm.DockWorkspace.Documents!.VisibleDockables!.Count;
vm.DockWorkspace.PinCurrentTab();
var newMainTab = factory.MainTab!;
newMainTab.Should().NotBeSameAs(pinnedTab);
factory.MainTab.Should().BeSameAs(pinnedTab, "pin alone keeps the slot");
// Phase 3: select a different type — should land in the NEW MainTab.
// Phase 3: select a different type — NOW a new preview tab should spawn AND
// receive the new content.
var typeB = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
"System.Private.Uri", "System", "System.Uri");
vm.AssemblyTreeModel.SelectNode(typeB);
await vm.DockWorkspace.WaitForDecompiledTextAsync(System.TimeSpan.FromSeconds(30));
await vm.DockWorkspace.WaitForDecompiledTextAsync(System.TimeSpan.FromSeconds(60));
// New tab spawned beside the pinned one.
vm.DockWorkspace.Documents!.VisibleDockables!.Count.Should().Be(tabCountBeforeSelection + 1,
"selecting a different node while the active tab is pinned must spawn a new preview tab");
var newMainTab = factory.MainTab!;
newMainTab.Should().NotBeSameAs(pinnedTab,
"factory.MainTab must rotate to the freshly-spawned preview tab once a selection change forces it");
newMainTab.IsPreview.Should().BeTrue(
"the freshly-spawned tab is itself a preview tab (the user can pin it next)");
ReferenceEquals(newMainTab.SourceNode, typeB).Should().BeTrue(
"new tree selection must land in the freshly-spawned preview MainTab");
"new tree selection must land in the freshly-spawned preview tab");
ReferenceEquals(pinnedTab.SourceNode, typeA).Should().BeTrue(
"the pinned tab must keep type A — not be overwritten by the new selection");
pinnedTab.Content.Should().BeSameAs(pinnedContent,
@ -372,17 +389,19 @@ public class PreviewTabPromotionTests @@ -372,17 +389,19 @@ public class PreviewTabPromotionTests
pinButton.IsVisible.Should().BeTrue("pin button must show while the tab is preview");
var previousMainTab = factory.MainTab!;
var tabCountBefore = vm.DockWorkspace.Documents!.VisibleDockables!.Count;
// Simulate a user click on the pin button.
pinButton.RaiseEvent(new global::Avalonia.Interactivity.RoutedEventArgs(
global::Avalonia.Controls.Button.ClickEvent));
// The previous MainTab should now be pinned; factory.MainTab should point at a new
// preview tab.
// New semantics: clicking pin flips IsPreview, doesn't spawn a new tab.
previousMainTab.IsPreview.Should().BeFalse(
"clicking the pin button must promote the previous MainTab");
factory.MainTab.Should().NotBeSameAs(previousMainTab,
"factory.MainTab must rotate to a fresh preview tab after the click");
"clicking the pin button must flip IsPreview=false on the current MainTab");
factory.MainTab.Should().BeSameAs(previousMainTab,
"pin alone must not rotate factory.MainTab; the new preview tab opens lazily on the next tree selection");
vm.DockWorkspace.Documents!.VisibleDockables!.Count.Should().Be(tabCountBefore,
"pin alone must not change tab count");
}
[AvaloniaTest]
@ -450,25 +469,75 @@ public class PreviewTabPromotionTests @@ -450,25 +469,75 @@ public class PreviewTabPromotionTests
[AvaloniaTest]
public void PinCurrentTab_Is_Noop_When_MainTab_Already_Pinned()
{
// Idempotency: calling Pin twice in a row only promotes the first time. The second
// call sees IsPreview=false on the (new) MainTab — actually, on the *new* preview
// MainTab — so it should NOT promote that one. Wait — the new MainTab IS preview.
// So a second pin DOES promote it. Let me restate: a single pin promotes once;
// calling pin again creates a second pinned tab. That's the intended behaviour.
// The actual noop case is when there's literally nothing pinnable. Test that.
// Idempotency: a second pin against an already-pinned MainTab is a no-op. The new
// "pin = flip-only, spawn-lazy" semantics make this simpler than the old version:
// PinCurrentMainTab() returns null when MainTab.IsPreview is already false, and
// PinCurrentTab early-returns.
var window = AppComposition.Current.GetExport<MainWindow>();
window.Show();
var vm = (MainWindowViewModel)window.DataContext!;
var factory = (ILSpyDockFactory)vm.DockWorkspace.Factory;
// Manually flip the current MainTab to pinned, simulating a state where there's
// no preview to promote.
// Manually flip the current MainTab to pinned, simulating the post-pin state.
factory.MainTab!.IsPreview = false;
var before = factory.MainTab;
vm.DockWorkspace.PinCurrentTab();
factory.MainTab.Should().BeSameAs(before,
"with no preview MainTab to promote, PinCurrentTab must leave the factory state untouched");
"with no preview MainTab to flip, PinCurrentTab must leave the factory state untouched");
factory.MainTab.IsPreview.Should().BeFalse(
"the already-pinned tab stays pinned");
}
[AvaloniaTest]
public async Task Tree_Selection_While_Frozen_Tab_Active_Opens_A_New_Preview_Tab()
{
// "Frozen" covers: (a) user-pinned tabs (IsPreview=false via PinCurrentTab),
// (b) carve-out tabs (born IsPreview=false via OpenNodeInNewTab), and (c) static
// content tabs (Options, About) whose Content is a static viewmodel. Tree-node
// selections while any of those are active must spawn a fresh preview tab and
// route the new content there. This test uses a carve-out as a stand-in for the
// frozen-tab family — the IsWritablePreview check returns null for all three.
var window = AppComposition.Current.GetExport<MainWindow>();
window.Show();
var vm = (MainWindowViewModel)window.DataContext!;
await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 3);
var factory = (ILSpyDockFactory)vm.DockWorkspace.Factory;
// Open a carve-out (born pinned) and let it become the active dockable.
var typeA = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
"System.Linq", "System.Linq", "System.Linq.Enumerable");
vm.DockWorkspace.OpenNodeInNewTab(typeA);
var carveOut = vm.DockWorkspace.Documents!.VisibleDockables!
.OfType<ContentTabPage>()
.Last(t => t.SourceNode == typeA);
factory.SetActiveDockable(carveOut);
carveOut.IsPreview.Should().BeFalse("baseline: carve-out tab is frozen");
ReferenceEquals(vm.DockWorkspace.Documents.ActiveDockable, carveOut).Should().BeTrue(
"baseline: the carve-out is the active document");
var tabCountBefore = vm.DockWorkspace.Documents!.VisibleDockables!.Count;
// Select a different tree node — the carve-out is frozen, so this must NOT
// overwrite it.
var typeB = vm.AssemblyTreeModel.FindNode<TypeTreeNode>(
"System.Private.Uri", "System", "System.Uri");
vm.AssemblyTreeModel.SelectNode(typeB);
await vm.DockWorkspace.WaitForDecompiledTextAsync(System.TimeSpan.FromSeconds(60));
vm.DockWorkspace.Documents!.VisibleDockables!.Count.Should().Be(tabCountBefore + 1,
"selecting a tree node while a frozen tab is active must spawn a new preview tab");
ReferenceEquals(carveOut.SourceNode, typeA).Should().BeTrue(
"the frozen carve-out tab must keep its original content");
var newPreview = factory.MainTab!;
newPreview.Should().NotBeSameAs(carveOut,
"the freshly-spawned preview must be a separate tab from the frozen one");
newPreview.IsPreview.Should().BeTrue(
"the freshly-spawned tab is itself a preview tab");
ReferenceEquals(newPreview.SourceNode, typeB).Should().BeTrue(
"the new preview tab must host the just-selected node's content");
}
}

109
ILSpy/Docking/DockWorkspace.cs

@ -156,17 +156,31 @@ namespace ILSpy.Docking @@ -156,17 +156,31 @@ namespace ILSpy.Docking
ShowSearchCommand = new RelayCommand(ExecuteShowSearch);
using (ILSpy.AppEnv.AppLog.Phase("ILSpyDockFactory ctor + CreateLayout + InitLayout"))
{
factory = new ILSpyDockFactory(toolPaneRegistry);
using (ILSpy.AppEnv.AppLog.Phase("ILSpyDockFactory ctor"))
factory = new ILSpyDockFactory(toolPaneRegistry);
// Prefer the user's saved layout (ILSpy.Layout.json sidecar next to
// ILSpy.xml); fall back to the default layout if there is no saved one
// or it failed to deserialize. The fallback path is the same shape the
// app uses on first launch.
Layout = factory.LoadLayout(GetLayoutFilePath()) ?? factory.CreateLayout();
IRootDock? loaded;
using (ILSpy.AppEnv.AppLog.Phase("ILSpyDockFactory.LoadLayout"))
loaded = factory.LoadLayout(GetLayoutFilePath());
if (loaded != null)
{
Layout = loaded;
ILSpy.AppEnv.AppLog.Mark("ILSpyDockFactory: using LOADED layout");
}
else
{
using (ILSpy.AppEnv.AppLog.Phase("ILSpyDockFactory.CreateLayout (default)"))
Layout = factory.CreateLayout();
}
// Build the layout, then InitLayout BEFORE the chrome ever sees it, rather than
// letting the DockControl's InitializeFactory / InitializeLayout flags run it
// post-template-apply. Wiring owners/factories/locators up front means the
// layout is fully resolvable before the first DeferredContentControl attaches.
factory.InitLayout(Layout);
using (ILSpy.AppEnv.AppLog.Phase("ILSpyDockFactory.InitLayout"))
factory.InitLayout(Layout);
}
assemblyTreeModel.PropertyChanged += OnAssemblyTreePropertyChanged;
@ -615,28 +629,47 @@ namespace ILSpy.Docking @@ -615,28 +629,47 @@ namespace ILSpy.Docking
void ShowSelectedNode()
{
using var _ = ILSpy.AppEnv.AppLog.Phase("DockWorkspace.ShowSelectedNode");
var nodes = assemblyTreeModel.SelectedItems.OfType<ILSpyTreeNode>().ToArray();
if (nodes.Length == 0)
{
lastShownNodes = null;
return;
}
if (factory.MainTab is not { } main)
return;
// SelectedItems.CollectionChanged and SelectedItem PropertyChanged both fan into
// here on a single click, so dedupe to avoid creating two TabPageModels for the
// same selection — the second one's columns would replace the first's, but the
// first's filter wiring would be left dangling on stale ColumnFilter instances.
// Dedupe is also what makes "re-clicking the same node after pin" a no-op: the
// pinned tab stays active, no new preview tab spawns.
if (lastShownNodes is { } prev && prev.SequenceEqual(nodes))
{
// Content didn't change, but the user may have returned to the tree from a
// sibling static tab (Options / About) — pull MainTab forward so re-clicking
// the same node still feels responsive.
ActivateMainTabIfNeeded(main);
if (factory.MainTab is { } existingMain)
ActivateMainTabIfNeeded(existingMain);
return;
}
lastShownNodes = nodes;
// Pick or spawn the target tab. If the currently-active document tab is a
// writable preview ContentTabPage we replace its content in place. Anything
// else — pinned tab, Options page, About page, no Documents dock — counts as
// frozen and gets a brand-new preview tab spawned beside it.
ContentTabPage? main;
if (IsWritablePreview(factory.Documents?.ActiveDockable) is ContentTabPage activePreview)
{
main = activePreview;
factory.MainTab = main;
}
else
{
main = factory.CreateAndAttachPreviewTab();
if (main == null)
return;
// Fresh tab needs a fresh DecompilerTabPageModel — the cached one (if any)
// belongs to a previous tab and must not be re-assigned across tabs.
decompilerContent = null;
}
// Tree-node selections always reuse the single document slot. The Document
// instance never changes; its inner Content swaps between viewmodels. The
// wrapper view (ContentTabPageView) holds pre-realised inner views and toggles
@ -644,7 +677,9 @@ namespace ILSpy.Docking @@ -644,7 +677,9 @@ namespace ILSpy.Docking
// deterministic without going through Dock's add+close lifecycle.
// Carve-outs ("Open in new tab", "Freeze tab") aren't implemented yet and would
// branch off this path before the Content swap.
var customContent = nodes.Length == 1 ? nodes[0].CreateTab() : null;
TabPageModel? customContent;
using (ILSpy.AppEnv.AppLog.Phase("ShowSelectedNode: node.CreateTab"))
customContent = nodes.Length == 1 ? nodes[0].CreateTab() : null;
if (customContent != null)
{
@ -660,12 +695,37 @@ namespace ILSpy.Docking @@ -660,12 +695,37 @@ namespace ILSpy.Docking
return;
}
var decTab = decompilerContent ??= CreateDecompilerContent();
main.Content = decTab;
DecompilerTabPageModel decTab;
using (ILSpy.AppEnv.AppLog.Phase("ShowSelectedNode: get/create decompilerContent"))
decTab = decompilerContent ??= CreateDecompilerContent();
using (ILSpy.AppEnv.AppLog.Phase("ShowSelectedNode: main.Content = decTab (Dock view-recycling)"))
main.Content = decTab;
decTab.Language = languageService.CurrentLanguage;
decTab.CurrentNodes = nodes;
using (ILSpy.AppEnv.AppLog.Phase("ShowSelectedNode: decTab.CurrentNodes = nodes (kicks off DecompileAsync)"))
decTab.CurrentNodes = nodes;
main.SourceNode = nodes.Length == 1 ? nodes[0] : null;
ActivateMainTabIfNeeded(main);
using (ILSpy.AppEnv.AppLog.Phase("ShowSelectedNode: ActivateMainTabIfNeeded"))
ActivateMainTabIfNeeded(main);
}
// Returns the active document tab if it's a writable preview ContentTabPage — meaning
// IsPreview=true AND not hosting static-content (Options, About). Otherwise returns
// null, signalling that ShowSelectedNode must spawn a fresh preview tab instead of
// overwriting the active one. Options pages and About pages are born with
// IsPreview=false (carve-out semantics) AND carry IsStaticContent=true on their
// content viewmodel — either flag alone would suffice, both check guards against
// drift.
static ContentTabPage? IsWritablePreview(IDockable? dockable)
{
if (dockable is not ContentTabPage tab)
return null;
if (!tab.IsPreview)
return null;
if (tab.Content is DecompilerTabPageModel { IsStaticContent: true })
return null;
if (tab.Content is Options.OptionsPageModel)
return null;
return tab;
}
// Brings MainTab to the front of the documents dock so the just-updated content is
@ -990,22 +1050,21 @@ namespace ILSpy.Docking @@ -990,22 +1050,21 @@ namespace ILSpy.Docking
}
/// <summary>
/// VS-style "promote preview tab" gesture. The current <c>factory.MainTab</c> keeps
/// its position and content but flips to pinned (<see cref="ContentTabPage.IsPreview"/>
/// false); a fresh preview <c>MainTab</c> is created and inserted beside it (rightmost),
/// becoming the new target for tree-node selections. The just-pinned tab stays active
/// so the user keeps looking at what they pinned.
/// VS-style "pin tab" gesture. The current <c>factory.MainTab</c> keeps its position,
/// content, and active state but flips to pinned
/// (<see cref="ContentTabPage.IsPreview"/> = false). No new tab spawns at pin time:
/// the next tree-node selection that finds the active tab frozen will lazily spawn
/// a fresh preview tab inside <see cref="ShowSelectedNode"/>. Re-clicking the same
/// node after pin is a no-op (the dedupe activates the pinned tab).
/// </summary>
public void PinCurrentTab()
{
if (factory.PromotePreviewMainTab() is null)
if (factory.PinCurrentMainTab() is null)
return;
// Cached decompiler viewmodel was bound to the previous MainTab — drop it so the
// next tree click materialises a fresh one inside the new MainTab.
// Cached decompiler viewmodel was bound to the just-pinned tab — drop the cache
// so the next selection-change-into-frozen path materialises a fresh
// DecompilerTabPageModel for the newly-spawned preview tab.
decompilerContent = null;
// Defeat the same-selection dedupe so re-clicking the just-pinned node's path
// actually populates the new MainTab.
lastShownNodes = null;
}
public void CloseAllTabs()

34
ILSpy/Docking/ILSpyDockFactory.cs

@ -46,24 +46,36 @@ namespace ILSpy.Docking @@ -46,24 +46,36 @@ namespace ILSpy.Docking
/// just-pinned tab keeps its position and content while MainTab points at a fresh
/// preview tab beside it.
/// </summary>
public ContentTabPage? MainTab { get; private set; }
public ContentTabPage? MainTab { get; internal set; }
/// <summary>
/// VS-style preview-tab promotion. Flips the current <see cref="MainTab"/> to pinned
/// (<see cref="ContentTabPage.IsPreview"/> = false), creates a new placeholder
/// preview tab, appends it to <see cref="Documents"/>.<c>VisibleDockables</c>, and
/// updates <see cref="MainTab"/> to point at the new one. The active dockable is
/// left as-is — the user keeps looking at what they just pinned. Returns the new
/// MainTab, or <see langword="null"/> when there's nothing to promote (no current
/// MainTab, MainTab already pinned, or Documents missing).
/// Flips the current <see cref="MainTab"/> to pinned
/// (<see cref="ContentTabPage.IsPreview"/> = false). Returns the pinned tab, or
/// <see langword="null"/> when there's nothing to pin (no current MainTab, MainTab
/// already pinned). Does NOT spawn a new preview tab — the next selection change
/// that finds the active tab frozen will spawn one lazily via
/// <see cref="CreateAndAttachPreviewTab"/>.
/// </summary>
public ContentTabPage? PromotePreviewMainTab()
public ContentTabPage? PinCurrentMainTab()
{
if (MainTab is not { IsPreview: true } previous)
if (MainTab is not { IsPreview: true } current)
return null;
current.IsPreview = false;
return current;
}
/// <summary>
/// Creates a fresh preview <see cref="ContentTabPage"/>, attaches it to
/// <see cref="Documents"/>, and sets <see cref="MainTab"/> to it. Used when a
/// tree-node selection changes while the active tab is frozen (pinned, Options,
/// About, …) — a brand-new preview slot is needed to host the new content.
/// Returns the new tab, or <see langword="null"/> if <see cref="Documents"/> is
/// missing.
/// </summary>
public ContentTabPage? CreateAndAttachPreviewTab()
{
if (Documents is not { } docs)
return null;
previous.IsPreview = false;
var fresh = new ContentTabPage { Title = "(no selection)" };
// Use the framework's AddDockable so owner/factory wiring matches every other
// add path; default insertion is at the end (rightmost — VS convention for the

Loading…
Cancel
Save