diff --git a/components/terminal/keywordHighlight.test.ts b/components/terminal/keywordHighlight.test.ts index 5417cdaa9..ed48dd875 100644 --- a/components/terminal/keywordHighlight.test.ts +++ b/components/terminal/keywordHighlight.test.ts @@ -1156,6 +1156,10 @@ test("user scroll during Enter keeps prior highlights mounted", async () => { getTranslatedLineIndexes().some((lineY) => lineY >= 10 && lineY < 20), "scrollback browsing during Enter should synchronously scan newly revealed lines", ); + assert.ok( + decorationStates.some((state) => !state.isDisposed && state.line >= 10 && state.line < 13), + "scrollback browsing during Enter should apply highlights on newly revealed lines", + ); raf.flush(); assert.equal( @@ -1417,7 +1421,9 @@ test("Enter dirty work continues after a queued full refresh", async () => { handlers.data?.("\r"); setLineText(22, "redrawn without a keyword"); handlers.writeParsed?.(); - await new Promise((resolve) => { setTimeout(resolve, 340); }); + // Idle Enter suppresses decoration mutation until the Enter guard clears + // (~600ms) so prompt redraw cannot flash still-visible keywords. + await new Promise((resolve) => { setTimeout(resolve, 850); }); raf.flush(); const originalDisposed = originalDecoration.isDisposed; @@ -1587,15 +1593,99 @@ test("continuous Enter output still refreshes highlights periodically", async () handlers.data?.("\r"); setLineText(22, "DEPLOY"); - for (let index = 0; index < 20; index += 1) { - handlers.writeParsed?.(); - raf.flush(); - await new Promise((resolve) => { setTimeout(resolve, 50); }); + const internals = highlighter as unknown as { + lastRefreshTime: number; + lastUserInputAt: number; + }; + // Sustained output must be a write burst, not spaced callbacks that look + // like a multi-batch prompt redraw. + internals.lastUserInputAt = Number.NEGATIVE_INFINITY; + const originalPerformance = globalThis.performance; + let simulatedNow = 0; + Object.defineProperty(globalThis, "performance", { + configurable: true, + value: { + now: () => simulatedNow, + }, + }); + try { + for (let index = 0; index < 12; index += 1) { + simulatedNow += 10; + internals.lastRefreshTime = Number.NEGATIVE_INFINITY; + handlers.writeParsed?.(); + raf.flush(); + } + } finally { + Object.defineProperty(globalThis, "performance", { + configurable: true, + value: originalPerformance, + }); } assert.ok( decorationStates.some(({ isDisposed }) => !isDisposed), - "ongoing output should not postpone new highlights until the stream stops", + "ongoing write-burst output should not postpone new highlights until the stream stops", + ); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("slow post-Enter output still applies highlights after the Enter guard", () => { + const raf = installAnimationFrameQueue(); + try { + const { term, decorationStates, handlers, setLineText } = createFakeTerminal("no match", { + lineCount: 40, + }); + term.buffer.active.viewportY = 20; + term.buffer.active.baseY = 20; + term.buffer.active.cursorY = 2; + const highlighter = new KeywordHighlighter(term as never); + highlighter.setRules([{ + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }], true); + raf.flush(); + assert.equal(decorationStates.filter(({ isDisposed }) => !isDisposed).length, 0); + + const internals = highlighter as unknown as { + lastRefreshTime: number; + lastUserInputAt: number; + }; + const originalPerformance = globalThis.performance; + let simulatedNow = 1_000; + Object.defineProperty(globalThis, "performance", { + configurable: true, + value: { + now: () => simulatedNow, + }, + }); + try { + handlers.data?.("\r"); + setLineText(22, "DEPLOY"); + internals.lastUserInputAt = Number.NEGATIVE_INFINITY; + // Intervals above WRITE_BURST_INTERVAL_MS never reach the burst + // threshold, and each write rearms the real idle timer. + for (let index = 0; index < 10; index += 1) { + simulatedNow += 80; + internals.lastRefreshTime = Number.NEGATIVE_INFINITY; + handlers.writeParsed?.(); + raf.flush(); + } + } finally { + Object.defineProperty(globalThis, "performance", { + configurable: true, + value: originalPerformance, + }); + } + + assert.ok( + decorationStates.some(({ isDisposed }) => !isDisposed), + "steady non-bursty output should highlight after the Enter window, not wait for a pause", ); highlighter.dispose(); } finally { @@ -1923,7 +2013,9 @@ test("Enter input still detects redraws away from the cursor", async () => { handlers.data?.("\r"); setLineText(20, "redrawn without a keyword"); handlers.writeParsed?.(); - await new Promise((resolve) => { setTimeout(resolve, 220); }); + // Idle Enter defers decoration dispose/apply until the Enter guard clears. + await new Promise((resolve) => { setTimeout(resolve, 850); }); + raf.flush(); assert.equal(originalDecoration.isDisposed, true); highlighter.dispose(); @@ -2245,6 +2337,301 @@ test("idle Enter scroll before writeParsed does not rescan visible keywords", () } }); +test("idle Enter scroll before buffer dims update does not rescan", () => { + const raf = installAnimationFrameQueue(); + try { + const { + term, + decorationStates, + handlers, + getTranslateCount, + resetTranslateCount, + refreshCalls, + resetRefreshCalls, + } = createFakeTerminal("hello DEPLOY world", { lineCount: 40 }); + term.buffer.active.viewportY = 20; + term.buffer.active.baseY = 20; + term.buffer.active.cursorY = 2; + const highlighter = new KeywordHighlighter(term as never); + highlighter.setRules([{ + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }], true); + raf.flush(); + const existingDecorations = [...decorationStates]; + assert.ok(existingDecorations.length > 0); + + const internals = highlighter as unknown as { + lastWriteAt: number; + lastRenderRange: { start: number; end: number } | null; + }; + internals.lastWriteAt = performance.now() - 10_000; + internals.lastRenderRange = null; + resetTranslateCount(); + resetRefreshCalls(); + + // Ubuntu RTT: onScroll can fire while length/baseY/cursor still match the + // last snapshot, so output-driven detection is false. Bottom-pinned Enter + // must still defer — requiring hasOutputDrivenViewportChange reopens flash. + handlers.data?.("\r"); + handlers.scroll?.(); + + assert.equal( + getTranslateCount(), + 0, + "Enter-pending bottom scroll without buffer-dim change must not rescan", + ); + assert.deepEqual( + refreshCalls, + [], + "Enter-pending bottom scroll without buffer-dim change must not repaint", + ); + assert.equal( + existingDecorations.filter(({ isDisposed }) => isDisposed).length, + 0, + "Enter-pending bottom scroll must keep existing keyword decorations mounted", + ); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("idle Enter prompt redraw does not repaint existing keyword rows", async () => { + const raf = installAnimationFrameQueue(); + try { + const { + term, + decorationStates, + handlers, + setLineText, + refreshCalls, + resetRefreshCalls, + } = createFakeTerminal("hello DEPLOY world", { lineCount: 40 }); + term.buffer.active.viewportY = 20; + term.buffer.active.baseY = 20; + term.buffer.active.cursorY = 2; + const highlighter = new KeywordHighlighter(term as never); + highlighter.setRules([ + { + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }, + { + id: "prompt", + label: "Prompt", + patterns: ["~", "#"], + color: "#60A5FA", + enabled: true, + }, + ], true); + raf.flush(); + const existingDecorations = [...decorationStates]; + assert.ok(existingDecorations.length > 0); + resetRefreshCalls(); + + handlers.data?.("\r"); + term.buffer.active.viewportY += 1; + term.buffer.active.baseY += 1; + term.buffer.active.length += 1; + // New prompt line matches custom ~/# rules — applying those decorations + // makes xterm repaint the full viewport and flashes still-visible keywords. + setLineText(22, "user@host:~# "); + handlers.scroll?.(); + handlers.writeParsed?.(); + await new Promise((resolve) => { setTimeout(resolve, 220); }); + raf.flush(); + + assert.equal( + existingDecorations.filter(({ isDisposed }) => isDisposed).length, + 0, + "idle Enter must keep prior keyword decorations mounted", + ); + assert.deepEqual( + refreshCalls, + [], + "idle Enter prompt redraw must not register decorations that force a viewport repaint", + ); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("idle Enter keeps suppression across split echo and prompt writes", async () => { + const raf = installAnimationFrameQueue(); + try { + const { + term, + decorationStates, + handlers, + setLineText, + refreshCalls, + resetRefreshCalls, + } = createFakeTerminal("hello DEPLOY world", { lineCount: 40 }); + term.buffer.active.viewportY = 20; + term.buffer.active.baseY = 20; + term.buffer.active.cursorY = 2; + const highlighter = new KeywordHighlighter(term as never); + highlighter.setRules([ + { + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }, + { + id: "prompt", + label: "Prompt", + patterns: ["~", "#"], + color: "#60A5FA", + enabled: true, + }, + ], true); + raf.flush(); + const existingDecorations = [...decorationStates]; + assert.ok(existingDecorations.length > 0); + resetRefreshCalls(); + + handlers.data?.("\r"); + // Batch 1: newline echo advances the buffer. + term.buffer.active.viewportY += 1; + term.buffer.active.baseY += 1; + term.buffer.active.length += 1; + handlers.scroll?.(); + handlers.writeParsed?.(); + await new Promise((resolve) => { setTimeout(resolve, 40); }); + raf.flush(); + + // Batch 2: prompt text for the same idle Enter (custom ~/# matches). + setLineText(22, "user@host:~# "); + handlers.writeParsed?.(); + await new Promise((resolve) => { setTimeout(resolve, 40); }); + raf.flush(); + + // Batch 3: trailing mode/control write — still the same prompt redraw, + // not sustained command output. + handlers.writeParsed?.(); + await new Promise((resolve) => { setTimeout(resolve, 220); }); + raf.flush(); + + assert.equal( + existingDecorations.filter(({ isDisposed }) => isDisposed).length, + 0, + "split idle-Enter writes must keep prior keyword decorations mounted", + ); + assert.deepEqual( + refreshCalls, + [], + "a three-batch idle prompt must not register decorations that flash the viewport", + ); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("pre-Enter write burst does not lift idle-Enter decoration suppression", () => { + const raf = installAnimationFrameQueue(); + try { + const { + term, + decorationStates, + handlers, + setLineText, + refreshCalls, + resetRefreshCalls, + } = createFakeTerminal("hello DEPLOY world", { lineCount: 40 }); + term.buffer.active.viewportY = 20; + term.buffer.active.baseY = 20; + term.buffer.active.cursorY = 2; + const highlighter = new KeywordHighlighter(term as never); + highlighter.setRules([ + { + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }, + { + id: "prompt", + label: "Prompt", + patterns: ["~", "#"], + color: "#60A5FA", + enabled: true, + }, + ], true); + raf.flush(); + const existingDecorations = [...decorationStates]; + assert.ok(existingDecorations.length > 0); + resetRefreshCalls(); + + const internals = highlighter as unknown as { + recentWriteBurst: number; + lastWriteAt: number; + lastBurstDecayAt: number; + }; + const originalPerformance = globalThis.performance; + let simulatedNow = 1_000; + Object.defineProperty(globalThis, "performance", { + configurable: true, + value: { + now: () => simulatedNow, + }, + }); + try { + internals.recentWriteBurst = 8; + internals.lastWriteAt = simulatedNow; + internals.lastBurstDecayAt = simulatedNow; + + handlers.data?.("\r"); + term.buffer.active.viewportY += 1; + term.buffer.active.baseY += 1; + term.buffer.active.length += 1; + handlers.scroll?.(); + simulatedNow += 5; + handlers.writeParsed?.(); + raf.flush(); + + setLineText(22, "user@host:~# "); + simulatedNow += 5; + handlers.writeParsed?.(); + raf.flush(); + + simulatedNow += 5; + handlers.writeParsed?.(); + raf.flush(); + } finally { + Object.defineProperty(globalThis, "performance", { + configurable: true, + value: originalPerformance, + }); + } + + assert.equal( + existingDecorations.filter(({ isDisposed }) => isDisposed).length, + 0, + "stale pre-Enter burst must not dispose still-visible keyword decorations", + ); + assert.deepEqual( + refreshCalls, + [], + "stale pre-Enter burst must not register prompt decorations that flash the viewport", + ); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + test("Enter without write clears pending so later user scroll can highlight", async () => { const raf = installAnimationFrameQueue(); try { diff --git a/components/terminal/keywordHighlight.ts b/components/terminal/keywordHighlight.ts index fe4584cdd..6047e7350 100644 --- a/components/terminal/keywordHighlight.ts +++ b/components/terminal/keywordHighlight.ts @@ -123,6 +123,18 @@ export class KeywordHighlighter implements IDisposable { private enterQueuedWriteCancellationPending = false; private enterViewportScanInProgress = false; private enterViewportScanNeedsRepeat = false; + /** True while idle-Enter output should not mutate decorations (xterm full-viewport repaint flash). */ + private enterSuppressDecorationMutation = false; + /** + * Whether onWriteParsed has already observed the current Enter submission. + * Prompt redraw can span several writeParsed batches (control sequences, + * prompt text, mode sets); callback count is not a sustained-output signal. + */ + private enterWriteParsedSeen = false; + /** performance.now() when the current Enter started suppressing mutations. */ + private enterSuppressionStartedAt = 0; + /** Viewport browsing state before the latest scroll handler ran. */ + private wasBrowsingScrollback = false; private static readonly DIRTY_SCAN_PADDING = XTERM_PERFORMANCE_CONFIG.highlighting.dirtyScanPadding; private static readonly INPUT_QUIET_MS = XTERM_PERFORMANCE_CONFIG.highlighting.inputQuietMs; private static readonly WRITE_BURST_INTERVAL_MS = 28; @@ -154,6 +166,17 @@ export class KeywordHighlighter implements IDisposable { if (data.includes("\r") || data.includes("\n")) { this.enterInputPending = true; this.enterQueuedWriteCancellationPending = true; + // First prompt redraw after Enter must not register/dispose decorations: + // xterm repaints the full viewport on decoration mutation and flashes + // still-visible keyword highlights (custom ~/# rules make this obvious). + this.enterSuppressDecorationMutation = true; + this.enterWriteParsedSeen = false; + this.enterSuppressionStartedAt = performance.now(); + // Pre-Enter burst must not lift mute on a split prompt. Keep + // lastWriteAt so output-driven scroll detection still works before + // the first post-Enter updateWriteBurst. + this.recentWriteBurst = 0; + this.lastBurstDecayAt = 0; // Time-bound Enter protection even when no echo/write arrives (echo // off, stalled PTY). onWriteParsed re-arms this on each write. this.scheduleEnterInputIdleClear(); @@ -224,11 +247,25 @@ export class KeywordHighlighter implements IDisposable { const inputProtectionActive = this.isInputProtectionActive(performance.now()); if (inputProtectionActive || this.enterInputPending) { if (this.enterInputPending) { - if (this.enterViewportScanInProgress) { + if (this.enterWriteParsedSeen) { this.updateWriteBurst(); - this.enterViewportScanNeedsRepeat = true; + // Multi-batch prompt redraw is still not command output. Keep + // decoration mute until a write burst or the Enter idle guard. + if (this.shouldLiftEnterDecorationSuppression()) { + this.enterSuppressDecorationMutation = false; + } + if (this.enterViewportScanInProgress) { + this.enterViewportScanNeedsRepeat = true; + } else { + const buffer = this.term.buffer.active; + this.addDirtyRange(buffer.viewportY, buffer.viewportY + this.term.rows - 1); + this.enterViewportScanInProgress = true; + } } else { + this.enterWriteParsedSeen = true; this.markDirtyFromWrite({ includeViewportProbe: false }); + // Index the viewport while muted so multi-frame Enter continuation + // can finish; decoration mutate stays suppressed for idle prompt. const buffer = this.term.buffer.active; this.addDirtyRange(buffer.viewportY, buffer.viewportY + this.term.rows - 1); this.enterViewportScanInProgress = true; @@ -266,6 +303,7 @@ export class KeywordHighlighter implements IDisposable { }) ); this.lastBufferSnapshot = this.readBufferSnapshot(); + this.wasBrowsingScrollback = this.isBrowsingScrollback(); } public setRules(rules: readonly RuntimeKeywordHighlightRule[], enabled: boolean) { @@ -430,6 +468,9 @@ export class KeywordHighlighter implements IDisposable { this.enterQueuedWriteCancellationPending = false; this.enterViewportScanInProgress = false; this.enterViewportScanNeedsRepeat = false; + this.enterSuppressDecorationMutation = false; + this.enterWriteParsedSeen = false; + this.enterSuppressionStartedAt = 0; if (hadDecorations) { this.term.refresh(0, this.term.rows - 1); } @@ -715,16 +756,23 @@ export class KeywordHighlighter implements IDisposable { private triggerViewportChangeRefresh() { const isBrowsingScrollback = this.isBrowsingScrollback(); + // End (or other jump-to-bottom) while Enter is pending: we were browsing + // scrollback and just landed on the bottom without waiting for echo. + // Distinguish that from idle-Enter echo still pinned at the bottom. + const returningToBottomFromScrollback = + this.wasBrowsingScrollback && !isBrowsingScrollback; + this.wasBrowsingScrollback = isBrowsingScrollback; // Enter echo often emits onScroll before onWriteParsed. After an idle gap // lastWriteAt looks stale and lastRenderRange is usually null (cleared by // the previous write refresh), so the output-driven scroll path would // synchronously rescan the viewport and flash keywords still on screen. - // Keep real scrollback browsing synchronous; only defer the bottom-pinned - // viewport movement that can be caused by the pending Enter echo. + // Keep real scrollback browsing synchronous; defer bottom-pinned Enter + // echo even when buffer dims have not updated yet (Ubuntu RTT). Do not + // require hasOutputDrivenViewportChange — that hole reopened the flash. if ( this.enterInputPending && !isBrowsingScrollback - && this.hasOutputDrivenViewportChange() + && !returningToBottomFromScrollback ) { if (this.pendingRefreshReason === "scroll") { this.cancelQueuedRefreshSchedule(); @@ -1086,6 +1134,9 @@ export class KeywordHighlighter implements IDisposable { this.enterInputIdleTimer = setTimeout(() => { this.enterInputIdleTimer = null; this.enterInputPending = false; + this.enterSuppressDecorationMutation = false; + this.enterWriteParsedSeen = false; + this.enterSuppressionStartedAt = 0; // Catch up any viewport motion deferred while Enter protection blocked // scroll refresh (e.g. user scrolled during the post-Enter window). this.markVisibleRangeDirty(); @@ -1093,6 +1144,23 @@ export class KeywordHighlighter implements IDisposable { }, KeywordHighlighter.ENTER_INPUT_GUARD_MS); } + /** + * True when follow-up Enter writes look like sustained command output. + * WriteParsed callback count alone cannot prove that — a split prompt may + * need three (or more) batches — so a write burst lifts mute early. + * Slow streams never burst and keep rearming the idle timer; after the + * Enter window, prompt redraw is done and mutation is safe. + */ + private shouldLiftEnterDecorationSuppression(): boolean { + if (!this.enterSuppressDecorationMutation) return false; + const now = performance.now(); + if (this.isWriteBurstActive(now)) return true; + return ( + this.enterSuppressionStartedAt > 0 + && now - this.enterSuppressionStartedAt >= KeywordHighlighter.ENTER_INPUT_GUARD_MS + ); + } + private isBrowsingScrollback(): boolean { const buffer = this.term.buffer.active; return buffer.viewportY < buffer.baseY; @@ -1516,16 +1584,25 @@ export class KeywordHighlighter implements IDisposable { if (end < start) return; const buffer = this.term.buffer.active; const pressure = getTerminalOutputPressure(this.term); + // Idle Enter must not mutate prompt-area decorations (xterm full-viewport + // flash). A real scrollback browse still needs new decorations: the scroll + // path records lastRenderRange even when this guard skips apply. + const allowEnterBrowseDecorations = this.enterSuppressDecorationMutation + && this.isBrowsingScrollback(); for (let lineY = start; lineY <= end; lineY++) { const line = buffer.getLine(lineY); if (!line) { - this.disposeLineDecorations(lineY); + if (!this.enterSuppressDecorationMutation) { + this.disposeLineDecorations(lineY); + } continue; } const lineText = line.translateToString(true); // true = trim right whitespace if (!lineText) { - this.disposeLineDecorations(lineY); + if (!this.enterSuppressDecorationMutation) { + this.disposeLineDecorations(lineY); + } continue; } @@ -1536,7 +1613,9 @@ export class KeywordHighlighter implements IDisposable { : this.scanWrappedLine(buffer, lineY, line, lineText, wrappedBlockCache) : this.getCachedRanges(line, lineText); if (cachedRanges.length === 0) { - this.disposeLineDecorations(lineY); + if (!this.enterSuppressDecorationMutation) { + this.disposeLineDecorations(lineY); + } continue; } @@ -1553,6 +1632,18 @@ export class KeywordHighlighter implements IDisposable { continue; } + // Idle Enter: keep prior decorations mounted. Scrollback browse still + // creates decorations on unindexed lines so matches are visible before + // the 600ms guard; lines that already have decorations stay untouched. + if (this.enterSuppressDecorationMutation) { + if (!allowEnterBrowseDecorations) { + continue; + } + if (existing && existing.decorations.length > 0) { + continue; + } + } + this.disposeLineDecorations(lineY, existing); this.applyLineDecorations(lineY, cachedRanges, signature, cursorAbsoluteY); }