diff --git a/src/renderer/src/lib/pane-manager/pane-fit-resize-observer.test.ts b/src/renderer/src/lib/pane-manager/pane-fit-resize-observer.test.ts index c2cdb6e03..2085a3fa4 100644 --- a/src/renderer/src/lib/pane-manager/pane-fit-resize-observer.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-fit-resize-observer.test.ts @@ -62,6 +62,7 @@ function createPane(): ManagedPaneInternal { compositionHandler: null, debugLabel: null, pendingSplitScrollState: { + bufferType: 'normal', wasAtBottom: true, firstVisibleLineContent: '', viewportY: 0, diff --git a/src/renderer/src/lib/pane-manager/pane-manager-types.ts b/src/renderer/src/lib/pane-manager/pane-manager-types.ts index 32041bd81..08d1661f1 100644 --- a/src/renderer/src/lib/pane-manager/pane-manager-types.ts +++ b/src/renderer/src/lib/pane-manager/pane-manager-types.ts @@ -67,6 +67,7 @@ export type ManagedPane = { // --------------------------------------------------------------------------- export type ScrollState = { + bufferType: 'normal' | 'alternate' wasAtBottom: boolean firstVisibleLineContent: string viewportY: number diff --git a/src/renderer/src/lib/pane-manager/pane-scroll.ts b/src/renderer/src/lib/pane-manager/pane-scroll.ts index 6f4304762..ff895174c 100644 --- a/src/renderer/src/lib/pane-manager/pane-scroll.ts +++ b/src/renderer/src/lib/pane-manager/pane-scroll.ts @@ -51,11 +51,12 @@ export function findLineByContent(terminal: Terminal, content: string, hintRatio export function captureScrollState(terminal: Terminal): ScrollState { const buf = terminal.buffer.active + const bufferType = buf.type const viewportY = buf.viewportY const wasAtBottom = viewportY >= buf.baseY const firstVisibleLineContent = buf.getLine(viewportY)?.translateToString(true)?.trimEnd() ?? '' const totalLines = buf.baseY + terminal.rows - return { wasAtBottom, firstVisibleLineContent, viewportY, totalLines } + return { bufferType, wasAtBottom, firstVisibleLineContent, viewportY, totalLines } } export function restoreScrollState(terminal: Terminal, state: ScrollState): void { diff --git a/src/renderer/src/lib/pane-manager/pane-split-scroll.test.ts b/src/renderer/src/lib/pane-manager/pane-split-scroll.test.ts new file mode 100644 index 000000000..4b059bb29 --- /dev/null +++ b/src/renderer/src/lib/pane-manager/pane-split-scroll.test.ts @@ -0,0 +1,180 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { ManagedPaneInternal, ScrollState } from './pane-manager-types' + +const restoreScrollState = vi.hoisted(() => vi.fn()) + +vi.mock('./pane-scroll', () => ({ + restoreScrollState +})) + +import { scheduleSplitScrollRestore } from './pane-split-scroll' + +const scrollState = { + bufferType: 'normal', + wasAtBottom: true, + firstVisibleLineContent: '', + viewportY: 0, + totalLines: 24 +} satisfies ScrollState + +const alternateScrollState = { + ...scrollState, + bufferType: 'alternate' +} satisfies ScrollState + +function createPane(bufferType: 'normal' | 'alternate'): { + pane: ManagedPaneInternal + bufferChangeDisposable: { dispose: ReturnType } + triggerBufferChange: (bufferType: 'normal' | 'alternate') => void +} { + let bufferChangeHandler: ((buffer: { type: 'normal' | 'alternate' }) => void) | null = null + const bufferChangeDisposable = { dispose: vi.fn() } + const pane: ManagedPaneInternal = { + id: 1, + terminal: { + rows: 24, + refresh: vi.fn(), + buffer: { + active: { + type: bufferType, + length: 24 + }, + onBufferChange: vi.fn((handler: (buffer: { type: 'normal' | 'alternate' }) => void) => { + bufferChangeHandler = handler + return bufferChangeDisposable + }) + } + } as never, + container: { + querySelectorAll: vi.fn(() => []) + } as never, + xtermContainer: {} as never, + linkTooltip: {} as never, + terminalGpuAcceleration: 'auto', + gpuRenderingEnabled: true, + webglAttachmentDeferred: false, + webglDisabledAfterContextLoss: false, + hasComplexScriptOutput: false, + webglAddon: null, + ligaturesAddon: null, + fitResizeObserver: null, + pendingObservedFitRafId: null, + fitAddon: {} as never, + searchAddon: {} as never, + serializeAddon: { + serialize: vi.fn(() => '') + } as never, + unicode11Addon: {} as never, + webLinksAddon: {} as never, + compositionHandler: null, + pendingSplitScrollState: scrollState, + debugLabel: null + } + return { + pane, + bufferChangeDisposable, + triggerBufferChange: (bufferType) => bufferChangeHandler?.({ type: bufferType }) + } +} + +describe('scheduleSplitScrollRestore', () => { + beforeEach(() => { + vi.useFakeTimers() + vi.stubGlobal('requestAnimationFrame', (callback: FrameRequestCallback) => { + callback(16) + return 1 + }) + restoreScrollState.mockClear() + }) + + afterEach(() => { + vi.clearAllTimers() + vi.useRealTimers() + vi.unstubAllGlobals() + }) + + it('restores and refreshes normal-screen panes after split reparenting settles', () => { + const { pane } = createPane('normal') + const reattachWebgl = vi.fn() + + scheduleSplitScrollRestore( + () => pane, + pane.id, + scrollState, + () => false, + reattachWebgl + ) + + expect(restoreScrollState).toHaveBeenCalledTimes(1) + expect(pane.terminal.refresh).toHaveBeenCalledWith(0, 23) + + vi.advanceTimersByTime(200) + + expect(pane.pendingSplitScrollState).toBeNull() + expect(reattachWebgl).toHaveBeenCalledWith(pane) + expect(restoreScrollState).toHaveBeenCalledTimes(2) + expect(pane.terminal.refresh).toHaveBeenCalledTimes(2) + }) + + it('defers WebGL reattach and skips scroll restore for alternate-screen panes', () => { + const { pane, bufferChangeDisposable, triggerBufferChange } = createPane('alternate') + const reattachWebgl = vi.fn() + + scheduleSplitScrollRestore( + () => pane, + pane.id, + alternateScrollState, + () => false, + reattachWebgl + ) + + expect(restoreScrollState).not.toHaveBeenCalled() + expect(pane.terminal.refresh).not.toHaveBeenCalled() + expect(pane.pendingSplitScrollState).toBe(scrollState) + + vi.advanceTimersByTime(200) + + expect(pane.pendingSplitScrollState).toBeNull() + expect(reattachWebgl).not.toHaveBeenCalled() + expect(restoreScrollState).not.toHaveBeenCalled() + expect(pane.terminal.refresh).not.toHaveBeenCalled() + + triggerBufferChange('alternate') + + expect(reattachWebgl).not.toHaveBeenCalled() + expect(bufferChangeDisposable.dispose).not.toHaveBeenCalled() + + triggerBufferChange('normal') + + expect(bufferChangeDisposable.dispose).toHaveBeenCalledTimes(1) + expect(reattachWebgl).toHaveBeenCalledWith(pane) + expect(restoreScrollState).not.toHaveBeenCalled() + expect(pane.terminal.refresh).not.toHaveBeenCalled() + }) + + it('defers normal-screen scroll restore until an active TUI exits alternate screen', () => { + const { pane, triggerBufferChange } = createPane('alternate') + const reattachWebgl = vi.fn() + + scheduleSplitScrollRestore( + () => pane, + pane.id, + scrollState, + () => false, + reattachWebgl + ) + + vi.advanceTimersByTime(200) + + expect(pane.pendingSplitScrollState).toBe(scrollState) + expect(restoreScrollState).not.toHaveBeenCalled() + expect(pane.terminal.refresh).not.toHaveBeenCalled() + + triggerBufferChange('normal') + + expect(pane.pendingSplitScrollState).toBeNull() + expect(reattachWebgl).toHaveBeenCalledWith(pane) + expect(restoreScrollState).toHaveBeenCalledWith(pane.terminal, scrollState) + expect(pane.terminal.refresh).toHaveBeenCalledWith(0, 23) + }) +}) diff --git a/src/renderer/src/lib/pane-manager/pane-split-scroll.ts b/src/renderer/src/lib/pane-manager/pane-split-scroll.ts index ac102135d..b8b00747c 100644 --- a/src/renderer/src/lib/pane-manager/pane-split-scroll.ts +++ b/src/renderer/src/lib/pane-manager/pane-split-scroll.ts @@ -1,3 +1,4 @@ +import type { IBuffer, IDisposable } from '@xterm/xterm' import type { ManagedPaneInternal, ScrollState } from './pane-manager-types' import { restoreScrollState } from './pane-scroll' @@ -9,6 +10,43 @@ function refreshAfterReparent(pane: ManagedPaneInternal): void { } } +function runAfterNormalBuffer( + pane: ManagedPaneInternal, + getPaneById: (id: number) => ManagedPaneInternal | undefined, + paneId: number, + isDestroyed: () => boolean, + callback: (pane: ManagedPaneInternal) => void +): void { + let disposable: IDisposable | null = null + disposable = pane.terminal.buffer.onBufferChange((buffer: IBuffer) => { + if (buffer.type === 'alternate') { + return + } + disposable?.dispose() + disposable = null + if (isDestroyed()) { + return + } + const live = getPaneById(paneId) + if (live) { + callback(live) + } + }) +} + +function restoreCapturedScrollState( + pane: ManagedPaneInternal, + scrollState: ScrollState, + reattachWebgl?: (pane: ManagedPaneInternal) => void +): void { + pane.pendingSplitScrollState = null + if (reattachWebgl) { + reattachWebgl(pane) + } + restoreScrollState(pane.terminal, scrollState) + refreshAfterReparent(pane) +} + function logPaneHealth(pane: ManagedPaneInternal, phase: string): void { const canvases = pane.container.querySelectorAll('canvas') const canvasInfo = Array.from(canvases).map((c) => { @@ -89,10 +127,19 @@ export function scheduleSplitScrollRestore( return } const live = getPaneById(paneId) - if (live?.pendingSplitScrollState) { - restoreScrollState(live.terminal, scrollState) - refreshAfterReparent(live) + if (!live?.pendingSplitScrollState) { + return } + // Why: see the 200ms timer below — the alt-screen buffer belongs to a + // TUI and restore-during-draw knocks its cursor one row off (#1298). + if ( + scrollState.bufferType === 'alternate' || + live.terminal.buffer.active.type === 'alternate' + ) { + return + } + restoreScrollState(live.terminal, scrollState) + refreshAfterReparent(live) }) }) @@ -104,12 +151,32 @@ export function scheduleSplitScrollRestore( if (!live) { return } - live.pendingSplitScrollState = null - if (reattachWebgl) { - reattachWebgl(live) + // Why: the alt-screen buffer belongs to a full-screen TUI (Claude Code, + // vim, less) that owns its cursor position. Re-running scroll restore + // and a full refresh here clobbers an in-progress draw — refresh(0, + // rows-1) repaints rows from xterm's buffer, racing the TUI's next + // write and leaving its cursor one row off (#1298 regression). + // WebGL reattach also refreshes, so defer it until the TUI exits the + // alternate buffer. Alt-screen has no scrollback, so scroll restore has + // nothing legitimate to do. + if (scrollState.bufferType === 'alternate') { + live.pendingSplitScrollState = null + if (live.terminal.buffer.active.type === 'alternate' && reattachWebgl) { + runAfterNormalBuffer(live, getPaneById, paneId, isDestroyed, reattachWebgl) + return + } + if (reattachWebgl) { + reattachWebgl(live) + } + return } - restoreScrollState(live.terminal, scrollState) - refreshAfterReparent(live) + if (live.terminal.buffer.active.type === 'alternate') { + runAfterNormalBuffer(live, getPaneById, paneId, isDestroyed, (normalPane) => { + restoreCapturedScrollState(normalPane, scrollState, reattachWebgl) + }) + return + } + restoreCapturedScrollState(live, scrollState, reattachWebgl) }, 200) setTimeout(() => { diff --git a/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts b/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts index 695f3aaeb..f0c78cce2 100644 --- a/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts +++ b/src/renderer/src/lib/pane-manager/pane-tree-ops.test.ts @@ -106,6 +106,7 @@ describe('safeFit', () => { terminalRows: 32 }) pane.pendingSplitScrollState = { + bufferType: 'normal', wasAtBottom: true, firstVisibleLineContent: '', viewportY: 0,