diff --git a/ILSpy.Tests/Docking/PreviewTabPromotionTests.cs b/ILSpy.Tests/Docking/PreviewTabPromotionTests.cs index fc44b014f..5aefe64bd 100644 --- a/ILSpy.Tests/Docking/PreviewTabPromotionTests.cs +++ b/ILSpy.Tests/Docking/PreviewTabPromotionTests.cs @@ -544,19 +544,21 @@ public class PreviewTabPromotionTests } [AvaloniaTest] - public async Task Tree_Selection_While_Frozen_Tab_Active_Opens_A_New_Preview_Tab() + public async Task Tree_Selection_While_Frozen_Tab_Active_Reuses_The_One() { - // "Frozen" covers: (a) user-frozen tabs (IsPreview=false via FreezeCurrentTab), - // (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. + // Exactly one preview tab ("the One"): even when a frozen tab (carve-out / Options / + // About) is the active document, a tree-node selection routes to the EXISTING One -- + // reuse + activate it -- and never spawns a second preview. This is the regression test + // for the stray-preview bug (selecting while a frozen tab was active used to pile up + // previews). var (_, vm) = await TestHarness.BootAsync(3); - var factory = (ILSpyDockFactory)vm.DockWorkspace.Factory; - // Open a carve-out (born frozen) and let it become the active dockable. + // The One exists from boot. + var theOne = factory.MainTab!; + theOne.IsPreview.Should().BeTrue("baseline: the One is a preview tab"); + + // Open a carve-out (born frozen) and make it the active dockable. var typeA = vm.AssemblyTreeModel.FindNode( "System.Linq", "System.Linq", "System.Linq.Enumerable"); vm.DockWorkspace.OpenNodeInNewTab(typeA); @@ -571,25 +573,56 @@ public class PreviewTabPromotionTests var tabCountBefore = vm.DockWorkspace.Documents!.VisibleDockables!.Count; - // Select a different tree node — the carve-out is frozen, so this must NOT - // overwrite it. + // Select a different tree node while the frozen carve-out is active. var typeB = vm.AssemblyTreeModel.FindNode( "System.Private.Uri", "System", "System.Uri"); vm.AssemblyTreeModel.SelectNode(typeB); vm.DockWorkspace.SettleSelection(); - TestCapture.Step("type-b-opens-new-preview"); + TestCapture.Step("type-b-reuses-the-one"); - 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"); + // No new tab -- the existing One was reused, not a second preview spawned. + vm.DockWorkspace.Documents!.VisibleDockables!.Count.Should().Be(tabCountBefore, + "selecting while a frozen tab is active must reuse the One, not spawn a second preview"); + ReferenceEquals(factory.MainTab, theOne).Should().BeTrue( + "the One must be the same instance -- no fresh preview spawned"); + ReferenceEquals(theOne.SourceNode, typeB).Should().BeTrue( + "the One now hosts the just-selected node"); + // The frozen carve-out is untouched, and the One was brought to the front. 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"); + "the frozen carve-out must keep its original content"); + ReferenceEquals(vm.DockWorkspace.Documents.ActiveDockable, theOne).Should().BeTrue( + "reusing the One activates it"); + } + + [AvaloniaTest] + public async Task The_Preview_Is_Always_At_Index_0() + { + // The One lives at documents-dock index 0. After it is frozen, the next selection forges + // a fresh One -- which must again land at index 0 (the frozen ex-One slides to the right). + var (_, vm) = await TestHarness.BootAsync(3); + var factory = (ILSpyDockFactory)vm.DockWorkspace.Factory; + var docs = vm.DockWorkspace.Documents!; + + var typeA = vm.AssemblyTreeModel.FindNode( + "System.Linq", "System.Linq", "System.Linq.Enumerable"); + vm.AssemblyTreeModel.SelectNode(typeA); + vm.DockWorkspace.SettleSelection(); + docs.VisibleDockables!.IndexOf(factory.MainTab!).Should().Be(0, + "the One is the leftmost document tab"); + + // Freeze it, then select a different node -> a fresh One must appear at index 0. + vm.DockWorkspace.FreezeCurrentTab(); + var frozen = factory.MainTab!; + var typeB = vm.AssemblyTreeModel.FindNode( + "System.Private.Uri", "System", "System.Uri"); + vm.AssemblyTreeModel.SelectNode(typeB); + vm.DockWorkspace.SettleSelection(); + + ReferenceEquals(factory.MainTab, frozen).Should().BeFalse("a fresh One was forged"); + factory.MainTab!.IsPreview.Should().BeTrue("the fresh One is a preview tab"); + docs.VisibleDockables!.IndexOf(factory.MainTab!).Should().Be(0, + "the fresh One must be created at index 0, leftmost"); + docs.VisibleDockables!.IndexOf(frozen).Should().BeGreaterThan(0, + "the frozen ex-One slides to the right of the new One"); } } diff --git a/ILSpy/Docking/DockWorkspace.cs b/ILSpy/Docking/DockWorkspace.cs index b85017405..df59132d0 100644 --- a/ILSpy/Docking/DockWorkspace.cs +++ b/ILSpy/Docking/DockWorkspace.cs @@ -706,25 +706,10 @@ namespace ILSpy.Docking } 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 — frozen 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; - } + // Route to THE single preview tab ("the One") and replace its content in place. + var main = GetOrCreateThePreview(); + if (main == null) + return; // Tree-node selections always reuse the single document slot. The Document // instance never changes; its inner Content swaps between viewmodels. The @@ -764,13 +749,32 @@ namespace ILSpy.Docking 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. + // The "exactly one preview tab" rule: tree selections always route to the single preview + // ("the One"), tracked by factory.MainTab. Reuse it wherever it sits -- even if a frozen + // tab is currently active, ActivateMainTabIfNeeded brings it forward -- as long as it is + // still a writable preview AND still lives in the dock. Otherwise the One was frozen-away + // or closed, so forge a fresh one at index 0. This is what stops a selection-while-frozen + // from spawning a SECOND preview. + ContentTabPage? GetOrCreateThePreview() + { + if (factory.Documents is not { } docs) + return null; + if (factory.MainTab is { } current + && IsWritablePreview(current) is not null + && docs.VisibleDockables?.Contains(current) == true) + { + return current; + } + // A fresh One needs a fresh DecompilerTabPageModel -- the cached one (if any) belongs + // to a previous tab and must not be re-assigned across tabs. + decompilerContent = null; + return factory.CreateThePreviewTab(); + } + + // Pure predicate: returns the tab if it is a writable preview ContentTabPage -- IsPreview + // true AND not hosting static content (Options, About). Otherwise null. Options/About are + // born frozen (IsPreview=false) AND carry IsStaticContent=true; either flag alone would + // suffice, both guard against drift. static ContentTabPage? IsWritablePreview(IDockable? dockable) { if (dockable is not ContentTabPage tab) diff --git a/ILSpy/Docking/ILSpyDockFactory.cs b/ILSpy/Docking/ILSpyDockFactory.cs index 3687f3d47..fe6e16f9d 100644 --- a/ILSpy/Docking/ILSpyDockFactory.cs +++ b/ILSpy/Docking/ILSpyDockFactory.cs @@ -53,8 +53,8 @@ namespace ILSpy.Docking /// ( = false). Returns the frozen tab, or /// when there's nothing to freeze (no current MainTab, MainTab /// already frozen). Does NOT spawn a new preview tab — the next selection change - /// that finds the active tab frozen will spawn one lazily via - /// . + /// finds no preview to reuse and forges a fresh One lazily via + /// . /// public ContentTabPage? FreezeCurrentMainTab() { @@ -65,22 +65,20 @@ namespace ILSpy.Docking } /// - /// Creates a fresh preview , attaches it to - /// , and sets to it. Used when a - /// tree-node selection changes while the active tab is frozen (frozen, Options, - /// About, …) — a brand-new preview slot is needed to host the new content. - /// Returns the new tab, or if is - /// missing. + /// Creates THE single preview ("the One"), attaches it to + /// at index 0, and sets to it. Called when a + /// tree-node selection has no preview to reuse (the previous One was frozen, closed, or + /// none exists yet). Returns the new tab, or if + /// is missing. /// - public ContentTabPage? CreateAndAttachPreviewTab() + public ContentTabPage? CreateThePreviewTab() { if (Documents is not { } docs) return null; 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 - // preview slot). - AddDockable(docs, fresh); + // The One always occupies index 0 (leftmost); frozen tabs live to its right. Use + // InsertDockable (not AddDockable, which appends) so it lands at the front. + InsertDockable(docs, fresh, 0); MainTab = fresh; return fresh; }