fix(tui): eliminate rapidly blinking cursor during task execution
Refactored cursor positioning in TUI to consolidate cursor control sequences into the synchronized output buffer, preventing the rapid blink artifact that appeared on the right side of the terminal. - Extract #cursorControlSequence() from #positionHardwareCursor - Rename #positionHardwareCursor to #writeCursorPosition for clarity - Emit cursor control inside the synchronized output buffer rather than as a separate terminal write, eliminating the visible blink - Track #hardwareCursorRow state at write time instead of separately Closes #992
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
+51
-31
@@ -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`);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user