diff --git a/src/vs/sessions/contrib/chat/browser/chat.contribution.ts b/src/vs/sessions/contrib/chat/browser/chat.contribution.ts index 54ccec5e8d0d0c..f9ca2be6e80d66 100644 --- a/src/vs/sessions/contrib/chat/browser/chat.contribution.ts +++ b/src/vs/sessions/contrib/chat/browser/chat.contribution.ts @@ -189,7 +189,7 @@ class NewChatInSessionsWindowAction extends Action2 { }); } - override run(accessor: ServicesAccessor): void { + override async run(accessor: ServicesAccessor, options?: { toSide?: boolean }): Promise { const sessionsService = accessor.get(ISessionsService); const sessionsManagementService = accessor.get(ISessionsManagementService); const activeSession = sessionsService.activeSession.get(); @@ -201,8 +201,9 @@ class NewChatInSessionsWindowAction extends Action2 { // Inherit the active session's harness so the new session defaults to // the kind the user is working in — but only while the folder still // offers it (see `inheritableSessionTarget`). - sessionsService.openNewSession({ + await sessionsService.openNewSession({ folderUri, + toSide: options?.toSide, ...inheritableSessionTarget(sessionsManagementService, activeSession, folderUri), }); } diff --git a/src/vs/sessions/contrib/sessions/browser/sessionsActions.ts b/src/vs/sessions/contrib/sessions/browser/sessionsActions.ts index eb07462e50a693..2653f8c62c06df 100644 --- a/src/vs/sessions/contrib/sessions/browser/sessionsActions.ts +++ b/src/vs/sessions/contrib/sessions/browser/sessionsActions.ts @@ -42,7 +42,7 @@ import { ISessionsService } from '../../../services/sessions/browser/sessionsSer import { ChatOriginKind, getChatCapabilities, getGitHubPullRequestRefs, getHighestPriorityPullRequestIcon, getUntitledSessionTitle, IChat, ISession, SessionStatus } from '../../../services/sessions/common/session.js'; import { ISessionsPartService } from '../../../services/sessions/browser/sessionsPartService.js'; import { ISessionsListModelService } from '../../../services/sessions/browser/sessionsListModelService.js'; -import { $, append, EventHelper, ModifierKeyEmitter, reset } from '../../../../base/browser/dom.js'; +import { $, append, EventHelper, isMouseEvent, ModifierKeyEmitter, reset } from '../../../../base/browser/dom.js'; import { BaseActionViewItem } from '../../../../base/browser/ui/actionbar/actionViewItems.js'; import { Button } from '../../../../base/browser/ui/button/button.js'; import { HoverPosition } from '../../../../base/browser/ui/hover/hoverWidget.js'; @@ -1270,6 +1270,10 @@ export abstract class CompactButtonActionViewItem extends BaseActionViewItem { /** Hook invoked right before the action runs (e.g. for telemetry). */ protected onRun(): void { } + protected runAction(_event: MouseEvent | undefined): void { + void this.actionRunner.run(this.action, this._context); + } + protected configureButton(_button: Button): void { } override render(container: HTMLElement): void { @@ -1301,7 +1305,7 @@ export abstract class CompactButtonActionViewItem extends BaseActionViewItem { return; } this.onRun(); - this.actionRunner.run(this.action, this._context); + this.runAction(isMouseEvent(e) ? e : undefined); })); const buttonLabel = $('span.new-session-button-label', undefined, this.label); @@ -1362,7 +1366,7 @@ export abstract class CompactButtonActionViewItem extends BaseActionViewItem { * Renders the new-session action as the compact "New" pill, shared by the sessions sidebar * header and the titlebar. */ -class NewSessionActionViewItem extends CompactButtonActionViewItem { +export class NewSessionActionViewItem extends CompactButtonActionViewItem { constructor( action: IAction, @@ -1372,6 +1376,7 @@ class NewSessionActionViewItem extends CompactButtonActionViewItem { @IHoverService hoverService: IHoverService, @ITelemetryService private readonly telemetryService: ITelemetryService, @IContextKeyService contextKeyService: IContextKeyService, + @ICommandService private readonly commandService: ICommandService, ) { super(action, keybindingService, hoverService, contextKeyService); } @@ -1411,6 +1416,14 @@ class NewSessionActionViewItem extends CompactButtonActionViewItem { protected override onRun(): void { logSessionsInteraction(this.telemetryService, 'newSession', this.telemetrySource); } + + protected override runAction(event: MouseEvent | undefined): void { + if (event?.altKey) { + this.commandService.executeCommand(NEW_SESSION_ACTION_ID, { toSide: true }).catch(onUnexpectedError); + } else { + super.runAction(event); + } + } } /** diff --git a/src/vs/sessions/contrib/sessions/test/browser/sessionsActions.test.ts b/src/vs/sessions/contrib/sessions/test/browser/sessionsActions.test.ts index 094ec49a36448d..e15d22181985f7 100644 --- a/src/vs/sessions/contrib/sessions/test/browser/sessionsActions.test.ts +++ b/src/vs/sessions/contrib/sessions/test/browser/sessionsActions.test.ts @@ -8,24 +8,27 @@ import { constObservable, observableValue } from '../../../../../base/common/obs import { URI } from '../../../../../base/common/uri.js'; import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js'; import { isIMenuItem, isISubmenuItem, MenuRegistry } from '../../../../../platform/actions/common/actions.js'; -import { CommandsRegistry } from '../../../../../platform/commands/common/commands.js'; +import { CommandsRegistry, ICommandService } from '../../../../../platform/commands/common/commands.js'; import { IConfigurationService } from '../../../../../platform/configuration/common/configuration.js'; import { TestConfigurationService } from '../../../../../platform/configuration/test/common/testConfigurationService.js'; import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js'; +import { KeybindingsRegistry } from '../../../../../platform/keybinding/common/keybindingsRegistry.js'; import { workbenchInstantiationService } from '../../../../../workbench/test/browser/workbenchTestServices.js'; import { Menus } from '../../../../browser/menus.js'; import { SESSION_CONVERSATION_SIDE_CHATS_GROUP } from '../../../../browser/sessionConversationGroups.js'; import { SessionView } from '../../../../browser/parts/sessionView.js'; import { ISessionsPartService } from '../../../../services/sessions/browser/sessionsPartService.js'; -import { ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; +import { type IOpenNewSessionOptions, type IOpenNewSessionResult, ISessionsService } from '../../../../services/sessions/browser/sessionsService.js'; import { ChatOriginKind, IChat, SessionStatus } from '../../../../services/sessions/common/session.js'; import { IActiveSession, ISessionsManagementService } from '../../../../services/sessions/common/sessionsManagement.js'; import { mock, upcastPartial } from '../../../../../base/test/common/mock.js'; -import { SessionConversationActionsContribution } from '../../browser/sessionsActions.js'; +import { Action } from '../../../../../base/common/actions.js'; +import { NewSessionActionViewItem, type NewSessionButtonStyle, SessionConversationActionsContribution } from '../../browser/sessionsActions.js'; +import '../../../chat/browser/chat.contribution.js'; +import { NEW_SESSION_ACTION_ID, UNIFIED_WORKSPACE_PICKER_SETTING } from '../../../chat/common/constants.js'; import '../../browser/views/sessionsViewActions.js'; -import { createTestSession } from './sessionsListTestUtils.js'; -import { UNIFIED_WORKSPACE_PICKER_SETTING } from '../../../chat/common/constants.js'; +import { createTestSession, TestCommandService } from './sessionsListTestUtils.js'; suite('Sessions - Actions', () => { @@ -240,4 +243,148 @@ suite('Sessions - Actions', () => { { title: 'Side chat', group: SESSION_CONVERSATION_SIDE_CHATS_GROUP }, ]); }); + + test('does not register a separate New Session to the Side command or shortcut', () => { + const commandId = 'workbench.action.sessions.newChatToSide'; + assert.deepStrictEqual({ + command: CommandsRegistry.getCommand(commandId), + keybindings: KeybindingsRegistry.getDefaultKeybindings().filter(binding => binding.command === commandId), + }, { + command: undefined, + keybindings: [], + }); + }); + + test('New button hover only includes the existing keyboard shortcut', () => { + class TestNewSessionActionViewItem extends NewSessionActionViewItem { + override getHoverContent(keybindingLabel: string | undefined): string { + return super.getHoverContent(keybindingLabel); + } + } + + const instantiationService = disposables.add(workbenchInstantiationService(undefined, disposables)); + instantiationService.stub(ICommandService, new TestCommandService()); + const action = disposables.add(new Action(NEW_SESSION_ACTION_ID, 'New')); + const item = disposables.add(instantiationService.createInstance( + TestNewSessionActionViewItem, action, 'sidebar', constObservable('default') + )); + + assert.deepStrictEqual({ + withKeybinding: item.getHoverContent('Ctrl+N'), + withoutKeybinding: item.getHoverContent(undefined), + }, { + withKeybinding: 'New Session (Ctrl+N)', + withoutKeybinding: 'New Session', + }); + }); + + for (const source of ['sidebar', 'titleBar'] as const) { + test(`New button in the ${source} opens to the side only on Alt-click`, () => { + const instantiationService = disposables.add(workbenchInstantiationService(undefined, disposables)); + const commandService = new TestCommandService(); + instantiationService.stub(ICommandService, commandService); + let primaryRuns = 0; + const action = disposables.add(new Action(NEW_SESSION_ACTION_ID, 'New', undefined, true, async () => { + primaryRuns++; + })); + const style = observableValue('newButtonStyle', 'default'); + const item = disposables.add(instantiationService.createInstance(NewSessionActionViewItem, action, source, style)); + const container = document.createElement('div'); + item.render(container); + + const button = container.querySelector('.agent-sessions-compact-new-button.monaco-button'); + assert.ok(button); + assert.deepStrictEqual({ + buttonCount: container.querySelectorAll('.monaco-button').length, + dropdown: container.querySelector('.monaco-button-dropdown'), + popup: button.getAttribute('aria-haspopup'), + label: button.querySelector('.new-session-button-label')?.textContent, + }, { + buttonCount: 1, + dropdown: null, + popup: null, + label: 'New', + }); + + button.click(); + button.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', keyCode: 13, bubbles: true, cancelable: true })); + button.dispatchEvent(new KeyboardEvent('keydown', { key: ' ', keyCode: 32, bubbles: true, cancelable: true })); + button.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true, altKey: true })); + action.enabled = false; + button.click(); + button.dispatchEvent(new MouseEvent('click', { bubbles: true, cancelable: true, altKey: true })); + + assert.deepStrictEqual({ primaryRuns, commands: commandService.calls }, { + primaryRuns: 3, + commands: [{ commandId: NEW_SESSION_ACTION_ID, args: [{ toSide: true }] }], + }); + + style.set('lightweightWithKeybindingBackground', undefined); + assert.deepStrictEqual({ + lightweight: button.classList.contains('lightweight'), + keybindingBackground: button.classList.contains('lightweight-keybinding-background'), + }, { lightweight: true, keybindingBackground: true }); + style.set('default', undefined); + assert.deepStrictEqual({ + lightweight: button.classList.contains('lightweight'), + keybindingBackground: button.classList.contains('lightweight-keybinding-background'), + }, { lightweight: false, keybindingBackground: false }); + }); + } + + for (const toSide of [undefined, true]) { + for (const scenario of [ + { name: 'workspace', isQuickChat: false, targetAvailable: true }, + { name: 'unavailable provider', isQuickChat: false, targetAvailable: false }, + { name: 'quick chat', isQuickChat: true, targetAvailable: true }, + ]) { + test(`New Session preserves ${scenario.name} inheritance with toSide=${toSide}`, async () => { + const instantiationService = disposables.add(new TestInstantiationService()); + const { session } = createTestSession('active'); + const activeSession = upcastPartial({ + ...session, + isQuickChat: constObservable(scenario.isQuickChat), + }); + const requests: (IOpenNewSessionOptions | undefined)[] = []; + instantiationService.stub(ISessionsService, new class extends mock() { + override readonly activeSession = constObservable(activeSession); + override async openNewSession(options?: IOpenNewSessionOptions): Promise { + requests.push(options); + return { session: undefined, trustDeclined: false }; + } + }); + instantiationService.stub(ISessionsManagementService, new class extends mock() { + override isNewSessionTargetAvailable(): boolean { return scenario.targetAvailable; } + }); + + const command = CommandsRegistry.getCommand(NEW_SESSION_ACTION_ID); + assert.ok(command); + await command.handler(instantiationService, toSide ? { toSide } : undefined); + + assert.deepStrictEqual(requests, [{ + folderUri: scenario.isQuickChat ? undefined : session.workspace.get()?.uri, + toSide, + ...(!scenario.isQuickChat && scenario.targetAvailable ? { + providerId: session.providerId, + sessionTypeId: session.sessionType, + } : {}), + }]); + }); + } + } + + test('New Session propagates opening failures', async () => { + const instantiationService = disposables.add(new TestInstantiationService()); + const error = new Error('Opening failed'); + instantiationService.stub(ISessionsService, new class extends mock() { + override readonly activeSession = constObservable(undefined); + override async openNewSession(): Promise { + throw error; + } + }); + instantiationService.stub(ISessionsManagementService, new class extends mock() { }); + const command = CommandsRegistry.getCommand(NEW_SESSION_ACTION_ID); + assert.ok(command); + await assert.rejects(async () => command.handler(instantiationService, { toSide: true }), error); + }); }); diff --git a/src/vs/sessions/services/sessions/browser/sessionsService.ts b/src/vs/sessions/services/sessions/browser/sessionsService.ts index fa5adf45f2c325..52c4a03b4ef193 100644 --- a/src/vs/sessions/services/sessions/browser/sessionsService.ts +++ b/src/vs/sessions/services/sessions/browser/sessionsService.ts @@ -64,6 +64,12 @@ export interface IOpenNewSessionOptions extends ICreateNewSessionOptions { readonly folderUri?: URI; /** Cancel startup session restoration so this new-session navigation wins. */ readonly cancelRestore?: boolean; + + /** + * When `true`, opens the new session (or empty composer slot) to the side + * of the active session in the grid instead of replacing it in place. + */ + readonly toSide?: boolean; } /** @@ -270,9 +276,10 @@ export interface ISessionsService { /** * Insert (or move) a session into the grid positioned next to a target - * session that is already visible. + * session that is already visible. Passing `undefined` operates on the + * empty (new-session) slot. */ - insertAt(session: ISession, targetSessionId: string, side: 'left' | 'right', activate?: boolean): void; + insertAt(session: ISession | undefined, targetSessionId: string, side: 'left' | 'right', activate?: boolean): void; /** * Toggle a session's stickiness in the grid. The session keeps its grid @@ -1085,7 +1092,7 @@ export class SessionsService extends Disposable implements ISessionsService { this._startOpenSession(); try { const session = this.sessionsManagementService.createNewSession(folderUri, options); - this._activate(session); + this._activateOrInsert(session, options?.toSide); return { session, trustDeclined: false }; } catch (e) { // When the folder cannot be resolved (e.g. the active session's @@ -1097,11 +1104,11 @@ export class SessionsService extends Disposable implements ISessionsService { // Without a folder (or when folder resolution failed above): switch to // the new-session composer view. - // No-op when no session is active (empty new-session placeholder showing). + // No-op when the empty new-session placeholder is active, unless opening to the side. if (!folderUri) { this._dismissCustomViewForNavigation(intent); } - if (this._visibility.activeSession.get() === undefined) { + if (this._visibility.activeSession.get() === undefined && !options?.toSide) { return { session: undefined, trustDeclined: false }; } if (!folderUri) { @@ -1113,8 +1120,25 @@ export class SessionsService extends Disposable implements ISessionsService { // active session (first time / after send). const newSession = this.sessionsManagementService.newSession.get(); - this._activate(newSession ?? undefined); - return { session: newSession ?? undefined, trustDeclined: false }; + const targetSession = newSession ?? undefined; + this._activateOrInsert(targetSession, options?.toSide); + return { session: targetSession, trustDeclined: false }; + } + + /** Open or move beside the active session when requested, keeping a single empty slot. */ + private _activateOrInsert(session: ISession | undefined, toSide: boolean | undefined): void { + const activeSessionId = this._visibility.activeSession.get()?.sessionId; + const sessionId = session?.sessionId; + if (toSide && activeSessionId !== sessionId) { + const visible = this.visibleSessions.get(); + // An empty active slot has no id; fall back to the rightmost session. + const anchorId = activeSessionId ?? visible[visible.length - 1]?.sessionId; + if (anchorId && anchorId !== sessionId) { + this.insertAt(session, anchorId, 'right', true); + return; + } + } + this._activate(session); } openQuickChat(options?: ICreateNewSessionOptions): IActiveSession | undefined { @@ -1178,7 +1202,7 @@ export class SessionsService extends Disposable implements ISessionsService { this._onDidToggleSessionStickiness.fire({ session, sticky }); } - insertAt(session: ISession, targetSessionId: string, side: 'left' | 'right', activate: boolean = true): void { + insertAt(session: ISession | undefined, targetSessionId: string, side: 'left' | 'right', activate: boolean = true): void { this._visibility.insertAt(session, targetSessionId, side, activate); } diff --git a/src/vs/sessions/services/sessions/browser/visibleSessions.ts b/src/vs/sessions/services/sessions/browser/visibleSessions.ts index 7cf101bcd319ac..8342a58e01f237 100644 --- a/src/vs/sessions/services/sessions/browser/visibleSessions.ts +++ b/src/vs/sessions/services/sessions/browser/visibleSessions.ts @@ -472,10 +472,10 @@ export class VisibleSessions extends Disposable { * "open at position" entry points. * * - If the slot is not yet visible, a new non-sticky entry is created - * at the computed position. For an `undefined` session (empty slot), - * this is a no-op when an empty slot already exists in the grid. + * at the computed position. * - If the slot is already visible, it is moved to the computed - * position; its sticky / non-sticky state is preserved. + * position; its sticky / non-sticky state is preserved. This also + * moves the single empty slot when `session` is `undefined`. * * When `activate` is `true` (default), the inserted slot also becomes * the active session. When `false`, the active session is left @@ -491,12 +491,6 @@ export class VisibleSessions extends Disposable { return; } - // Invariant: at most one empty slot. If inserting the empty slot and - // one already exists, do not add or move another. - if (id === undefined && this._visibleList.includes(undefined)) { - return; - } - let destIdx = side === 'left' ? targetIdx : targetIdx + 1; const currentIdx = this._visibleList.indexOf(id); diff --git a/src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts b/src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts index de90014ff2fd69..710bfa1e459bcf 100644 --- a/src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts +++ b/src/vs/sessions/services/sessions/test/browser/sessionsManagementService.test.ts @@ -510,6 +510,141 @@ suite('SessionsManagementService', () => { }); }); + test('openNewSession with toSide places the new-session composer beside the active session', async () => { + const session = stubSession({ sessionId: 'active', providerId: 'test' }); + const { view } = createSessionsManagementService(session, disposables); + + await view.openSession(session.resource); + assert.deepStrictEqual(view.visibleSessions.get().map(s => s?.sessionId ?? null), ['active']); + + await view.openNewSession({ toSide: true }); + assert.deepStrictEqual(view.visibleSessions.get().map(s => s?.sessionId ?? null), ['active', null]); + assert.strictEqual(view.activeSession.get(), undefined); + }); + + test('openNewSession without toSide still replaces the active session', async () => { + const session = stubSession({ sessionId: 'active', providerId: 'test' }); + const { view } = createSessionsManagementService(session, disposables); + + await view.openSession(session.resource); + await view.openNewSession(); + + assert.deepStrictEqual({ + visible: view.visibleSessions.get().map(s => s?.sessionId ?? null), + active: view.activeSession.get(), + }, { visible: [null], active: undefined }); + }); + + test('openNewSession with toSide moves the existing composer beside the active session', async () => { + const first = stubSession({ sessionId: 'first', providerId: 'test' }); + const middle = stubSession({ sessionId: 'middle', providerId: 'test' }); + const last = stubSession({ sessionId: 'last', providerId: 'test' }); + const provider = new class extends TestSessionsProvider { + constructor() { super(first); } + override getSessions(): ISession[] { return [first, middle, last]; } + }; + const { view } = createSessionsManagementService(first, disposables, provider); + + await view.openSession(first.resource); + view.insertAt(middle, first.sessionId, 'right', false); + view.insertAt(last, middle.sessionId, 'right', false); + await view.openNewSession({ toSide: true }); + const inserted = view.visibleSessions.get().map(s => s?.sessionId ?? null); + await view.openNewSession({ toSide: true }); + const repeated = view.visibleSessions.get().map(s => s?.sessionId ?? null); + await view.openSession(last.resource); + await view.openNewSession({ toSide: true }); + const movedRight = view.visibleSessions.get().map(s => s?.sessionId ?? null); + await view.openSession(first.resource); + await view.openNewSession({ toSide: true }); + + assert.deepStrictEqual({ + inserted, + repeated, + movedRight, + movedLeft: view.visibleSessions.get().map(s => s?.sessionId ?? null), + active: view.activeSession.get(), + }, { + inserted: ['first', null, 'middle', 'last'], + repeated: ['first', null, 'middle', 'last'], + movedRight: ['first', 'middle', 'last', null], + movedLeft: ['first', null, 'middle', 'last'], + active: undefined, + }); + }); + + test('openNewSession without toSide leaves an existing composer in place', async () => { + const first = stubSession({ sessionId: 'first', providerId: 'test' }); + const last = stubSession({ sessionId: 'last', providerId: 'test' }); + const provider = new class extends TestSessionsProvider { + constructor() { super(first); } + override getSessions(): ISession[] { return [first, last]; } + }; + const { view } = createSessionsManagementService(first, disposables, provider); + + await view.openSession(first.resource); + view.insertAt(last, first.sessionId, 'right', false); + await view.openNewSession({ toSide: true }); + await view.openSession(last.resource); + await view.openNewSession(); + + assert.deepStrictEqual({ + visible: view.visibleSessions.get().map(s => s?.sessionId ?? null), + active: view.activeSession.get(), + }, { + visible: ['first', null, 'last'], + active: undefined, + }); + }); + + test('repeated openNewSession with toSide keeps the single empty slot and re-activates it', async () => { + const session = stubSession({ sessionId: 'active', providerId: 'test' }); + const { view } = createSessionsManagementService(session, disposables); + + await view.openSession(session.resource); + await view.openNewSession({ toSide: true }); + await view.openNewSession({ toSide: true }); + // Go back to the session, then ask for a side composer again. The grid caps + // at one empty slot, so the existing one is re-activated rather than duplicated. + await view.openSession(session.resource); + await view.openNewSession({ toSide: true }); + + assert.deepStrictEqual({ + visible: view.visibleSessions.get().map(s => s?.sessionId ?? null), + active: view.activeSession.get()?.sessionId ?? null, + }, { + visible: ['active', null], + active: null, + }); + }); + + test('openNewSession with toSide and folderUri places the new session beside the active session', async () => { + const makeWorkspace = (uri: URI): ISessionWorkspace => ({ + uri, + label: 'ws', + icon: Codicon.vm, + folders: [{ root: uri, workingDirectory: uri, name: 'ws', description: undefined }], + requiresWorkspaceTrust: false, + isVirtualWorkspace: false, + }); + const session = stubSession({ sessionId: 'active', providerId: 'test' }); + const folderUri = URI.file('/test/workspace'); + const newDraftSession = stubSession({ sessionId: 'new-draft', providerId: 'test', workspace: constObservable(makeWorkspace(folderUri)) }); + const provider = new class extends TestSessionsProvider { + constructor() { super(session); } + override resolveWorkspace(folder?: URI): ISessionWorkspace { return makeWorkspace(folder!); } + override createNewSession(): ISession { return newDraftSession; } + }; + const { view } = createSessionsManagementService(session, disposables, provider); + + await view.openSession(session.resource); + assert.deepStrictEqual(view.visibleSessions.get().map(s => s?.sessionId ?? null), ['active']); + + await view.openNewSession({ folderUri, toSide: true }); + assert.deepStrictEqual(view.visibleSessions.get().map(s => s?.sessionId ?? null), ['active', 'new-draft']); + assert.strictEqual(view.activeSession.get()?.sessionId, 'new-draft'); + }); + test('removing the active chat keeps the custom view open', async () => { const sideChat: IChat = { ...stubChat, diff --git a/src/vs/sessions/services/sessions/test/browser/visibleSessions.test.ts b/src/vs/sessions/services/sessions/test/browser/visibleSessions.test.ts index 00e80673c05fe1..31597cceca1783 100644 --- a/src/vs/sessions/services/sessions/test/browser/visibleSessions.test.ts +++ b/src/vs/sessions/services/sessions/test/browser/visibleSessions.test.ts @@ -580,23 +580,49 @@ suite('VisibleSessions', () => { }); }); - test('insertAt(undefined, ...) is a no-op when the empty slot already exists', () => { + for (const side of ['left', 'right'] as const) { + for (const activate of [false, true]) { + test(`moves the existing empty slot to the ${side} with activate=${activate}`, () => { + const model = createModel(); + const A = stubSession('A'); + const B = stubSession('B'); + const C = stubSession('C'); + model.restoreGrid([ + { session: A, sticky: true }, + { session: undefined, sticky: false }, + { session: B, sticky: true }, + { session: C, sticky: false }, + ], 2); + + model.insertAt(undefined, side === 'left' ? 'A' : 'C', side, activate); + + assert.deepStrictEqual(snapshot(model), { + visible: side === 'left' ? [undefined, 'A', 'B', 'C'] : ['A', 'B', 'C', undefined], + active: activate ? undefined : 'B', + sticky: ['A', 'B'], + }); + }); + } + } + + test('moving the empty slot makes it the most-recent non-sticky slot', () => { const model = createModel(); const A = stubSession('A'); const B = stubSession('B'); + const C = stubSession('C'); + model.restoreGrid([ + { session: A, sticky: true }, + { session: undefined, sticky: false }, + { session: B, sticky: false }, + ], 0); - model.setActive(A); - model.toggleStickiness(A); - model.setActive(B); - model.toggleStickiness(B); // [A, B] sticky:[A, B] - model.insertAt(undefined, 'A', 'right'); // [A, undefined, B] active becomes empty slot - model.setActive(B); // re-activate B - model.insertAt(undefined, 'B', 'right'); // no-op — empty slot already exists + model.insertAt(undefined, 'B', 'right', false); + model.setActive(C); assert.deepStrictEqual(snapshot(model), { - visible: ['A', undefined, 'B'], - active: 'B', - sticky: ['A', 'B'], + visible: ['A', 'B', 'C'], + active: 'C', + sticky: ['A'], }); }); });