From f6fca1f5cdbd6a17e7f89727fb42a78b25214895 Mon Sep 17 00:00:00 2001 From: can1357 Date: Thu, 4 Jun 2026 06:15:40 +0200 Subject: [PATCH] undo(tui): preserved bottom-anchored viewport during offscreen shrink" --- packages/tui/src/tui.ts | 59 +------- packages/tui/test/render-regressions.test.ts | 139 ------------------- 2 files changed, 4 insertions(+), 194 deletions(-) diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index d56cd7d42..b94625eda 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -306,11 +306,6 @@ 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. @@ -329,10 +324,7 @@ interface DeferredCursorUpdate { * 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, 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. + * while the user is scrolled. Defer all bytes until a safe rebuild checkpoint. * - `shrink`: trailing rows were dropped — clear extras inline. * - `diff`: differential repaint of visible rows / append new rows below. */ @@ -344,7 +336,7 @@ type RenderIntent = | { kind: "overlayRebuild" } | { kind: "viewportRepaint"; appendFrom?: number } | { kind: "deferredShrink"; paddedLength: number } - | { kind: "deferredMutation"; cursor?: DeferredCursorUpdate } + | { kind: "deferredMutation" } | { kind: "shrink" } | { kind: "diff"; firstChanged: number; lastChanged: number; appendedLines: boolean }; @@ -1397,7 +1389,6 @@ export class TUI extends Container { // 3. Classify intent. const intent = this.#planRender( lines, - cursorPos, widthChanged, heightChanged, prevViewportTop, @@ -1464,7 +1455,6 @@ 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( @@ -1493,46 +1483,6 @@ 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 @@ -1543,7 +1493,6 @@ export class TUI extends Container { */ #planRender( newLines: string[], - cursorPos: { row: number; col: number } | null, widthChanged: boolean, heightChanged: boolean, prevViewportTop: number, @@ -1645,7 +1594,7 @@ export class TUI extends Container { if (newLines.length <= paddedViewportTop) { return { kind: "deferredMutation" }; } - return this.#planDeferredShrink(newLines, height, paddedViewportTop, diff.firstChanged, cursorPos); + return { kind: "deferredShrink", paddedLength: this.#previousLines.length }; } // Non-ED3-risk POSIX with an unobservable viewport. If the shrink still @@ -1658,7 +1607,7 @@ export class TUI extends Container { return { kind: "historyRebuild" }; } this.#markNativeScrollbackDirty(); - return this.#planDeferredShrink(newLines, height, paddedViewportTop, diff.firstChanged, cursorPos); + return { kind: "deferredShrink", paddedLength: this.#previousLines.length }; } // 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 60f8518d8..dd166531d 100644 --- a/packages/tui/test/render-regressions.test.ts +++ b/packages/tui/test/render-regressions.test.ts @@ -2048,145 +2048,6 @@ 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