From 40ba8c19cfabf03eb136c243a72eb517f2bcaa20 Mon Sep 17 00:00:00 2001 From: can1357 Date: Thu, 4 Jun 2026 05:37:39 +0200 Subject: [PATCH] fix(tui): preserved bottom-anchored viewport during offscreen shrink - Added cursor remapping support to deferredMutation intents and emitted cursor positions when deferring offscreen mutations. - Introduced planDeferredShrink to defer bottom-anchored offscreen deletions only when displayed rows were byte-identical, otherwise repainted the viewport. - Added regression tests covering unknown-win32 offscreen middle-shrink behavior, including cursor-only moves and tail-content-change cases. --- packages/tui/src/tui.ts | 56 +++++++++- packages/tui/test/render-regressions.test.ts | 104 +++++++++++++++++++ 2 files changed, 156 insertions(+), 4 deletions(-) diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index b94625eda..0b5d807b7 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -306,6 +306,11 @@ export class Container implements Component { } } +interface DeferredCursorUpdate { + position: { row: number; col: number } | null; + totalLines: number; +} + /** * Render intent. `#planRender` decides which one a frame is, and the * corresponding `#emit*` method owns the bytes written and the state update. @@ -324,7 +329,10 @@ export class Container implements Component { * native history. Keep row indices stable with blank tail padding, repaint * only the viewport, and defer the real shorter replay to a checkpoint. * - `deferredMutation`: a row-inserting edit would reindex native scrollback - * while the user is scrolled. Defer all bytes until a safe rebuild checkpoint. + * while the user is scrolled, or an offscreen-only shrink left the visible + * rows byte-identical (merely renumbered). Defer the content bytes until a + * safe rebuild checkpoint; a remapped cursor update may still be emitted so a + * caret-only move lands while the preserved rows stay put. * - `shrink`: trailing rows were dropped — clear extras inline. * - `diff`: differential repaint of visible rows / append new rows below. */ @@ -336,7 +344,7 @@ type RenderIntent = | { kind: "overlayRebuild" } | { kind: "viewportRepaint"; appendFrom?: number } | { kind: "deferredShrink"; paddedLength: number } - | { kind: "deferredMutation" } + | { kind: "deferredMutation"; cursor?: DeferredCursorUpdate } | { kind: "shrink" } | { kind: "diff"; firstChanged: number; lastChanged: number; appendedLines: boolean }; @@ -1389,6 +1397,7 @@ export class TUI extends Container { // 3. Classify intent. const intent = this.#planRender( lines, + cursorPos, widthChanged, heightChanged, prevViewportTop, @@ -1455,6 +1464,7 @@ export class TUI extends Container { this.#emitViewportRepaint(lines, width, height, cursorPos); return; case "deferredMutation": + if (intent.cursor) this.#writeCursorPosition(intent.cursor.position, intent.cursor.totalLines); return; case "deferredShrink": this.#emitViewportRepaint( @@ -1483,6 +1493,43 @@ export class TUI extends Container { } } + /** + * Resolve a content shrink that re-exposes committed scrollback rows into the + * safe deferred intent. `deferredShrink` blank-pads the tail to keep the + * viewport's pre-shrink absolute indices — correct only when the rows above + * the change are unchanged (a pure trailing shrink). An offscreen *middle* + * deletion shifts every later row up, so that padded slice would draw + * post-shift content at the old viewport indices and hide the live rows. For + * an offscreen-only change, compare the new bottom-anchored viewport against + * the still-displayed window: when they match the visible set is merely + * renumbered, so a no-op preserves it (re-anchoring the cursor onto the + * displayed rows); when the visible tail truly changed, repaint it. A change + * landing at or inside the viewport keeps `deferredShrink`, whose blank-pad + * semantics are correct for visible drops/edits. + */ + #planDeferredShrink( + newLines: string[], + height: number, + paddedViewportTop: number, + firstChanged: number, + cursorPos: { row: number; col: number } | null, + ): RenderIntent { + if (firstChanged < paddedViewportTop) { + const newViewportTop = Math.max(0, newLines.length - height); + for (let row = 0; row < height; row++) { + if ((newLines[newViewportTop + row] ?? "") !== (this.#previousLines[paddedViewportTop + row] ?? "")) { + return { kind: "viewportRepaint" }; + } + } + const mappedCursor = + cursorPos === null + ? null + : { row: paddedViewportTop + (cursorPos.row - newViewportTop), col: cursorPos.col }; + return { kind: "deferredMutation", cursor: { position: mappedCursor, totalLines: this.#previousLines.length } }; + } + return { kind: "deferredShrink", paddedLength: this.#previousLines.length }; + } + /** * Map the current frame onto a single render intent. Order matters: forced * resets and session replacement short-circuit before any diff work. A real @@ -1493,6 +1540,7 @@ export class TUI extends Container { */ #planRender( newLines: string[], + cursorPos: { row: number; col: number } | null, widthChanged: boolean, heightChanged: boolean, prevViewportTop: number, @@ -1594,7 +1642,7 @@ export class TUI extends Container { if (newLines.length <= paddedViewportTop) { return { kind: "deferredMutation" }; } - return { kind: "deferredShrink", paddedLength: this.#previousLines.length }; + return this.#planDeferredShrink(newLines, height, paddedViewportTop, diff.firstChanged, cursorPos); } // Non-ED3-risk POSIX with an unobservable viewport. If the shrink still @@ -1607,7 +1655,7 @@ export class TUI extends Container { return { kind: "historyRebuild" }; } this.#markNativeScrollbackDirty(); - return { kind: "deferredShrink", paddedLength: this.#previousLines.length }; + return this.#planDeferredShrink(newLines, height, paddedViewportTop, diff.firstChanged, cursorPos); } // Multiplexer panes do not give us a safe native-history rebuild path, but diff --git a/packages/tui/test/render-regressions.test.ts b/packages/tui/test/render-regressions.test.ts index dd166531d..48b43950b 100644 --- a/packages/tui/test/render-regressions.test.ts +++ b/packages/tui/test/render-regressions.test.ts @@ -2048,6 +2048,110 @@ describe("TUI terminal-state regressions", () => { tui.stop(); } }); + + it("preserves bottom-anchored viewport across an offscreen middle-shrink on unknown win32", async () => { + // Repro for #1809: `win32-unknown-small` blanked the visible window after a + // `deleteMiddle` far above the viewport. The shrink fell into the deferred + // fallback, which blank-pads the tail to keep pre-shrink absolute indices — + // correct only for trailing shrinks. An offscreen middle deletion shifts + // later rows up, so the padded slice at the old viewport indices showed + // post-shift content and dropped the live rows. The fix keeps the no-op + // when the new bottom-anchored window equals the displayed one. + const originalPlatform = process.platform; + Object.defineProperty(process, "platform", { configurable: true, value: "win32" }); + const term = new UnknownViewportTerminal(32, 6); + const tui = new TUI(term); + const initial = rows("line-", 30); + const component = new MutableLinesComponent(initial); + tui.addChild(component); + try { + tui.start(); + await settle(term); + expect(visible(term).map(l => l.trim())).toEqual(["line-24", "line-25", "line-26", "line-27", "line-28", "line-29"]); + // Delete one offscreen row (index 5): the last six rows are unchanged, + // only their absolute indices shift. + component.setLines([...initial.slice(0, 5), ...initial.slice(6)]); + tui.requestRender(true, { allowUnknownViewportMutation: true }); + await settle(term); + expect(visible(term).map(l => l.trim())).toEqual(["line-24", "line-25", "line-26", "line-27", "line-28", "line-29"]); + } finally { + Object.defineProperty(process, "platform", { configurable: true, value: originalPlatform }); + tui.stop(); + } + }); + + it("repaints the viewport when an offscreen shrink also changes the visible tail", async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, "platform", { configurable: true, value: "win32" }); + const term = new UnknownViewportTerminal(32, 6); + const tui = new TUI(term); + const initial = rows("line-", 30); + const component = new MutableLinesComponent(initial); + tui.addChild(component); + try { + tui.start(); + await settle(term); + // Offscreen deletion AND a visible tail change: the no-op would freeze + // the stale tail, so the renderer must repaint the true bottom. + component.setLines([...initial.slice(0, 5), ...initial.slice(6, -1), "prompt-updated"]); + tui.requestRender(true, { allowUnknownViewportMutation: true }); + await settle(term); + expect(visible(term).map(l => l.trim())).toEqual(["line-24", "line-25", "line-26", "line-27", "line-28", "prompt-updated"]); + } finally { + Object.defineProperty(process, "platform", { configurable: true, value: originalPlatform }); + tui.stop(); + } + }); + + it("preserves a cursor-only tail move while deferring an offscreen shrink on unknown win32", async () => { + const originalPlatform = process.platform; + Object.defineProperty(process, "platform", { configurable: true, value: "win32" }); + const term = new UnknownViewportTerminal(32, 6); + const tui = new TUI(term); + const initial = [...rows("line-", 29), `prompt>a${CURSOR_MARKER}b`]; + const component = new MutableLinesComponent(initial); + tui.addChild(component); + try { + tui.start(); + await settle(term); + expect(term.getCursor()).toEqual({ row: 5, col: "prompt>a".length }); + // Offscreen deletion; the visible rows are byte-identical after stripping + // CURSOR_MARKER, but the caret moves to the end. The deferred no-op must + // still emit the remapped cursor. + component.setLines([...initial.slice(0, 5), ...initial.slice(6, -1), `prompt>ab${CURSOR_MARKER}`]); + tui.requestRender(true, { allowUnknownViewportMutation: true }); + await settle(term); + expect(visible(term).map(l => l.trim())).toEqual(["line-24", "line-25", "line-26", "line-27", "line-28", "prompt>ab"]); + expect(term.getCursor()).toEqual({ row: 5, col: "prompt>ab".length }); + } finally { + Object.defineProperty(process, "platform", { configurable: true, value: originalPlatform }); + tui.stop(); + } + }); + + it("preserves bottom-anchored viewport across an offscreen middle-shrink on unknown ED3 eager-off", async () => { + // The same guard covers the ED3-risk eager-off deferredShrink site (the + // non-forced path, where the viewport probe is unobservable and the user + // may be following the live tail). An offscreen-only delete must keep the + // visible rows, not blank-pad shifted content. + await withTerminalRisk(true, async () => { + const term = new UnknownViewportTerminal(32, 6); + const tui = new TUI(term); + const initial = rows("line-", 30); + const component = new MutableLinesComponent(initial); + tui.addChild(component); + try { + tui.start(); + await settle(term); + component.setLines([...initial.slice(0, 5), ...initial.slice(6)]); + tui.requestRender(); + await settle(term); + expect(visible(term).map(l => l.trim())).toEqual(["line-24", "line-25", "line-26", "line-27", "line-28", "line-29"]); + } finally { + tui.stop(); + } + }); + }); it("defers bottom-anchored shrink when POSIX viewport state is unknown", async () => { // Repro for #1566 follow-up (kitty/Linux): a bottom-anchored shrink across the // viewport boundary used to fall through to `viewportRepaint`, which redrew the