diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 7a45ce4c7..84d8f3b76 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -1,6 +1,10 @@ # Changelog + ## [Unreleased] +### Fixed + +- Fixed rapidly blinking cursor artifact during task execution by consolidating cursor control sequences into the synchronized output buffer ([#992](https://github.com/can1357/oh-my-pi/issues/992)) ## [14.5.7] - 2026-04-29 diff --git a/packages/tui/src/tui.ts b/packages/tui/src/tui.ts index a39500c5b..abf13485d 100644 --- a/packages/tui/src/tui.ts +++ b/packages/tui/src/tui.ts @@ -1018,10 +1018,12 @@ export class TUI extends Container { const line = newLines[i]; buffer += TERMINAL.isImageLine(line) ? line : line + reset; } + this.#cursorRow = Math.max(0, newLines.length - 1); + const { seq, toRow } = this.#cursorControlSequence(cursorPos, newLines.length, this.#cursorRow); + this.#hardwareCursorRow = toRow; + buffer += seq; buffer += "\x1b[?2026l"; // End synchronized output this.terminal.write(buffer); - this.#cursorRow = Math.max(0, newLines.length - 1); - this.#hardwareCursorRow = this.#cursorRow; // Reset max lines when clearing, otherwise track growth if (clear) { this.#maxLinesRendered = newLines.length; @@ -1029,7 +1031,6 @@ export class TUI extends Container { this.#maxLinesRendered = Math.max(this.#maxLinesRendered, newLines.length); } this.#viewportTopRow = Math.max(0, this.#maxLinesRendered - height); - this.#positionHardwareCursor(cursorPos, newLines.length); this.#previousLines = newLines; this.#previousWidth = width; this.#previousHeight = height; @@ -1101,7 +1102,7 @@ export class TUI extends Container { // No changes - but still need to update hardware cursor position if it moved if (firstChanged === -1) { - this.#positionHardwareCursor(cursorPos, newLines.length); + this.#writeCursorPosition(cursorPos, newLines.length); this.#viewportTopRow = Math.max(0, this.#maxLinesRendered - height); return; } @@ -1135,12 +1136,13 @@ export class TUI extends Container { if (moveUp > 0) { buffer += `\x1b[${moveUp}A`; } + this.#cursorRow = targetRow; + const { seq, toRow } = this.#cursorControlSequence(cursorPos, newLines.length, targetRow); + this.#hardwareCursorRow = toRow; + buffer += seq; buffer += "\x1b[?2026l"; this.terminal.write(buffer); - this.#cursorRow = targetRow; - this.#hardwareCursorRow = targetRow; } - this.#positionHardwareCursor(cursorPos, newLines.length); this.#previousLines = newLines; this.#previousWidth = width; this.#previousHeight = height; @@ -1166,7 +1168,7 @@ export class TUI extends Container { this.#cursorRow = Math.max(0, newLines.length - 1); this.#maxLinesRendered = newLines.length; this.#viewportTopRow = Math.max(0, newLines.length - height); - this.#positionHardwareCursor(cursorPos, newLines.length); + this.#writeCursorPosition(cursorPos, newLines.length); this.#previousLines = newLines; this.#previousWidth = width; this.#previousHeight = height; @@ -1249,6 +1251,9 @@ export class TUI extends Container { buffer += `\x1b[${extraLines}A`; } + const { seq, toRow } = this.#cursorControlSequence(cursorPos, newLines.length, finalCursorRow); + this.#hardwareCursorRow = toRow; + buffer += seq; buffer += "\x1b[?2026l"; // End synchronized output if ($flag("PI_TUI_DEBUG")) { @@ -1262,6 +1267,7 @@ export class TUI extends Container { `height: ${height}`, `lineDiff: ${lineDiff}`, `hardwareCursorRow: ${hardwareCursorRow}`, + `hardwareCursorRow (post): ${this.#hardwareCursorRow}`, `renderEnd: ${renderEnd}`, `finalCursorRow: ${finalCursorRow}`, `cursorPos: ${JSON.stringify(cursorPos)}`, @@ -1283,51 +1289,65 @@ export class TUI extends Container { // Write entire buffer at once this.terminal.write(buffer); - // Track cursor position for next render - // cursorRow tracks end of content (for viewport calculation) - // hardwareCursorRow tracks actual terminal cursor position (for movement) + // Track cursor position for next render. + // cursorRow tracks end of content (for viewport calculation). + // #hardwareCursorRow was already updated by #cursorControlSequence above. this.#cursorRow = Math.max(0, newLines.length - 1); - this.#hardwareCursorRow = finalCursorRow; // Track content height for viewport calculation this.#maxLinesRendered = newLines.length; this.#viewportTopRow = Math.max(0, newLines.length - height); - // Position hardware cursor for IME - this.#positionHardwareCursor(cursorPos, newLines.length); - this.#previousLines = newLines; this.#previousWidth = width; this.#previousHeight = height; } /** - * Position the hardware cursor for IME candidate window. - * @param cursorPos The cursor position extracted from rendered output, or null - * @param totalLines Total number of rendered lines + * Build cursor control sequences to position the hardware cursor for the IME + * candidate window. Returns escape sequences and the resulting cursor row for + * the caller to update `#hardwareCursorRow`. The sequences should be appended + * into the caller's own synchronized output block to avoid a flicker between + * content and cursor frames. */ - #positionHardwareCursor(cursorPos: { row: number; col: number } | null, totalLines: number): void { - if (!cursorPos || totalLines <= 0) { - this.terminal.hideCursor(); - return; - } + #cursorControlSequence( + cursorPos: { row: number; col: number } | null, + totalLines: number, + fromRow: number, + ): { seq: string; toRow: number } { + // No IME target or no content — hide cursor regardless of preference + if (!cursorPos || totalLines <= 0) return { seq: "\x1b[?25l", toRow: fromRow }; // Clamp cursor position to valid range const targetRow = Math.max(0, Math.min(cursorPos.row, totalLines - 1)); const targetCol = Math.max(0, cursorPos.col); // Move cursor from current position to target - const rowDelta = targetRow - this.#hardwareCursorRow; - let buffer = ""; + const rowDelta = targetRow - fromRow; + let seq = ""; if (rowDelta > 0) { - buffer += `\x1b[${rowDelta}B`; // Move down + seq += `\x1b[${rowDelta}B`; // Move down } else if (rowDelta < 0) { - buffer += `\x1b[${-rowDelta}A`; // Move up + seq += `\x1b[${-rowDelta}A`; // Move up } // Move to absolute column (1-indexed) - buffer += `\x1b[${targetCol + 1}G`; - buffer += this.#showHardwareCursor ? "\x1b[?25h" : "\x1b[?25l"; + seq += `\x1b[${targetCol + 1}G`; + seq += this.#showHardwareCursor ? "\x1b[?25h" : "\x1b[?25l"; - this.terminal.write(`\x1b[?2026h${buffer}\x1b[?2026l`); - this.#hardwareCursorRow = targetRow; + return { seq, toRow: targetRow }; + } + + /** + * Write the hardware cursor position to the terminal as a standalone + * synchronized output block. Use when there is no surrounding render buffer + * to embed the sequences into. + */ + #writeCursorPosition(cursorPos: { row: number; col: number } | null, totalLines: number): void { + if (!cursorPos || totalLines <= 0) { + this.terminal.hideCursor(); + return; + } + const { seq, toRow } = this.#cursorControlSequence(cursorPos, totalLines, this.#hardwareCursorRow); + this.#hardwareCursorRow = toRow; + this.terminal.write(`\x1b[?2026h${seq}\x1b[?2026l`); } } diff --git a/packages/tui/test/render-regressions.test.ts b/packages/tui/test/render-regressions.test.ts index 264aed4fe..9dd4da21e 100644 --- a/packages/tui/test/render-regressions.test.ts +++ b/packages/tui/test/render-regressions.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it } from "bun:test"; +import { afterEach, describe, expect, it, vi } from "bun:test"; import { type Component, TUI } from "@oh-my-pi/pi-tui"; import { VirtualTerminal } from "./virtual-terminal"; @@ -814,4 +814,146 @@ describe("TUI terminal-state regressions", () => { } }); }); + describe("cursor escape sequences stay inside synchronized output blocks", () => { + // Cursor placement sequences that must not leak outside \x1b[?2026h…\x1b[?2026l + const CURSOR_SEQ = /\x1b\[\?(?:25[hl]|\d+[A-G])/g; + const BSU = "\x1b[?2026h"; + const ESU = "\x1b[?2026l"; + + function getWrites(term: VirtualTerminal): string[] { + const writes: string[] = []; + const spy = vi.spyOn(term, "write"); + spy.mockImplementation((data: string) => { + writes.push(data); + }); + return writes; + } + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("all cursor sequences fall inside BSU/ESU brackets on full render", async () => { + const term = new VirtualTerminal(40, 10); + const tui = new TUI(term); + const writes = getWrites(term); + + const component = new MutableLinesComponent(["hello", "world"]); + tui.addChild(component); + + try { + tui.start(); + await settle(term); + assertCursorSequencesInsideSyncBlocks(writes); + } finally { + tui.stop(); + } + }); + + it("all cursor sequences fall inside BSU/ESU brackets on differential render", async () => { + const term = new VirtualTerminal(40, 10); + const tui = new TUI(term); + + const component = new MutableLinesComponent(["AAA", "BBB", "CCC"]); + tui.addChild(component); + + try { + tui.start(); + await settle(term); + + const writes = getWrites(term); + component.setLines(["AAA", "XXX", "CCC"]); + tui.requestRender(); + await settle(term); + assertCursorSequencesInsideSyncBlocks(writes); + } finally { + tui.stop(); + } + }); + + it("all cursor sequences fall inside BSU/ESU brackets on deleted-lines render", async () => { + const term = new VirtualTerminal(40, 10); + const tui = new TUI(term); + tui.setClearOnShrink(true); + + const component = new MutableLinesComponent(["A", "B", "C", "D"]); + tui.addChild(component); + + try { + tui.start(); + await settle(term); + + const writes = getWrites(term); + component.setLines(["A", "B"]); + tui.requestRender(); + await settle(term); + assertCursorSequencesInsideSyncBlocks(writes); + } finally { + tui.stop(); + } + }); + + it("all cursor sequences fall inside BSU/ESU brackets on repeated no-op renders", async () => { + const term = new VirtualTerminal(40, 10); + const tui = new TUI(term); + + const component = new MutableLinesComponent(["hello", "world", "stable"]); + tui.addChild(component); + + try { + tui.start(); + await settle(term); + + const writes = getWrites(term); + for (let i = 0; i < 4; i++) { + tui.requestRender(); + await settle(term); + } + assertCursorSequencesInsideSyncBlocks(writes); + } finally { + tui.stop(); + } + }); + + /** + * Assert that every cursor escape sequence in every write call appears + * strictly between a matched BSU/ESU pair, or is the sole payload of a + * standalone hideCursor call (from a no-change path). + */ + function assertCursorSequencesInsideSyncBlocks(writes: string[]): void { + for (const write of writes) { + if (write === "\x1b[?25l") { + // Standalone hideCursor — allowed (no-change path) + continue; + } + // Walk through the write, tracking BSU/ESU nesting + let depth = 0; + let idx = 0; + while (idx < write.length) { + CURSOR_SEQ.lastIndex = idx; + const match = CURSOR_SEQ.exec(write); + if (!match) break; + + const matchIdx = match.index; + // Count BSU/ESU depth up to the match position + let scanIdx = idx; + while (scanIdx < matchIdx) { + if (write.startsWith(BSU, scanIdx)) { + depth++; + scanIdx += BSU.length; + } else if (write.startsWith(ESU, scanIdx)) { + depth--; + scanIdx += ESU.length; + } else { + scanIdx++; + } + } + + expect(depth).toBeGreaterThan(0); + + idx = matchIdx + match[0].length; + } + } + } + }); });