diff --git a/.github/skills/sessions/SKILL.md b/.github/skills/sessions/SKILL.md index 192799c372f..d8f93d7488d 100644 --- a/.github/skills/sessions/SKILL.md +++ b/.github/skills/sessions/SKILL.md @@ -87,7 +87,7 @@ Whenever the user flags a wrong pattern, rejects an approach, or gives design/ru - **Overriding a workbench toolbar hover needs matching specificity**: the core rule `.monaco-workbench .monaco-action-bar:not(.vertical) .action-label:not(.disabled):hover` (and the `:hover` outline rule) sets the toolbar hover background/outline at ~6-7 class specificity. An action-item label that needs to override that hover (either to suppress it for a non-interactive label, or to re-skin it) must use an equal-or-higher-specificity selector (prefix `.monaco-workbench ... .monaco-action-bar:not(.vertical) .action-item. .action-label:hover`), not a short `. .action-label:hover` that loses the cascade. (The single-pane diff-stats pill was once suppressed this way while static; it is now a clickable action that opens the multi-file diff, so it keeps the standard `--vscode-toolbar-hoverBackground` hover.) -- **A managed tab must never reveal the docked editor content — but the core workbench must not hardcode which, and the excluded editor's deliberate open must reveal explicitly**: the empty Files placeholder (`EmptyFileEditorInput`) and the Changes multi-diff (`SessionChangesEditorInput`) surface their content in the detail panel, so activating either (e.g. the placeholder activating as a side effect of closing the Changes tab, or clicking the Changes tab) must not reveal the editor area. The core workbench `_handleWillOpenEditor` reveal handler skips revealing for editors matched by a **contrib-provided predicate** (`IAgentWorkbenchLayoutService.setEditorRevealOnOpenExclusion`), which the single-pane layout controller sets to its `_isManagedEditor` check — so the editor-type policy lives in contrib (which can `instanceof` the real inputs), not as hardcoded type-id literals in `src/vs/sessions/browser/*` (core). **Caveat:** because tab activation and opening a file diff both fire `onWillOpenEditor` for the *same* Changes editor (and the event carries no options to tell them apart), excluding it from the auto-reveal also blocks the deliberate file-open reveal — so the Changes view's `_openMultiFileDiffEditor` (clicking a file) must **explicitly** `setPartHidden(false, EDITOR_PART)` before opening (revealing before the open also avoids the multi-diff hanging while laid out in a hidden 0-size editor part). Do NOT use a broad "editor already in group -> skip" rule. Keep `_handleWillOpenEditor` a named method so it is unit-testable via `Reflect.get(Workbench.prototype, ...)`. +- **A docked-detail editor must not reveal the editor area while the detail panel is already showing its content — model it as a base editor input the single-pane workbench recognises**: the empty Files placeholder (`EmptyFileEditorInput`) and the Changes multi-diff (`SessionChangesEditorInput`) surface their content in the docked detail panel (auxiliary bar). Activating one (closing a neighbouring tab so the workbench auto-opens the next editor via `editorGroupView.doCloseActiveEditor` → `doOpenEditor`, or clicking the tab) fires `onWillOpenEditor` *unsuppressed* and would otherwise reveal the hidden editor area. Both inputs extend the abstract base `DockedEditorInput` (`src/vs/sessions/common/dockedEditorInput.ts`, extends `EditorInput`). The base `Workbench.revealEditorOnOpen(e)` (the `onWillOpenEditor` handler — a **protected** method, renamed from `_handleWillOpenEditor`) does the generic reveal; `SinglePaneWorkbench` **overrides `revealEditorOnOpen`** and returns early (no reveal) when `e.editor instanceof DockedEditorInput && partVisibility.auxiliaryBar && !partVisibility.editor` — i.e. only when the detail panel is open and the editor area is closed — otherwise it calls `super.revealEditorOnOpen(e)`. So the docked-editor policy lives in `SinglePaneWorkbench` (the only workbench with a docked detail panel) via a proper type + the current part visibility, not a per-input marker, a contrib-registered predicate, or a remembered set. Note the condition means that when the detail panel is **closed** (whole side pane closed), opening a docked editor **does** reveal the editor area so its content is visible. **Caveat:** a deliberate open of the already-open Changes tab (session-header pill `ViewAllChangesAction`) or a file diff (`_openMultiFileDiffEditor`) while the detail panel is open is still suppressed by this rule, so those must **explicitly** reveal via `revealEditorPartExplicitly()` before opening (revealing before the open also avoids the multi-diff hanging while laid out in a hidden 0-size editor part). Keep `revealEditorOnOpen` a named protected method so it is unit-testable via `Reflect.get(...prototype, ...)`. - **Gate single-pane editor-title *layout/view* actions on `MainEditorAreaVisibleContext`; the Create Pull Request bar lives in the title bar, not the editor**: single-pane (`config.`-gated) editor-title *layout/view* items (Maximize/Restore, Toggle Details, Hide Editor, Open in Modal, the diff-view actions collapse/expand/toggle-inline/list-tree) must include `MainEditorAreaVisibleContext` so they disappear when the editor content is closed. The **Create Pull Request** anchor (`CHANGES_HEADER_ACTIONS_ID`) is *not* an editor-title action: it is contributed to `Menus.TitleBarSessionMenu` (the sessions title bar's session-actions area) by `ChangesHeaderActionsAction` in `changesViewActions.ts`, gated on `IsSessionsWindowContext` + `IsAuxiliaryWindowContext.toNegated()` + `config.` + `SessionHasChangesContext` (independent of editor-area visibility), and its `ChangesActionsBar` view item is registered for `(Menus.TitleBarSessionMenu, CHANGES_HEADER_ACTIONS_ID)` via `IActionViewItemService`. The docked reveal-sync (`_syncDockedEditorVisibility`) must be *symmetric*: it reveals when the node widens past the detail width and **hides** (sets `partVisibility.editor=false`, flips `MainEditorAreaVisibleContext`) when a sash drag squeezes the editor content back down to the detail width — same guards (`_syncingDockedEditorVisibility`, `_suppressDockedEditorRevealSync`, `_dockDetailPanel`, and only while the detail is visible). @@ -129,7 +129,7 @@ You **must** run these checks before declaring work complete: - **Single-pane detail (aux bar) ownership is split cleanly in two — visibility vs content — never three overlapping aux strategies**: in single-pane the auxiliary bar *is* the detail panel, so exactly two strategies touch it, with non-overlapping responsibilities. `SinglePaneDetailVisibilityStrategy` owns **only** per-session *shown/hidden* memory: it captures the user's choice ([D1]/[D2]), restores it on switch ([D3]) by revealing/hiding the aux **part** (`setPartHidden` / `hideAuxiliaryBarForRestore`), and handles the submit transition ([D4]). `SinglePaneDetailPanelStrategy` owns **everything about content**: which container (Changes/Files, mapped from the active editor), the transient browser-tab hide, editor-maximize → Changes, and the "nothing to show" hide (quick chat / no workspace / empty group → `Hidden`). Do NOT reintroduce a separate `EmptyAuxCleanup`/D10 strategy or desktop's saved-container machinery (`auxiliaryBarActiveViewContainerId` restore, `_openDefaultAuxiliaryBarContainer`, `_restoreSavedAuxiliaryBarContainerOnReveal`, pinned-container checks) into the visibility strategy — the container always follows the active editor, so a stored container preference is redundant and races the detail-panel mapping. Because the visibility strategy reveals the part and the detail-panel strategy fills it, the detail-panel strategy registers **immediately** in `_registerViewStateManagement` (not deferred to `Restored` like the managed tabs), so a reveal and its container open happen in the same turn. -- **Single-pane is a *sibling* of the desktop controller and composes strategy objects — it does not extend `LayoutController`**: `SinglePaneLayoutController` (file `contrib/layout/browser/singlePaneLayoutController.ts`) extends `BaseLayoutController` directly, NOT the classic desktop `LayoutController`, so the desktop controller can be deprecated/deleted without touching single-pane. Its behaviour is composed from strategy objects under `contrib/layout/browser/singlePane/` (each a `Disposable`, created via `createInstance` with a leading `ISinglePaneLayoutContext` arg): `SinglePaneDetailVisibilityStrategy` (per-session detail shown/hidden: D1/D2/D3/D4) and `SinglePaneDetailPanelStrategy` (container + maximize + browser-hide + nothing-to-show hide) — the detail split above; `SinglePaneManagedTabsStrategy` + `SinglePaneEditorAreaCollapseStrategy` (share a `SinglePaneDockedTabsCoordinator` holding the tab `Sequencer`, `internallyClosingEditors`, `collapsedEditors`, and the `isManagedEditor`/`getChangesEditorResource` helpers); `SinglePaneQuickChatEditorHideStrategy`; `SinglePaneResponsiveSidebarStrategy` (owns the Toggle Details action + sidebar auto-hide); `SinglePaneNewSessionRulesStrategy` (R1). Shared controller state (`isRestoringSessionLayout`, `withSessionLayoutRestore`, `togglingSidePane`, the obs, `viewStateBySession`, `hidingAuxiliaryBarForRestore`/`hideAuxiliaryBarForRestore`) is exposed to strategies through `ISinglePaneLayoutContext` (built lazily in the controller because base's constructor calls the `_registerViewStateManagement`/`_registerAuxiliaryControllers` hooks *before* subclass field initializers run). The detail-visibility/detail-panel/responsive/R1 strategies register in `_registerViewStateManagement`; the managed-tab/collapse/quick-chat strategies register in `_registerAuxiliaryControllers` deferred to `LifecyclePhase.Restored`. **Fresh storage**: single-pane persists to `sessions.singlePane.layoutState` + `sessions.singlePane.newSessionViewState` (base `_layoutStateStorageKey`/`_legacyWorkingSetsStorageKey` are overridable; single-pane skips legacy migration), so it never shares state with the classic desktop controller — the test harness seeds both keys. +- **Single-pane is a *sibling* of the desktop controller and composes strategy objects — it does not extend `LayoutController`**: `SinglePaneLayoutController` (file `contrib/layout/browser/singlePaneLayoutController.ts`) extends `BaseLayoutController` directly, NOT the classic desktop `LayoutController`, so the desktop controller can be deprecated/deleted without touching single-pane. Its behaviour is composed from strategy objects under `contrib/layout/browser/singlePane/` (each a `Disposable`, created via `createInstance` with a leading `ISinglePaneLayoutContext` arg): `SinglePaneDetailVisibilityStrategy` (per-session detail shown/hidden: D1/D2/D3/D4) and `SinglePaneDetailPanelStrategy` (container + maximize + browser-hide + nothing-to-show hide) — the detail split above; `SinglePaneManagedTabsStrategy` + `SinglePaneEditorAreaCollapseStrategy` (share a `SinglePaneDockedTabsCoordinator` holding the tab `Sequencer`, `internallyClosingEditors`, `collapsedEditors`, and the `getChangesEditorResource` helper; docked (managed) tabs are identified by `instanceof DockedEditorInput`); `SinglePaneQuickChatEditorHideStrategy`; `SinglePaneResponsiveSidebarStrategy` (owns the Toggle Details action + sidebar auto-hide); `SinglePaneNewSessionRulesStrategy` (R1). Shared controller state (`isRestoringSessionLayout`, `withSessionLayoutRestore`, `togglingSidePane`, the obs, `viewStateBySession`, `hidingAuxiliaryBarForRestore`/`hideAuxiliaryBarForRestore`) is exposed to strategies through `ISinglePaneLayoutContext` (built lazily in the controller because base's constructor calls the `_registerViewStateManagement`/`_registerAuxiliaryControllers` hooks *before* subclass field initializers run). The detail-visibility/detail-panel/responsive/R1 strategies register in `_registerViewStateManagement`; the managed-tab/collapse/quick-chat strategies register in `_registerAuxiliaryControllers` deferred to `LifecyclePhase.Restored`. **Fresh storage**: single-pane persists to `sessions.singlePane.layoutState` + `sessions.singlePane.newSessionViewState` (base `_layoutStateStorageKey`/`_legacyWorkingSetsStorageKey` are overridable; single-pane skips legacy migration), so it never shares state with the classic desktop controller — the test harness seeds both keys. - **Single-pane detail/tab behaviour lives ON the layout controller (or its strategies), not in separate contribution controllers or a shared service**: `SinglePaneLayoutController` owns both the managed docked tabs (pinned Changes multi-diff + empty Files placeholder) and the detail-panel mapping (active editor → Changes/Files container, aux-bar reveal/hide). They were previously `ChangesTabController`/`DetailPanelController` (registered by a `SinglePaneModeController` contribution) coordinating via global `IAgentWorkbenchLayoutService` flags, then briefly via an `ISessionLayoutCoordinatorService`. Both were removed: "is a session-switch restore in progress?" is just the base protected getter `this._isRestoringSessionLayout` (set by `_withSessionLayoutRestore`) — surfaced to the strategies via `ISinglePaneLayoutContext.isRestoringSessionLayout` — so a restore-driven editor change never force-reveals the detail or dismisses a managed tab. The base controller has `IChangesViewService` + `IContextKeyService` deps and a protected `_editorGroupsService` (a subclass can't add DI ctor params without redeclaring all base params, so shared services live on the base). Tests: the layout harness got `activeGroupEditors`/`closeSuppressionFlags`, a real `mainPart.activeGroup`, an `activateAux` opt-in that resolves the lifecycle, and a `TestSinglePaneController.runWithRestore(...)` seam to hold `_isRestoringSessionLayout` across an async editor change; `changesTabController.test.ts` was deleted and its scenarios moved into `desktopSessionLayoutController.test.ts`. @@ -155,7 +155,7 @@ You **must** run these checks before declaring work complete: - **An auto-collapsed sessions list must be restored once the side pane is fully hidden**: the single-pane responsive rule auto-collapses the sessions list to free width for a *visible* side pane (Toggle Details, opening a file). It must also restore an auto-hidden list when the side pane becomes fully hidden (both editor and aux bar closed) — e.g. switching to a quick chat (no side pane) — otherwise the list is left collapsed with nothing to make room for (bug: "sessions list closed even though the side pane is hidden"). Implement as an autorun in `_registerResponsiveSidebar` on an `observableFromEvent(onDidChangePartVisibility, () => editorVisible || auxVisible)` (the value-dedup is essential: hiding the *sidebar* itself doesn't change the computed side-pane visibility, so the pre-reveal auto-hide from opening an editor is never undone). Restore only when `_sidebarAutoHidden` is true, so a list the user closed **manually** stays closed. - **Single-pane per-session editor-part visibility must be restored *both* ways — `_applyWorkingSet` only ever revealed it**: `baseSessionLayoutController._applyWorkingSet` revealed the editor part when a session wanted it visible but never *hid* it, so returning to a session whose docked editor was closed (Detail-only or whole side pane closed) left the editor visible/inherited from the previously-active session (bug: "side pane opened when returning to a session where it was closed"). The per-session `_editorPartHiddenBySession` state was only consumed to *suppress* the reveal (`!editorPartHidden`), never to actively hide. Fix: add a symmetric Template-Method hook `_shouldHideEditorPartOnApply(editorPartHidden)` (base returns `false` — classic layout doesn't treat editor-part visibility as per-session; single-pane returns `editorPartHidden && isCreated && !isQuickChat`) and, in both the empty and non-empty `_applyWorkingSet` branches, hide the editor part (mutually exclusive with revealing, skipped on `isInitialRestore` which preserves the workbench-restored visibility). The hide runs inside `_withSessionLayoutRestore`'s `suppressEditorPartAutoVisibility` window so it is never mistaken for a user close. Note the aux bar was already restored both ways by the inherited D3 `_syncAuxiliaryBarVisibility`; only the editor part lacked the hide. -- **Explicit managed-editor opens must reveal outside the auto-reveal path — and mark the reveal *explicit***: managed Changes/Files tabs are excluded from `_handleWillOpenEditor` so tab activation and layout-driven restores do not reveal the docked editor. A deliberate user gesture that should show managed editor content (session-header Changes pill `ViewAllChangesAction`, opening a file diff in `_openMultiFileDiffEditor`) must reveal the editor part before opening the managed editor via `IAgentWorkbenchLayoutService.revealEditorPartExplicitly()` — **not** the generic `setPartHidden(false, EDITOR_PART)`. The generic call routes to `setEditorHidden(hidden, explicit=false)`, leaving `_editorRevealedExplicitly = false`, so R1 / the working-set apply (`_shouldHideEditorPartOnApply`) can re-hide it (especially across a session-switch race). `revealEditorPartExplicitly()` sets the explicit flag (and re-asserts it even when already visible, since `setEditorHidden` early-returns when the part is already visible). Do not weaken the managed-editor exclusion or add timing delays. +- **Explicit managed-editor opens must reveal outside the auto-reveal path — and mark the reveal *explicit***: docked-detail Changes/Files editors (`DockedEditorInput`) are kept from revealing the docked editor by `SinglePaneWorkbench.revealEditorOnOpen` (see the entry above), so tab activation and layout-driven restores do not reveal it. A deliberate user gesture that should show managed editor content (session-header Changes pill `ViewAllChangesAction`, opening a file diff in `_openMultiFileDiffEditor`) must reveal the editor part before opening the managed editor via `IAgentWorkbenchLayoutService.revealEditorPartExplicitly()` — **not** the generic `setPartHidden(false, EDITOR_PART)`. The generic call routes to `setEditorHidden(hidden, explicit=false)`, leaving `_editorRevealedExplicitly = false`, so R1 / the working-set apply (`_shouldHideEditorPartOnApply`) can re-hide it (especially across a session-switch race). `revealEditorPartExplicitly()` sets the explicit flag (and re-asserts it even when already visible, since `setEditorHidden` early-returns when the part is already visible). Do not weaken the `DockedEditorInput` reveal suppression or add timing delays. - **A `MutableDisposable`-backed content slot must not `clearNode` its shared container on cleanup**: `EditorGroupView.setHeaderContent` appends a new content node into the shared `headerContainer`, then assigns the new store to `_headerContent` (a `MutableDisposable`) — which **synchronously disposes the previous store**. If that store's cleanup calls `clearNode(headerContainer)`, it wipes the freshly-appended new content (blank header, height stuck at 0, orphaned `ResizeObserver`). Fix: clear the previous content **before** appending the new one (`this._headerContent.clear()` at the top), and have each store's cleanup remove only **its own** node (`content.remove()`), never the shared container. This bug surfaces on consecutive header→header renders (e.g. `Changes(sessionA)` → `Changes(sessionB)`). - **Per-session editor-part (side-pane) hidden state must be captured *eagerly* on the visibility change, not lazily re-read at switch-away**: `baseSessionLayoutController._saveWorkingSet` used to record `_editorPartHiddenBySession[prev] = !isVisible(EDITOR_PART)` at the moment it saved the outgoing session. That races: the working-set derive (`activeSessionForWorkingSet`) lags the raw `activeSession` (it gates on workspace-folder readiness), so other autoruns driven by the raw active session (managed-tab open, D3 aux sync) have already revealed the editor for the *incoming* session by the time `_saveWorkingSet(prev)` runs — so the previous session gets recorded as `editorPartHidden=false` and its closed side pane reopens on return (symptom: only the editor content re-appears, details stay closed, and no `setEditorHidden` fires on the switch because nothing on the switch path toggles it). Fix: capture it in a `[B2]` `onDidChangePartVisibility(EDITOR_PART)` listener (mirroring the existing `[B1]` panel-visibility capture) guarded by `!multipleSessionsVisibleObs && !_isRestoringSessionLayout`, so the value is written the instant the user closes/opens the side pane and layout-driven restore changes are ignored. Remove the lazy read from `_saveWorkingSet` entirely (keeping it would let the racy switch-time value overwrite the good eager one). The unit harness can't reproduce the derive-lag, so add a focused test that fires the EDITOR_PART event to assert eager capture, plus one that fires a reveal inside `_withSessionLayoutRestore` to assert the captured closed state is preserved. diff --git a/src/vs/sessions/LAYOUT.md b/src/vs/sessions/LAYOUT.md index d1830de8606..dd62223853f 100644 --- a/src/vs/sessions/LAYOUT.md +++ b/src/vs/sessions/LAYOUT.md @@ -229,7 +229,7 @@ The entire third-pane redesign is gated behind the experimental setting `session - The shared editor title's inline layout cluster orders maximize/restore before the Hide Editor chevron, followed by the detail-panel toggle. The detail-panel toggle is conditional (shown only when the active tab is Changes or Files, i.e. hidden for Browser/Search tabs, which have no detail). No chevron is shown while the editor is hidden; opening a file or diff from the detail panel reveals the editor again. If the detail-panel toggle hides the detail while editor content is hidden, it reveals the editor content instead of leaving the pane empty; **Toggle Side Panel** remains the separate action that can hide both. - Changes opens as a **custom `SessionChangesEditor`** (the multi-diff editor; in single-pane its *Branch Changes* dropdown + diff-stats + primary actions render in the full-width header part above, so the editor itself is header-less and the diff fills the pane), and clicking a Branch Changes file always reveals that file in this multi-diff editor (ignoring `sessions.changes.openSingleFileDiff` and the Alt inversion used by the standard layout). The auxiliary bar's composite tab strip + title are hidden, and `DetailPanelController` maps the active editor tab to the detail container (Changes → files + Checks, File → Explorer, Browser → hidden). Activating a File or Changes editor reveals the matching detail panel once; after that, an explicit detail-panel hide is respected until the active editor changes. Browser-driven hides are transient: switching back to File or Changes re-opens the detail panel. - Closing the last editor tab hides both the editor content and the docked detail panel, leaving the Agents window chat-only. Opening any tab reveals the editor part again, and `DetailPanelController` restores the matching detail content for File/Changes tabs. -- **Editor-area tab collapse:** when the editor area is hidden (detail-only), the single-pane controller closes the non-managed (real file) editor tabs so only the managed Changes and Files tabs remain, capturing each one's untyped input **and tab index** (`editor.toUntyped()`); when the editor area is shown again they are reopened **at their original positions** (`SinglePaneLayoutController._registerEditorAreaTabCollapse` / `_collapseNonManagedTabs` / `_restoreCollapsedTabs`). It is serialized on the managed-tab `Sequencer`, skipped during a layout-driven restore (`_isRestoringSessionLayout`), skips dirty editors, and the capture is dropped on a session change. +- **Editor-area tab collapse:** when the editor area is hidden (detail-only), the single-pane controller closes **every non-docked** editor tab (anything not `instanceof DockedEditorInput`) so only the docked Changes and Files tabs remain, capturing each closable one's untyped input **and tab index** (`editor.toUntyped()`); when the editor area is shown again the captured ones are reopened **at their original positions** (`SinglePaneEditorAreaCollapseStrategy._collapseNonManagedTabs` / `_restoreCollapsedTabs`, registered by `SinglePaneLayoutController._registerAuxiliaryControllers`). It is serialized on the shared docked-tab `Sequencer`, skipped during a layout-driven restore (`ISinglePaneLayoutContext.isRestoringSessionLayout`), and the capture is dropped on a session change. Non-restorable tabs (e.g. an **untitled Search editor**, whose `toUntyped()` returns `undefined`) are still closed but not restored; dirty editors are closed too (the workbench save/confirm flow applies), so they don't linger in a "closed" editor area. - While a new (uncreated) workspace session view is active, the editor content is kept hidden **continuously** so the Files detail panel and editor tab bar remain visible without showing editor content by default. The rule is **level-triggered** on the active editor + editor-part visibility (in `singlePaneLayoutController.ts`): it hides the editor whenever the active editor is **not real content** (a `FileEditorInput` for a real file or a `BrowserEditorInput`), treating the managed empty landing tab (`EmptyFileEditorInput`) and "no active editor" as not real content. Because it re-reads visibility + active editor, any spurious reveal (a session-switch working-set restore, a layout race, the 60%-of-window split) is **re-hidden** — so reopening a new session after visiting a created session keeps the editor closed. While there is no real content the width-based reveal-sync is also suppressed (`setSuppressDockedEditorRevealSync(true)`), so sidebar-collapse, grid relayout, and grid-sash drags never re-reveal the editor there. Once a real file/diff is the active editor the hide **short-circuits** (and the suppress flag clears), so a real open (via `onWillOpenEditor` → `setEditorHidden(false)`) or the detail-panel toggle reveals it and sticks. On submit, a Changes tab is added and the Changes detail is shown, but the editor content stays closed. The auto-managed Changes and File tabs never reveal the editor content. - CSS is scoped by a `.dock-detail-panel` class on the workbench container; `:not(.dock-detail-panel)` reproduces the original grid-based styling. - The docked auxiliary bar draws its own left and top borders with `--vscode-agentsPanel-border` so the detail panel reads as a bordered region connected to the middle divider. diff --git a/src/vs/sessions/browser/singlePaneWorkbench.ts b/src/vs/sessions/browser/singlePaneWorkbench.ts index efae7e2ff1d..d7c1e815059 100644 --- a/src/vs/sessions/browser/singlePaneWorkbench.ts +++ b/src/vs/sessions/browser/singlePaneWorkbench.ts @@ -4,7 +4,9 @@ *--------------------------------------------------------------------------------------------*/ import { ISerializableView, ISerializedNode, IViewSize } from '../../base/browser/ui/grid/grid.js'; +import { IEditorWillOpenEvent } from '../../workbench/common/editor.js'; import { Parts } from '../../workbench/services/layout/browser/layoutService.js'; +import { DockedEditorInput } from '../common/dockedEditorInput.js'; import { DockedAuxiliaryBarController } from './dockedAuxiliaryBarController.js'; import { SinglePaneMainEditorPart } from './parts/singlePaneEditorPart.js'; import { EDITOR_PART_MINIMUM_WIDTH, SIDE_PANE_WIDTH_RATIO } from './parts/editorPartSizing.js'; @@ -58,6 +60,20 @@ export class SinglePaneWorkbench extends Workbench { return true; } + /** + * A docked-detail editor (Changes/Files) renders its content in the docked + * detail panel. While that panel is open and the editor area is closed, + * re-activating such an editor (closing a neighbouring tab, or clicking the + * tab) must not reveal the editor area. When the detail panel is closed the + * base reveal still runs so the content becomes visible. + */ + protected override revealEditorOnOpen(e: IEditorWillOpenEvent): void { + if (e.editor instanceof DockedEditorInput && this.partVisibility.auxiliaryBar && !this.partVisibility.editor) { + return; + } + super.revealEditorOnOpen(e); + } + override getDockedAuxiliaryBarWidth(): number { return this._dockedAuxiliaryBarWidth; } diff --git a/src/vs/sessions/browser/workbench.ts b/src/vs/sessions/browser/workbench.ts index 866dd317672..236dd91bb40 100644 --- a/src/vs/sessions/browser/workbench.ts +++ b/src/vs/sessions/browser/workbench.ts @@ -42,7 +42,6 @@ import { setBaseLayerHoverDelegate } from '../../base/browser/ui/hover/hoverDele import { Registry } from '../../platform/registry/common/platform.js'; import { IWorkbenchContributionsRegistry, Extensions as WorkbenchExtensions } from '../../workbench/common/contributions.js'; import { IEditorFactoryRegistry, EditorExtensions, IEditorWillOpenEvent } from '../../workbench/common/editor.js'; -import { EditorInput } from '../../workbench/common/editor/editorInput.js'; import { setARIAContainer } from '../../base/browser/ui/aria/aria.js'; import { FontMeasurements } from '../../editor/browser/config/fontMeasurements.js'; import { createBareFontInfoFromRawSettings } from '../../editor/common/config/fontInfoFromSettings.js'; @@ -186,15 +185,6 @@ export interface IDockedEditorLayout { */ getDockedAuxiliaryBarWidth(): number; setDockedAuxiliaryBarWidth(width: number): void; - - /** - * Sets a predicate deciding which editors, when opened while the editor area is - * hidden, should NOT reveal it (their content lives in the detail panel, e.g. the - * managed Changes and Files tabs). Lets contrib own the policy — including the - * editor types involved — instead of the core workbench hardcoding type ids. - * Returns a disposable that clears the predicate. - */ - setEditorRevealOnOpenExclusion(predicate: (editor: EditorInput) => boolean): IDisposable; } export const IAgentWorkbenchLayoutService = refineServiceDecorator(IWorkbenchLayoutService); @@ -370,7 +360,6 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic private _restoreAttachedEditorMaximizedOnShow = false; protected _editorPartAutoVisibilitySuppressionCount = 0; protected _hasAppliedInitialEditorSplit = false; - private _editorRevealOnOpenExclusion: ((editor: EditorInput) => boolean) | undefined; private readonly restoredPromise = new DeferredPromise(); readonly whenRestored = this.restoredPromise.p; @@ -1121,10 +1110,11 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic // they actually target one of the main editor groups. Modal // opens stay neutral. Programmatic opens that suppress auto // visibility (e.g. working set application) are ignored. - // The managed empty Files tab is a placeholder that activates as a side - // effect of closing another tab (e.g. the Changes tab); it must never - // reveal a hidden editor. Real content (files, diffs, browser) still does. - this._register(this.editorService.onWillOpenEditor(e => this._handleWillOpenEditor(e))); + // The base handler reveals a hidden editor for any such open; + // `SinglePaneWorkbench` overrides `revealEditorOnOpen` to keep a + // docked-detail editor (Changes/Files) from revealing the editor area + // while the detail panel is already showing its content. + this._register(this.editorService.onWillOpenEditor(e => this.revealEditorOnOpen(e))); // Hide editor part when last editor closes this._register(this.editorService.onDidCloseEditor(() => this.handleDidCloseEditor())); @@ -1157,7 +1147,7 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic return true; } - private _handleWillOpenEditor(e: IEditorWillOpenEvent): void { + protected revealEditorOnOpen(e: IEditorWillOpenEvent): void { if (this._editorPartAutoVisibilitySuppressionCount > 0) { return; } @@ -1167,13 +1157,6 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic return; } - // A contrib-provided policy decides which editors surface their content in - // the detail panel (e.g. the managed Changes and Files tabs) and so must not - // reveal the hidden editor area when opened. - if (this._editorRevealOnOpenExclusion?.(e.editor)) { - return; - } - if (!this.partVisibility.editor) { this.setEditorHidden(false, /* explicit */ true); this.restoreAttachedEditorMaximizedState(); @@ -1200,15 +1183,6 @@ export class Workbench extends Disposable implements IAgentWorkbenchLayoutServic }); } - setEditorRevealOnOpenExclusion(predicate: (editor: EditorInput) => boolean): IDisposable { - this._editorRevealOnOpenExclusion = predicate; - return toDisposable(() => { - if (this._editorRevealOnOpenExclusion === predicate) { - this._editorRevealOnOpenExclusion = undefined; - } - }); - } - protected rememberAttachedEditorMaximizedState(): void { this._restoreAttachedEditorMaximizedOnShow = this._editorMaximized && this.partVisibility.auxiliaryBar; } diff --git a/src/vs/sessions/common/dockedEditorInput.ts b/src/vs/sessions/common/dockedEditorInput.ts new file mode 100644 index 00000000000..36c58e07362 --- /dev/null +++ b/src/vs/sessions/common/dockedEditorInput.ts @@ -0,0 +1,14 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { EditorInput } from '../../workbench/common/editor/editorInput.js'; + +/** + * Base class for Agents window editors whose content is surfaced in the docked + * detail panel (the managed Changes and Files tabs) rather than the main editor + * area. In the single-pane (docked details) layout, re-activating such an editor + * must not reveal the hidden editor area — {@link SinglePaneWorkbench} handles that. + */ +export abstract class DockedEditorInput extends EditorInput { } diff --git a/src/vs/sessions/contrib/changes/browser/sessionChangesEditorInput.ts b/src/vs/sessions/contrib/changes/browser/sessionChangesEditorInput.ts index ba3efbc66e5..7e019ae186b 100644 --- a/src/vs/sessions/contrib/changes/browser/sessionChangesEditorInput.ts +++ b/src/vs/sessions/contrib/changes/browser/sessionChangesEditorInput.ts @@ -12,13 +12,14 @@ import { EditorInput } from '../../../../workbench/common/editor/editorInput.js' import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; import { MultiDiffEditorInput } from '../../../../workbench/contrib/multiDiffEditor/browser/multiDiffEditorInput.js'; import { MultiDiffEditorViewModel } from '../../../../editor/browser/widget/multiDiffEditor/multiDiffEditorViewModel.js'; +import { DockedEditorInput } from '../../../common/dockedEditorInput.js'; /** * Editor input for the Agents window Changes tab. It wraps the session's * multi-diff source and exposes the resolved multi-diff view model so the * {@link SessionChangesEditor} can render the diffs beneath its own header. */ -export class SessionChangesEditorInput extends EditorInput { +export class SessionChangesEditorInput extends DockedEditorInput { static readonly ID = 'workbench.input.agentSessions.sessionChanges'; static readonly EDITOR_ID = 'workbench.editor.agentSessions.sessionChanges'; diff --git a/src/vs/sessions/contrib/editor/browser/emptyFileEditorInput.ts b/src/vs/sessions/contrib/editor/browser/emptyFileEditorInput.ts index 15f14a5fdbd..9ac04775bb7 100644 --- a/src/vs/sessions/contrib/editor/browser/emptyFileEditorInput.ts +++ b/src/vs/sessions/contrib/editor/browser/emptyFileEditorInput.ts @@ -10,8 +10,9 @@ import { URI } from '../../../../base/common/uri.js'; import { EditorInputCapabilities, IEditorSerializer, IUntypedEditorInput, Verbosity } from '../../../../workbench/common/editor.js'; import { EditorInput } from '../../../../workbench/common/editor/editorInput.js'; import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js'; +import { DockedEditorInput } from '../../../common/dockedEditorInput.js'; -export class EmptyFileEditorInput extends EditorInput { +export class EmptyFileEditorInput extends DockedEditorInput { static readonly ID = 'workbench.editors.agentSessions.emptyFile'; static readonly EDITOR_ID = 'workbench.editor.agentSessions.emptyFile'; diff --git a/src/vs/sessions/contrib/layout/browser/singlePane/singlePaneEditorAreaCollapseStrategy.ts b/src/vs/sessions/contrib/layout/browser/singlePane/singlePaneEditorAreaCollapseStrategy.ts index 638fd140b57..13adde243a5 100644 --- a/src/vs/sessions/contrib/layout/browser/singlePane/singlePaneEditorAreaCollapseStrategy.ts +++ b/src/vs/sessions/contrib/layout/browser/singlePane/singlePaneEditorAreaCollapseStrategy.ts @@ -12,13 +12,15 @@ import { IEditorGroupsService } from '../../../../../workbench/services/editor/c import { IEditorService } from '../../../../../workbench/services/editor/common/editorService.js'; import { Parts } from '../../../../../workbench/services/layout/browser/layoutService.js'; import { IAgentWorkbenchLayoutService } from '../../../../browser/workbench.js'; +import { DockedEditorInput } from '../../../../common/dockedEditorInput.js'; import { ISinglePaneLayoutContext, SinglePaneDockedTabsCoordinator, SinglePaneLayoutStrategy } from './singlePaneLayoutStrategy.js'; /** - * When the editor area is hidden (detail-only), closes the non-managed (real - * file) editors so only the managed Changes and Files tabs remain, capturing - * them so they are reopened when the editor area is shown again. Serializes on - * the shared docked-tab sequencer so it never races the managed-tab sync. + * When the editor area is hidden (detail-only), closes every non-docked editor + * so only the docked Changes and Files tabs remain. Editors that can be captured + * as a reopenable input are remembered and restored when the editor area is shown + * again; non-restorable ones (e.g. an untitled Search editor) are simply dropped. + * Serializes on the shared docked-tab sequencer so it never races the managed-tab sync. */ export class SinglePaneEditorAreaCollapseStrategy extends SinglePaneLayoutStrategy { @@ -67,14 +69,16 @@ export class SinglePaneEditorAreaCollapseStrategy extends SinglePaneLayoutStrate const captured: { editor: IUntypedEditorInput; index: number }[] = []; const toClose: EditorInput[] = []; group.editors.forEach((editor, index) => { - if (this._coordinator.isManagedEditor(editor) || editor.isDirty()) { + if (editor instanceof DockedEditorInput) { return; } + // Capture editors that can be reopened so they are restored when the + // editor area is shown again; the rest are still closed but not restored. const untyped = editor.toUntyped(); if (untyped) { captured.push({ editor: untyped, index }); - toClose.push(editor); } + toClose.push(editor); }); if (toClose.length === 0) { return; diff --git a/src/vs/sessions/contrib/layout/browser/singlePane/singlePaneLayoutStrategy.ts b/src/vs/sessions/contrib/layout/browser/singlePane/singlePaneLayoutStrategy.ts index 186465710ed..a7c96586395 100644 --- a/src/vs/sessions/contrib/layout/browser/singlePane/singlePaneLayoutStrategy.ts +++ b/src/vs/sessions/contrib/layout/browser/singlePane/singlePaneLayoutStrategy.ts @@ -11,7 +11,6 @@ import { URI } from '../../../../../base/common/uri.js'; import { EditorInput } from '../../../../../workbench/common/editor/editorInput.js'; import { IUntypedEditorInput } from '../../../../../workbench/common/editor.js'; import { ISessionChangesService } from '../../../changes/browser/sessionChangesService.js'; -import { EmptyFileEditorInput } from '../../../editor/browser/emptyFileEditorInput.js'; import { ISessionViewState } from '../baseSessionLayoutController.js'; /** @@ -56,17 +55,13 @@ export class SinglePaneDockedTabsCoordinator extends Disposable { /** Editors the controller itself is closing, so their close is not a user dismissal. */ readonly internallyClosingEditors = new Set(); - /** Non-managed editors closed (as reopenable inputs + tab index) while the editor area is hidden. */ + /** Non-docked editors closed (as reopenable inputs + tab index) while the editor area is hidden. */ collapsedEditors: { readonly editor: IUntypedEditorInput; readonly index: number }[] | undefined; constructor(private readonly _sessionChangesService: ISessionChangesService) { super(); } - isManagedEditor(editor: EditorInput): boolean { - return editor instanceof EmptyFileEditorInput || this.getChangesEditorResource(editor) !== undefined; - } - getChangesEditorResource(editor: EditorInput): URI | undefined { const resource = editor.resource; return resource && this._sessionChangesService.getSessionResource(resource) ? resource : undefined; diff --git a/src/vs/sessions/contrib/layout/browser/singlePane/singlePaneManagedTabsStrategy.ts b/src/vs/sessions/contrib/layout/browser/singlePane/singlePaneManagedTabsStrategy.ts index 1be29b9f07e..50944eaa112 100644 --- a/src/vs/sessions/contrib/layout/browser/singlePane/singlePaneManagedTabsStrategy.ts +++ b/src/vs/sessions/contrib/layout/browser/singlePane/singlePaneManagedTabsStrategy.ts @@ -24,6 +24,7 @@ import { ISessionsService } from '../../../../services/sessions/browser/sessions import { ISessionChangesService } from '../../../changes/browser/sessionChangesService.js'; import { IChangesViewService } from '../../../changes/common/changesViewService.js'; import { EmptyFileEditorInput } from '../../../editor/browser/emptyFileEditorInput.js'; +import { DockedEditorInput } from '../../../../common/dockedEditorInput.js'; import { ISinglePaneLayoutContext, SinglePaneDockedTabsCoordinator, SinglePaneLayoutStrategy } from './singlePaneLayoutStrategy.js'; const changesEditorOptions: IEditorOptions = { @@ -220,7 +221,7 @@ export class SinglePaneManagedTabsStrategy extends SinglePaneLayoutStrategy { /** Whether the editor shows a workspace file (a file-system resource), excluding managed placeholders. */ private _isWorkspaceFileEditor(editor: EditorInput): boolean { - if (this._coordinator.isManagedEditor(editor)) { + if (editor instanceof DockedEditorInput) { return false; } const resource = EditorResourceAccessor.getCanonicalUri(editor, { supportSideBySide: SideBySideEditor.PRIMARY }); diff --git a/src/vs/sessions/contrib/layout/browser/singlePaneLayoutController.ts b/src/vs/sessions/contrib/layout/browser/singlePaneLayoutController.ts index 0fd646b3206..d8e63a56cfc 100644 --- a/src/vs/sessions/contrib/layout/browser/singlePaneLayoutController.ts +++ b/src/vs/sessions/contrib/layout/browser/singlePaneLayoutController.ts @@ -97,12 +97,6 @@ export class SinglePaneLayoutController extends BaseLayoutController { } const coordinator = this._register(new SinglePaneDockedTabsCoordinator(this._sessionChangesService)); - // Managed tabs (Changes multi-diff, Files placeholder) surface their - // content in the detail panel, so opening them must not reveal the editor - // area. Own that policy here rather than hardcoding editor ids in the core - // workbench. - this._register(this._layoutService.setEditorRevealOnOpenExclusion(editor => coordinator.isManagedEditor(editor))); - this._register(this._instantiationService.createInstance(SinglePaneManagedTabsStrategy, this._ctx, coordinator)); this._register(this._instantiationService.createInstance(SinglePaneEditorAreaCollapseStrategy, this._ctx, coordinator)); this._register(this._instantiationService.createInstance(SinglePaneQuickChatEditorHideStrategy, this._ctx)); diff --git a/src/vs/sessions/contrib/layout/test/browser/desktopSessionLayoutController.test.ts b/src/vs/sessions/contrib/layout/test/browser/desktopSessionLayoutController.test.ts index 6ec5ab5109d..227c6cf79f4 100644 --- a/src/vs/sessions/contrib/layout/test/browser/desktopSessionLayoutController.test.ts +++ b/src/vs/sessions/contrib/layout/test/browser/desktopSessionLayoutController.test.ts @@ -2605,6 +2605,49 @@ suite('LayoutController (desktop)', () => { }); }); + test('[single-pane] closes a non-restorable non-docked tab (e.g. untitled Search) when the editor area hides, without restoring it', async () => { + createSinglePaneController({ activateAux: true }); + await settle(); + + harness.activeSessionObs.set(makeSession(URI.parse('session:1')), undefined); + await settle(); + + // A dirty, non-restorable editor (like an untitled Search editor) opens + // between the managed tabs while the editor area is visible. + const searchResource = URI.parse('search-editor:/Untitled-1'); + harness.activeGroupEditors.splice(1, 0, store.add(new TestStubEditorInput(searchResource, { dirty: true, nonRestorable: true }))); + harness.partVisibility.set(Parts.EDITOR_PART, true); + harness.onDidChangePartVisibility.fire({ partId: Parts.EDITOR_PART, visible: true }); + await settle(); + + // Hide the editor area: the non-docked tab closes even though it is dirty and + // cannot be captured; only the managed Files tab remains. + harness.partVisibility.set(Parts.EDITOR_PART, false); + harness.onDidChangePartVisibility.fire({ partId: Parts.EDITOR_PART, visible: false }); + await settle(); + + const closedSearch = harness.closedEditors.some(e => isEqual(e.resource!, searchResource)); + const searchTabGone = !harness.activeGroupEditors.some(e => e.resource && isEqual(e.resource, searchResource)); + + // Show the editor area again: the non-restorable tab is NOT reopened. + harness.openedEditors = []; + harness.partVisibility.set(Parts.EDITOR_PART, true); + harness.onDidChangePartVisibility.fire({ partId: Parts.EDITOR_PART, visible: true }); + await settle(); + + assert.deepStrictEqual({ + closedSearch, + searchTabGone, + filesTabKept: hasFilesTab(), + reopenedSearch: harness.openedEditors.some(e => isResourceEditorInput(e) && isEqual(e.resource, searchResource)), + }, { + closedSearch: true, + searchTabGone: true, + filesTabKept: true, + reopenedSearch: false, + }); + }); + test('[managed tabs / Change 2] does not re-ensure a managed tab after the user closes it', async () => { createSinglePaneController({ activateAux: true }); await settle(); diff --git a/src/vs/sessions/contrib/layout/test/browser/layoutControllerTestUtils.ts b/src/vs/sessions/contrib/layout/test/browser/layoutControllerTestUtils.ts index f184f830859..a6b5f984832 100644 --- a/src/vs/sessions/contrib/layout/test/browser/layoutControllerTestUtils.ts +++ b/src/vs/sessions/contrib/layout/test/browser/layoutControllerTestUtils.ts @@ -44,10 +44,11 @@ export function makeChange(filePath: string): ISessionFileChange { /** A minimal editor input for tests, identified only by its resource. */ export class TestStubEditorInput extends EditorInput { - constructor(private readonly _resource: URI) { super(); } + constructor(private readonly _resource: URI, private readonly _options?: { readonly dirty?: boolean; readonly nonRestorable?: boolean }) { super(); } override get typeId(): string { return 'test.stubEditor'; } override get resource(): URI { return this._resource; } - override toUntyped(): IUntypedEditorInput { return { resource: this._resource }; } + override isDirty(): boolean { return this._options?.dirty ?? false; } + override toUntyped(): IUntypedEditorInput | undefined { return this._options?.nonRestorable ? undefined : { resource: this._resource }; } } export function makeSession(resource: URI, opts?: { @@ -178,8 +179,6 @@ export interface ITestLayoutHarness { setPartHiddenCalls: { hidden: boolean; part: Parts }[]; /** Value returned by the layout service's `isEditorRevealedExplicitly()` mock. */ editorRevealedExplicitly: boolean; - /** Predicate registered via `setEditorRevealOnOpenExclusion()` (managed editors that shouldn't reveal the editor area). */ - editorRevealOnOpenExclusion: ((editor: EditorInput) => boolean) | undefined; /** Current suppression depth for `suppressEditorPartAutoVisibility()`. */ editorPartAutoVisibilitySuppressionDepth: number; /** Whether the lifecycle `Restored` phase has resolved (activates single-pane managed-tab / detail-panel behaviour). */ @@ -271,7 +270,6 @@ export function createTestHarness(store: DisposableStore, options: ICreateOption openedViews: [], setPartHiddenCalls: [], editorRevealedExplicitly: false, - editorRevealOnOpenExclusion: undefined, editorPartAutoVisibilitySuppressionDepth: 0, activateAux: options.activateAux ?? false, activeGroupEditors: [], @@ -366,10 +364,6 @@ export function createTestHarness(store: DisposableStore, options: ICreateOption harness.editorPartAutoVisibilitySuppressionDepth++; return toDisposable(() => harness.editorPartAutoVisibilitySuppressionDepth--); } - setEditorRevealOnOpenExclusion(predicate: (editor: EditorInput) => boolean): IDisposable { - harness.editorRevealOnOpenExclusion = predicate; - return toDisposable(() => { harness.editorRevealOnOpenExclusion = undefined; }); - } isEditorRevealedExplicitly(): boolean { return harness.editorRevealedExplicitly; } revealEditorPartExplicitly(): void { harness.editorRevealedExplicitly = true; diff --git a/src/vs/sessions/test/browser/workbench.test.ts b/src/vs/sessions/test/browser/workbench.test.ts index 3ea4986478b..f01017cb8ae 100644 --- a/src/vs/sessions/test/browser/workbench.test.ts +++ b/src/vs/sessions/test/browser/workbench.test.ts @@ -12,9 +12,16 @@ import { DockedAuxiliaryBarController, IDockedAuxiliaryBarHost } from '../../bro import { Workbench } from '../../browser/workbench.js'; import { DockedEditorSizeMemento, SinglePaneWorkbench } from '../../browser/singlePaneWorkbench.js'; import { SinglePaneMainEditorPart } from '../../browser/parts/singlePaneEditorPart.js'; +import { DockedEditorInput } from '../../common/dockedEditorInput.js'; interface IViewSize { width: number; height: number } +/** Minimal docked editor input for testing the single-pane reveal policy. */ +class TestDockedEditorInput extends DockedEditorInput { + override get typeId(): string { return 'test.dockedEditor'; } + override get resource(): undefined { return undefined; } +} + suite('Sessions - Workbench', () => { ensureNoDisposablesAreLeakedInTestSuite(); @@ -34,7 +41,8 @@ suite('Sessions - Workbench', () => { const restoreAttachedEditorMaximizedState = Reflect.get(Workbench.prototype, 'restoreAttachedEditorMaximizedState') as (this: IWorkbenchTestHarness) => void; const loadPartVisibility = Reflect.get(Workbench.prototype, '_loadPartVisibility') as (this: IWorkbenchTestHarness, storageService: { get(): string | undefined; remove(): void }) => { editor?: boolean; auxiliaryBar?: boolean; sidebar?: boolean }; const savePartVisibility = Reflect.get(Workbench.prototype, '_savePartVisibility') as (this: IWorkbenchTestHarness) => void; - const handleWillOpenEditor = Reflect.get(Workbench.prototype, '_handleWillOpenEditor') as (this: IWillOpenTestHarness, e: { groupId: number; editor: { typeId: string } }) => void; + const revealEditorOnOpen = Reflect.get(Workbench.prototype, 'revealEditorOnOpen') as (this: IWillOpenTestHarness, e: { groupId: number; editor: unknown }) => void; + const revealEditorOnOpenSinglePane = Reflect.get(SinglePaneWorkbench.prototype, 'revealEditorOnOpen') as (this: IWillOpenTestHarness, e: { groupId: number; editor: unknown }) => void; const createDesktopGridDescriptor = Reflect.get(Workbench.prototype, 'createDesktopGridDescriptor') as (this: IGridDescriptorTestHarness, width: number, height: number) => { root: { data: readonly unknown[] } }; const savePartSizes = Reflect.get(Workbench.prototype, '_savePartSizes') as (this: ISavePartSizesTestHarness) => void; @@ -576,8 +584,7 @@ suite('Sessions - Workbench', () => { interface IWillOpenTestHarness { _editorPartAutoVisibilitySuppressionCount: number; - _editorRevealOnOpenExclusion?: (editor: { typeId: string }) => boolean; - partVisibility: { editor: boolean }; + partVisibility: { editor: boolean; auxiliaryBar: boolean }; editorGroupService: { mainPart: { groups: { id: number }[] } }; setEditorHidden(hidden: boolean, explicit?: boolean): void; restoreAttachedEditorMaximizedState(): void; @@ -587,12 +594,7 @@ suite('Sessions - Workbench', () => { const setEditorHiddenCalls: { hidden: boolean; explicit?: boolean }[] = []; const harness: IWillOpenTestHarness = { _editorPartAutoVisibilitySuppressionCount: 0, - // Mirrors the predicate the single-pane layout controller registers for the - // managed Changes and Files tabs (their content lives in the detail panel). - _editorRevealOnOpenExclusion: editor => - editor.typeId === 'workbench.editors.agentSessions.emptyFile' || - editor.typeId === 'workbench.input.agentSessions.sessionChanges', - partVisibility: { editor: false }, + partVisibility: { editor: false, auxiliaryBar: false }, editorGroupService: { mainPart: { groups: [{ id: 1 }] } }, setEditorHidden: (hidden, explicit) => setEditorHiddenCalls.push({ hidden, explicit }), restoreAttachedEditorMaximizedState: () => { }, @@ -601,40 +603,67 @@ suite('Sessions - Workbench', () => { return { harness, setEditorHiddenCalls }; } - test('[Scenario 5] does not reveal a hidden editor when the managed empty Files tab is activated', () => { - const { harness, setEditorHiddenCalls } = createWillOpenHarness({ partVisibility: { editor: false } }); + test('[Scenario 5] base revealEditorOnOpen reveals a hidden editor on open', () => { + const { harness, setEditorHiddenCalls } = createWillOpenHarness({ partVisibility: { editor: false, auxiliaryBar: true } }); - // Closing the Changes tab activates the managed empty Files placeholder. - handleWillOpenEditor.call(harness, { groupId: 1, editor: { typeId: 'workbench.editors.agentSessions.emptyFile' } }); - - assert.deepStrictEqual(setEditorHiddenCalls, []); - }); - - test('[Scenario 5] does not reveal a hidden editor when the managed Changes tab is activated', () => { - const { harness, setEditorHiddenCalls } = createWillOpenHarness({ partVisibility: { editor: false } }); - - // Clicking the Changes tab activates the managed Changes multi-diff editor. - handleWillOpenEditor.call(harness, { groupId: 1, editor: { typeId: 'workbench.input.agentSessions.sessionChanges' } }); - - assert.deepStrictEqual(setEditorHiddenCalls, []); - }); - - test('[Scenario 5] reveals a hidden editor when a real editor is opened', () => { - const { harness, setEditorHiddenCalls } = createWillOpenHarness({ partVisibility: { editor: false } }); - - handleWillOpenEditor.call(harness, { groupId: 1, editor: { typeId: 'workbench.editors.files.fileEditorInput' } }); + revealEditorOnOpen.call(harness, { groupId: 1, editor: { typeId: 'workbench.editors.files.fileEditorInput' } }); assert.deepStrictEqual(setEditorHiddenCalls, [{ hidden: false, explicit: true }]); }); - test('[Scenario 5] does not reveal when the open targets a non-main-part group', () => { - const { harness, setEditorHiddenCalls } = createWillOpenHarness({ partVisibility: { editor: false } }); + test('[Scenario 5] base revealEditorOnOpen does not reveal when the open targets a non-main-part group', () => { + const { harness, setEditorHiddenCalls } = createWillOpenHarness(); - handleWillOpenEditor.call(harness, { groupId: 99, editor: { typeId: 'workbench.editors.files.fileEditorInput' } }); + revealEditorOnOpen.call(harness, { groupId: 99, editor: { typeId: 'workbench.editors.files.fileEditorInput' } }); assert.deepStrictEqual(setEditorHiddenCalls, []); }); + test('[Scenario 5] base revealEditorOnOpen does not reveal while editor-part auto-visibility is suppressed', () => { + const { harness, setEditorHiddenCalls } = createWillOpenHarness({ _editorPartAutoVisibilitySuppressionCount: 1 }); + + revealEditorOnOpen.call(harness, { groupId: 1, editor: { typeId: 'workbench.editors.files.fileEditorInput' } }); + + assert.deepStrictEqual(setEditorHiddenCalls, []); + }); + + test('[Scenario 5] single-pane does not reveal a docked editor while the detail panel is open and the editor is closed', () => { + // Re-activating a docked-detail editor (closing a neighbouring tab, or + // clicking the tab) while the detail panel already shows its content must + // not reveal the closed editor area. + const dockedEditor = new TestDockedEditorInput(); + const { harness, setEditorHiddenCalls } = createWillOpenHarness({ partVisibility: { editor: false, auxiliaryBar: true } }); + + try { + revealEditorOnOpenSinglePane.call(harness, { groupId: 1, editor: dockedEditor }); + assert.deepStrictEqual(setEditorHiddenCalls, []); + } finally { + dockedEditor.dispose(); + } + }); + + test('[Scenario 5] single-pane reveals a docked editor when the detail panel is closed', () => { + // With the whole side pane closed (detail panel hidden), opening a docked + // editor must reveal the editor area so its content becomes visible. + const dockedEditor = new TestDockedEditorInput(); + const { harness, setEditorHiddenCalls } = createWillOpenHarness({ partVisibility: { editor: false, auxiliaryBar: false } }); + + try { + revealEditorOnOpenSinglePane.call(harness, { groupId: 1, editor: dockedEditor }); + assert.deepStrictEqual(setEditorHiddenCalls, [{ hidden: false, explicit: true }]); + } finally { + dockedEditor.dispose(); + } + }); + + test('[Scenario 5] single-pane reveals a non-docked editor even while the detail panel is open', () => { + const { harness, setEditorHiddenCalls } = createWillOpenHarness({ partVisibility: { editor: false, auxiliaryBar: true } }); + + revealEditorOnOpenSinglePane.call(harness, { groupId: 1, editor: { typeId: 'workbench.editors.files.fileEditorInput' } }); + + assert.deepStrictEqual(setEditorHiddenCalls, [{ hidden: false, explicit: true }]); + }); + test('restores the docked editor node size when showing after hide', () => { const host = createHost({ single: true, sessionsWidth: 1000, hasAppliedInitialEditorSplit: true, dockedWidth: 320, editorWidth: 900, partVisibility: { editor: true, auxiliaryBar: true } });