From 7735c88e450fa766a83c1ac1ce359e8feeabb1fd Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 7 Aug 2026 09:05:48 +0000 Subject: [PATCH 1/5] fix(terminal): stop keyword highlight flicker on Enter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Idle Enter was misclassified as user scroll so overscan decorations were pruned before writeParsed cancel (Ubuntu RTT). Also swap cursor-line decorations atomically to avoid clear-before-create flash (fixes #2784). Co-authored-by: 陈大猫 --- components/terminal/keywordHighlight.test.ts | 87 +++++++++++++++++++ components/terminal/keywordHighlight.ts | 18 ++++ .../runtime/cursorLineHighlight.test.ts | 24 +++++ .../terminal/runtime/cursorLineHighlight.ts | 43 +++++++-- 4 files changed, 165 insertions(+), 7 deletions(-) diff --git a/components/terminal/keywordHighlight.test.ts b/components/terminal/keywordHighlight.test.ts index b244d1793f..a5da4e2474 100644 --- a/components/terminal/keywordHighlight.test.ts +++ b/components/terminal/keywordHighlight.test.ts @@ -1120,6 +1120,93 @@ test("pressing Enter keeps unchanged keyword decorations mounted", async () => { } }); +test("Enter-driven scroll before writeParsed does not prune keyword decorations", async () => { + const raf = installAnimationFrameQueue(); + try { + const { term, decorationStates, markers, handlers } = 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); + + // Idle long enough that Enter echo scroll is no longer classified as a + // recent write burst. xterm emits onScroll during the write, before + // onWriteParsed — that must not take the user-scroll prune path. + const internals = highlighter as unknown as { lastWriteAt: number }; + internals.lastWriteAt = performance.now() - 10_000; + + handlers.data?.("\r"); + for (const marker of markers) marker.line += 1; + term.buffer.active.viewportY += 1; + term.buffer.active.baseY += 1; + term.buffer.active.length += 1; + handlers.scroll?.(); + await new Promise((resolve) => { setTimeout(resolve, 150); }); + + assert.equal( + existingDecorations.filter(({ isDisposed }) => isDisposed).length, + 0, + "Enter-driven scroll before writeParsed should not dispose keyword decorations", + ); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("Enter cancels a queued user-scroll prune before it can flash keywords", async () => { + const raf = installAnimationFrameQueue(); + try { + const { term, decorationStates, handlers } = 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 }; + internals.lastWriteAt = performance.now() - 10_000; + + // Arm a user-scroll refresh, then press Enter before the debounce fires. + term.buffer.active.viewportY += 1; + handlers.scroll?.(); + handlers.data?.("\r"); + await new Promise((resolve) => { setTimeout(resolve, 150); }); + + assert.equal( + existingDecorations.filter(({ isDisposed }) => isDisposed).length, + 0, + "pressing Enter should cancel a pending scroll prune of keyword decorations", + ); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + test("pasted Enter remains protected when more input arrives before output", async () => { const raf = installAnimationFrameQueue(); try { diff --git a/components/terminal/keywordHighlight.ts b/components/terminal/keywordHighlight.ts index 35cdc3c2fb..cdcc8da3f1 100644 --- a/components/terminal/keywordHighlight.ts +++ b/components/terminal/keywordHighlight.ts @@ -164,6 +164,13 @@ export class KeywordHighlighter implements IDisposable { clearTimeout(this.enterInputIdleTimer); this.enterInputIdleTimer = null; } + // Drop any pending user-scroll refresh so Enter echo cannot finish a + // scroll pass that prunes still-visible keyword decorations. + this.cancelScrollRefresh(); + if (this.pendingRefreshReason === "scroll") { + this.cancelQueuedRefreshSchedule(); + this.pendingRefreshReason = "write"; + } } }), // When new data is written, refresh on the next frame so highlights land @@ -686,6 +693,17 @@ export class KeywordHighlighter implements IDisposable { private triggerViewportChangeRefresh() { this.cancelScrollRefresh(); + // Enter echo often emits onScroll before onWriteParsed. After an idle gap + // lastWriteAt looks stale, so the user-scroll path would prune overscan + // decorations and flash keywords still on screen. While Enter output is + // pending, let onWriteParsed own refresh scheduling. + if (this.enterInputPending) { + if (this.pendingRefreshReason === "scroll") { + this.cancelQueuedRefreshSchedule(); + this.pendingRefreshReason = "write"; + } + return; + } const now = performance.now(); const isOutputDrivenViewportChange = this.lastWriteAt > 0 && diff --git a/components/terminal/runtime/cursorLineHighlight.test.ts b/components/terminal/runtime/cursorLineHighlight.test.ts index d98e8b0354..c4eeb584fa 100644 --- a/components/terminal/runtime/cursorLineHighlight.test.ts +++ b/components/terminal/runtime/cursorLineHighlight.test.ts @@ -387,6 +387,30 @@ test('CursorLineHighlighter follows cursor moves and clears when disabled', () = highlighter.dispose(); }); +test('CursorLineHighlighter swaps decorations atomically on refresh', () => { + const term = createFakeTerm(80); + const highlighter = new CursorLineHighlighter(term as never); + highlighter.setEnabled(true); + const firstDecoration = term.decorations[0]; + assert.ok(firstDecoration); + assert.equal(firstDecoration.disposed, false); + + let sawOverlap = false; + const originalRegisterMarker = term.registerMarker.bind(term); + term.registerMarker = (offset: number) => { + // New marker must be created while the previous decoration is still live. + if (!firstDecoration.disposed) sawOverlap = true; + return originalRegisterMarker(offset); + }; + + term.moveCursor(2); + + assert.equal(sawOverlap, true, 'new marker should register before old decoration disposal'); + assert.equal(firstDecoration.disposed, true); + assert.equal(term.decorations.at(-1)?.disposed, false); + highlighter.dispose(); +}); + test('CursorLineHighlighter recreates on resize and overlay color changes', () => { const term = createFakeTerm(40); const highlighter = new CursorLineHighlighter(term as never); diff --git a/components/terminal/runtime/cursorLineHighlight.ts b/components/terminal/runtime/cursorLineHighlight.ts index 191e208287..1fdf2bd4a9 100644 --- a/components/terminal/runtime/cursorLineHighlight.ts +++ b/components/terminal/runtime/cursorLineHighlight.ts @@ -118,10 +118,21 @@ export class CursorLineHighlighter implements IDisposable { return; } - this.clear(); + // Register the next marker/decorations before disposing the previous set so + // Enter / cursor moves never leave an empty frame (clear-then-create flash). + const previousMarker = this.marker; + const previousDecorations = this.decorations; + const previousDisposeListeners = this.decorationDisposeListeners; const marker = this.term.registerMarker(0); - if (!marker) return; + if (!marker) { + this.clearOwned( + previousMarker, + previousDecorations, + previousDisposeListeners, + ); + return; + } const decorations: IDecoration[] = []; for (const range of ranges) { @@ -164,6 +175,12 @@ export class CursorLineHighlighter implements IDisposable { this.activeColor = color; this.activeRanges = ranges; this.activeTailRanges = tailRanges; + + this.clearOwned( + previousMarker, + previousDecorations, + previousDisposeListeners, + ); } dispose(): void { @@ -221,12 +238,14 @@ export class CursorLineHighlighter implements IDisposable { } private clear(): void { - for (const disposable of this.decorationDisposeListeners) disposable.dispose(); - this.decorationDisposeListeners = []; - for (const decoration of this.decorations) decoration.dispose(); - this.decorations = []; - this.marker?.dispose(); + this.clearOwned( + this.marker, + this.decorations, + this.decorationDisposeListeners, + ); this.marker = null; + this.decorations = []; + this.decorationDisposeListeners = []; this.activeLine = null; this.activeCols = null; this.activeColor = null; @@ -234,6 +253,16 @@ export class CursorLineHighlighter implements IDisposable { this.activeTailRanges = []; } + private clearOwned( + marker: IMarker | null, + decorations: IDecoration[], + disposeListeners: IDisposable[], + ): void { + for (const disposable of disposeListeners) disposable.dispose(); + for (const decoration of decorations) decoration.dispose(); + marker?.dispose(); + } + private markPendingRefresh(): void { if (!this.disposed && this.enabled) this.pendingRefresh = true; } From f3d319af440a57c159110b3cc8644d52c9c2cb9b Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 7 Aug 2026 09:16:02 +0000 Subject: [PATCH 2/5] fix(terminal): catch up highlights after Enter protection window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While enterInputPending blocked scroll-prune, viewport scrolls were also ignored with no dirty mark. Mark dirty during the window and trigger a write refresh when Enter protection clears. Co-authored-by: 陈大猫 --- components/terminal/keywordHighlight.test.ts | 49 ++++++++++++++++++++ components/terminal/keywordHighlight.ts | 8 +++- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/components/terminal/keywordHighlight.test.ts b/components/terminal/keywordHighlight.test.ts index a5da4e2474..6904b7470b 100644 --- a/components/terminal/keywordHighlight.test.ts +++ b/components/terminal/keywordHighlight.test.ts @@ -1166,6 +1166,55 @@ test("Enter-driven scroll before writeParsed does not prune keyword decorations" } }); +test("scroll during Enter protection marks viewport dirty for idle catch-up", async () => { + const raf = installAnimationFrameQueue(); + try { + const { term, handlers } = createFakeTerminal("hello DEPLOY world", { + lineCount: 40, + }); + term.buffer.active.viewportY = 20; + term.buffer.active.baseY = 20; + const highlighter = new KeywordHighlighter(term as never); + highlighter.setRules([{ + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }], true); + raf.flush(); + + const internals = highlighter as unknown as { + lastWriteAt: number; + enterInputPending: boolean; + dirtyAllInRenderRange: boolean; + pendingRefreshReason: string | null; + }; + internals.lastWriteAt = performance.now() - 10_000; + + handlers.data?.("\r"); + handlers.writeParsed?.(); + assert.equal(internals.enterInputPending, true); + + // User scrolls while Enter protection is still active — must not scroll-prune, + // but must dirty the viewport so idle-clear catch-up can rescan. + internals.dirtyAllInRenderRange = false; + term.buffer.active.viewportY += 5; + handlers.scroll?.(); + assert.equal( + internals.dirtyAllInRenderRange, + true, + "scroll during Enter protection should mark the viewport dirty for catch-up", + ); + + await new Promise((resolve) => { setTimeout(resolve, 700); }); + assert.equal(internals.enterInputPending, false); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + test("Enter cancels a queued user-scroll prune before it can flash keywords", async () => { const raf = installAnimationFrameQueue(); try { diff --git a/components/terminal/keywordHighlight.ts b/components/terminal/keywordHighlight.ts index cdcc8da3f1..fdc130a33a 100644 --- a/components/terminal/keywordHighlight.ts +++ b/components/terminal/keywordHighlight.ts @@ -696,12 +696,14 @@ export class KeywordHighlighter implements IDisposable { // Enter echo often emits onScroll before onWriteParsed. After an idle gap // lastWriteAt looks stale, so the user-scroll path would prune overscan // decorations and flash keywords still on screen. While Enter output is - // pending, let onWriteParsed own refresh scheduling. + // pending, skip scroll-prune but mark the viewport dirty so idle-clear / + // writeParsed can catch up (including user scroll during the protection window). if (this.enterInputPending) { if (this.pendingRefreshReason === "scroll") { this.cancelQueuedRefreshSchedule(); this.pendingRefreshReason = "write"; } + this.markVisibleRangeDirty(); return; } const now = performance.now(); @@ -1002,6 +1004,10 @@ export class KeywordHighlighter implements IDisposable { this.enterInputIdleTimer = setTimeout(() => { this.enterInputIdleTimer = null; this.enterInputPending = false; + // Catch up any viewport motion deferred while Enter protection blocked + // scroll-prune (e.g. user scrolled during the post-Enter window). + this.markVisibleRangeDirty(); + this.triggerRefresh("debounced", "write"); }, KeywordHighlighter.WRITE_PRUNE_IDLE_MS); } From 90ef805132a73fd2f2ed97c9905d3dead3c7ed50 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 7 Aug 2026 09:53:05 +0000 Subject: [PATCH 3/5] fix(terminal): persist keyword highlights across scroll (#2784) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace Enter scroll-skip band-aid with mam15mon/#2348 persistence: keep decorations across scroll, sync only newly exposed rows, bound retention (~20 viewports / max 1200). Keep Enter write-path protections and cursor-line atomic swap. Continuous Enter may scroll; prune-on-scroll is gone. Co-authored-by: mam15mon Co-authored-by: 陈大猫 --- components/terminal/keywordHighlight.test.ts | 635 +++++++++++++------ components/terminal/keywordHighlight.ts | 335 +++++----- infrastructure/config/xtermPerformance.ts | 7 +- 3 files changed, 648 insertions(+), 329 deletions(-) diff --git a/components/terminal/keywordHighlight.test.ts b/components/terminal/keywordHighlight.test.ts index 6904b7470b..1744c372f2 100644 --- a/components/terminal/keywordHighlight.test.ts +++ b/components/terminal/keywordHighlight.test.ts @@ -14,6 +14,7 @@ import { TERMINAL_AUX_LONG_LINE_SCAN_LIMIT_CHARS, TERMINAL_LONG_LINE_PRESSURE_BYTES, } from "./runtime/terminalFlowConstants.ts"; +import { XTERM_PERFORMANCE_CONFIG } from "../../infrastructure/config/xtermPerformance.ts"; type RafCallback = (time: number) => void; @@ -72,6 +73,36 @@ function createFakeLine(text: string, onTranslate?: () => void) { }; } +let nextFakeMarkerId = 1; +let fakeMarkerLineReadCount = 0; + +function createFakeMarker(line: number) { + const listeners = new Set<() => void>(); + let currentLine = line; + return { + id: nextFakeMarkerId++, + get line() { + fakeMarkerLineReadCount += 1; + return currentLine; + }, + set line(value: number) { + currentLine = value; + }, + isDisposed: false, + onDispose(listener: () => void) { + listeners.add(listener); + return { dispose: () => listeners.delete(listener) }; + }, + dispose() { + if (this.isDisposed) return; + this.isDisposed = true; + this.line = -1; + for (const listener of listeners) listener(); + listeners.clear(); + }, + }; +} + function createFakeWrappedLine(text: string, isWrapped: boolean) { return { ...createFakeLine(text), @@ -102,13 +133,7 @@ function createFakeTerminalFromLines(lines: Array<{ text: string; isWrapped: boo onResize: () => noopDisposable, onRender: () => noopDisposable, registerMarker(offset: number) { - return { - line: offset, - isDisposed: false, - dispose() { - this.isDisposed = true; - }, - }; + return createFakeMarker(offset); }, registerDecoration(options: { x: number; width: number; foregroundColor: string }) { decorations.push(options); @@ -162,8 +187,8 @@ function createFakeTerminalFromLargeWrappedBlock({ }, }, }, - onScroll(handler: () => void) { - handlers.scroll = handler; + onScroll(handler: (viewportY: number) => void) { + handlers.scroll = () => handler(term.buffer.active.viewportY); return noopDisposable; }, onData(handler: (data: string) => void) { @@ -183,13 +208,7 @@ function createFakeTerminalFromLargeWrappedBlock({ return noopDisposable; }, registerMarker(offset: number) { - return { - line: offset, - isDisposed: false, - dispose() { - this.isDisposed = true; - }, - }; + return createFakeMarker(offset); }, registerDecoration(options: { x: number; width: number; foregroundColor: string }) { decorations.push(options); @@ -258,8 +277,8 @@ function createFakeTerminal(lineText: string, options: { lineCount?: number } = getLine: (lineY: number) => lines[lineY], }, }, - onScroll: (handler: () => void) => { - handlers.scroll = handler; + onScroll: (handler: (viewportY: number) => void) => { + handlers.scroll = () => handler(term.buffer.active.viewportY); return noopDisposable; }, onData: (handler: (data: string) => void) => { @@ -279,13 +298,9 @@ function createFakeTerminal(lineText: string, options: { lineCount?: number } = return noopDisposable; }, registerMarker(offset: number) { - const marker = { - line: term.buffer.active.baseY + term.buffer.active.cursorY + offset, - isDisposed: false, - dispose() { - this.isDisposed = true; - }, - }; + const marker = createFakeMarker( + term.buffer.active.baseY + term.buffer.active.cursorY + offset, + ); markers.push(marker); return marker; }, @@ -322,6 +337,7 @@ function createFakeTerminal(lineText: string, options: { lineCount?: number } = handlers, getTranslateCount: () => translateCount, getTranslatedLineIndexes: () => [...translatedLineIndexes], + getActiveDecorationCount: () => decorationStates.filter((state) => !state.isDisposed).length, resetTranslateCount: () => { translateCount = 0; translatedLineIndexes.length = 0; @@ -377,17 +393,18 @@ test("marker reindexing moves keyword decorations to the current buffer line", ( raf.flush(); const internals = highlighter as unknown as { - lineDecorations: Map; - reindexLineDecorationsFromMarkers: () => void; + lineDecorations: Map; + syncLineDecorationIndex: (force?: boolean) => void; }; const state = internals.lineDecorations.get(0); assert.ok(state); state.marker.line = 1; - internals.reindexLineDecorationsFromMarkers(); + internals.syncLineDecorationIndex(true); assert.equal(internals.lineDecorations.has(0), false); assert.equal(internals.lineDecorations.get(1), state); + assert.equal(state.indexedLine, 1); highlighter.dispose(); } finally { raf.restore(); @@ -606,7 +623,7 @@ test("output-driven viewport changes defer keyword highlight scans", async () => } }); -test("user scroll defers keyword highlight scans and scans only visible rows", async () => { +test("user scroll reuses persistent prehighlighted lines without delayed scans", async () => { const raf = installAnimationFrameQueue(); try { const { term, handlers, getTranslateCount, resetTranslateCount } = createFakeTerminal("hello DEPLOY world", { @@ -635,14 +652,14 @@ test("user scroll defers keyword highlight scans and scans only visible rows", a assert.equal(getTranslateCount(), 0); await new Promise((resolve) => { setTimeout(resolve, 130); }); - assert.equal(getTranslateCount(), term.rows); + assert.equal(getTranslateCount(), 0); highlighter.dispose(); } finally { raf.restore(); } }); -test("continuous user scroll cancels stale keyword highlight continuation work", async () => { +test("distant user scroll synchronously highlights only the target viewport", async () => { const raf = installAnimationFrameQueue(); try { const { @@ -650,7 +667,7 @@ test("continuous user scroll cancels stale keyword highlight continuation work", handlers, getTranslatedLineIndexes, resetTranslateCount, - } = createFakeTerminal("hello DEPLOY world", { lineCount: 120 }); + } = createFakeTerminal("hello DEPLOY world", { lineCount: 220 }); term.rows = 30; const highlighter = new KeywordHighlighter(term as never); const rules: KeywordHighlightRule[] = [ @@ -667,23 +684,420 @@ test("continuous user scroll cancels stale keyword highlight continuation work", raf.flush(); resetTranslateCount(); - term.buffer.active.viewportY = 10; + term.buffer.active.baseY = 190; + term.buffer.active.viewportY = 120; handlers.scroll?.(); - await new Promise((resolve) => { setTimeout(resolve, 60); }); - term.buffer.active.viewportY = 60; - handlers.scroll?.(); + assert.deepEqual( + getTranslatedLineIndexes(), + Array.from({ length: term.rows }, (_, index) => 120 + index), + "the destination viewport should be indexed before the scroll event returns", + ); + await new Promise((resolve) => { setTimeout(resolve, 130); }); + assert.equal(getTranslatedLineIndexes().length, term.rows); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("user scrollback browsing stays synchronous during a write burst", () => { + const raf = installAnimationFrameQueue(); + try { + const { + term, + handlers, + getTranslatedLineIndexes, + resetTranslateCount, + } = createFakeTerminal("hello DEPLOY world", { lineCount: 220 }); + term.rows = 30; + const highlighter = new KeywordHighlighter(term as never); + const rules: KeywordHighlightRule[] = [ + { + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }, + ]; + + highlighter.setRules(rules, true); raf.flush(); + resetTranslateCount(); + + for (let index = 0; index < 6; index += 1) { + handlers.writeParsed?.(); + } + resetTranslateCount(); + term.buffer.active.baseY = 190; + term.buffer.active.viewportY = 120; + handlers.scroll?.(); assert.deepEqual( - getTranslatedLineIndexes().filter((lineY) => lineY > 17 && lineY < 60), - [], - "stale continuation from the first scroll should not keep scanning old viewport rows", + getTranslatedLineIndexes(), + Array.from({ length: term.rows }, (_, index) => 120 + index), ); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("continuous distant scrolls do not scan skipped viewport ranges", () => { + const raf = installAnimationFrameQueue(); + try { + const { + term, + handlers, + getTranslatedLineIndexes, + resetTranslateCount, + } = createFakeTerminal("hello DEPLOY world", { lineCount: 260 }); + term.rows = 30; + const highlighter = new KeywordHighlighter(term as never); + const rules: KeywordHighlightRule[] = [ + { + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }, + ]; + + highlighter.setRules(rules, true); + raf.flush(); + resetTranslateCount(); + + term.buffer.active.baseY = 230; + term.buffer.active.viewportY = 120; + handlers.scroll?.(); + term.buffer.active.viewportY = 180; + handlers.scroll?.(); + + assert.deepEqual( + getTranslatedLineIndexes(), + [ + ...Array.from({ length: term.rows }, (_, index) => 120 + index), + ...Array.from({ length: term.rows }, (_, index) => 180 + index), + ], + ); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("persistent highlights remain until xterm disposes their markers", () => { + const raf = installAnimationFrameQueue(); + try { + const { + term, + handlers, + getActiveDecorationCount, + markers, + } = createFakeTerminal("hello DEPLOY world", { lineCount: 200 }); + term.rows = 3; + const highlighter = new KeywordHighlighter(term as never); + const rules: KeywordHighlightRule[] = [ + { + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }, + ]; + + highlighter.setRules(rules, true); + raf.flush(); + assert.equal(getActiveDecorationCount(), 9); + + term.buffer.active.baseY = 170; + term.buffer.active.viewportY = 120; + handlers.scroll?.(); + assert.equal(getActiveDecorationCount(), 12); + + markers[0].dispose(); + handlers.scroll?.(); + assert.equal(getActiveDecorationCount(), 11); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("persistent highlight lookup follows uniform scrollback marker shifts", () => { + const raf = installAnimationFrameQueue(); + try { + const { + term, + handlers, + getActiveDecorationCount, + getTranslateCount, + markers, + resetTranslateCount, + } = createFakeTerminal("hello DEPLOY world", { lineCount: 30 }); + term.rows = 3; + const highlighter = new KeywordHighlighter(term as never); + const rules: KeywordHighlightRule[] = [ + { + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }, + ]; + + highlighter.setRules(rules, true); + raf.flush(); + markers[0].dispose(); + for (const marker of markers) { + if (!marker.isDisposed) marker.line -= 1; + } + resetTranslateCount(); + + term.buffer.active.baseY = 27; + term.buffer.active.viewportY = 0; + handlers.scroll?.(); + + assert.equal(getTranslateCount(), term.rows); + assert.equal(getActiveDecorationCount(), 8); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("persistent marker index synchronization stays constant-time", () => { + const raf = installAnimationFrameQueue(); + try { + const { term, handlers } = createFakeTerminal("hello DEPLOY world", { lineCount: 400 }); + term.rows = 30; + const highlighter = new KeywordHighlighter(term as never); + const rules: KeywordHighlightRule[] = [ + { + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }, + ]; + + highlighter.setRules(rules, true); + raf.flush(); + term.buffer.active.baseY = 370; + for (const viewportY of [120, 180, 240, 300]) { + term.buffer.active.viewportY = viewportY; + handlers.scroll?.(); + } + + fakeMarkerLineReadCount = 0; + handlers.scroll?.(); + assert.ok( - getTranslatedLineIndexes().some((lineY) => lineY >= 60 && lineY < 90), - "latest viewport should be highlighted after scroll settles", + fakeMarkerLineReadCount < 10, + `expected constant marker reads, got ${fakeMarkerLineReadCount}`, + ); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("persistent highlights stay bounded for broad matching rules", () => { + const raf = installAnimationFrameQueue(); + try { + const { + term, + handlers, + getActiveDecorationCount, + } = createFakeTerminal("hello DEPLOY world", { lineCount: 2_000 }); + term.rows = 30; + const highlighter = new KeywordHighlighter(term as never); + const rules: KeywordHighlightRule[] = [ + { + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }, + ]; + + highlighter.setRules(rules, true); + raf.flush(); + term.buffer.active.baseY = 1_970; + for (let viewportY = 120; viewportY <= 1_800; viewportY += term.rows) { + term.buffer.active.viewportY = viewportY; + handlers.scroll?.(); + } + + const expectedLimit = Math.min( + XTERM_PERFORMANCE_CONFIG.highlighting.maxPersistentDecorationLines, + Math.max( + XTERM_PERFORMANCE_CONFIG.highlighting.minPersistentDecorationLines, + term.rows * XTERM_PERFORMANCE_CONFIG.highlighting.persistentDecorationViewports, + ), + ); + assert.equal(getActiveDecorationCount(), expectedLimit); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("external marker reset invalidates persistent highlight coverage", () => { + const raf = installAnimationFrameQueue(); + try { + const { + term, + handlers, + getActiveDecorationCount, + getTranslateCount, + markers, + resetTranslateCount, + } = createFakeTerminal("hello DEPLOY world", { lineCount: 9 }); + term.rows = 3; + const highlighter = new KeywordHighlighter(term as never); + const rules: KeywordHighlightRule[] = [ + { + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }, + ]; + + highlighter.setRules(rules, true); + raf.flush(); + for (const marker of [...markers]) marker.dispose(); + resetTranslateCount(); + + handlers.scroll?.(); + + assert.equal(getTranslateCount(), term.rows); + assert.equal(getActiveDecorationCount(), term.rows); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("in-place redraw removes a persistent highlight when text stops matching", async () => { + const raf = installAnimationFrameQueue(); + try { + const { + term, + handlers, + getActiveDecorationCount, + setLineText, + } = createFakeTerminal("hello DEPLOY world", { lineCount: 9 }); + term.rows = 3; + const highlighter = new KeywordHighlighter(term as never); + const rules: KeywordHighlightRule[] = [ + { + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }, + ]; + + highlighter.setRules(rules, true); + raf.flush(); + assert.equal(getActiveDecorationCount(), 9); + + setLineText(1, "hello SAFE world 1"); + handlers.writeParsed?.(); + await new Promise((resolve) => { setTimeout(resolve, 120); }); + raf.flush(); + + assert.equal(getActiveDecorationCount(), 8); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("Enter-driven scroll does not dispose nearby keyword decorations", async () => { + const raf = installAnimationFrameQueue(); + try { + const { term, decorationStates, handlers } = 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); + + // Idle Enter: onScroll before onWriteParsed. Persistence keeps nearby + // highlights mounted instead of pruning them as user-scroll leftovers. + handlers.data?.("\r"); + 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, 220); }); + + assert.equal( + existingDecorations.filter(({ isDisposed }) => isDisposed).length, + 0, + "Enter-driven scroll should not dispose keyword decorations still on screen", + ); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + +test("user scroll during Enter keeps prior highlights mounted", async () => { + const raf = installAnimationFrameQueue(); + try { + const { term, decorationStates, handlers } = createFakeTerminal("hello DEPLOY world", { + lineCount: 80, + }); + term.rows = 3; + 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); + + handlers.data?.("\r"); + handlers.writeParsed?.(); + term.buffer.active.viewportY = 10; + handlers.scroll?.(); + + assert.equal( + existingDecorations.filter(({ isDisposed }) => isDisposed).length, + 0, + "scroll during Enter should keep prior persistent highlights", ); highlighter.dispose(); } finally { @@ -1120,142 +1534,6 @@ test("pressing Enter keeps unchanged keyword decorations mounted", async () => { } }); -test("Enter-driven scroll before writeParsed does not prune keyword decorations", async () => { - const raf = installAnimationFrameQueue(); - try { - const { term, decorationStates, markers, handlers } = 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); - - // Idle long enough that Enter echo scroll is no longer classified as a - // recent write burst. xterm emits onScroll during the write, before - // onWriteParsed — that must not take the user-scroll prune path. - const internals = highlighter as unknown as { lastWriteAt: number }; - internals.lastWriteAt = performance.now() - 10_000; - - handlers.data?.("\r"); - for (const marker of markers) marker.line += 1; - term.buffer.active.viewportY += 1; - term.buffer.active.baseY += 1; - term.buffer.active.length += 1; - handlers.scroll?.(); - await new Promise((resolve) => { setTimeout(resolve, 150); }); - - assert.equal( - existingDecorations.filter(({ isDisposed }) => isDisposed).length, - 0, - "Enter-driven scroll before writeParsed should not dispose keyword decorations", - ); - highlighter.dispose(); - } finally { - raf.restore(); - } -}); - -test("scroll during Enter protection marks viewport dirty for idle catch-up", async () => { - const raf = installAnimationFrameQueue(); - try { - const { term, handlers } = createFakeTerminal("hello DEPLOY world", { - lineCount: 40, - }); - term.buffer.active.viewportY = 20; - term.buffer.active.baseY = 20; - const highlighter = new KeywordHighlighter(term as never); - highlighter.setRules([{ - id: "deploy", - label: "Deploy", - patterns: ["DEPLOY"], - color: "#F87171", - enabled: true, - }], true); - raf.flush(); - - const internals = highlighter as unknown as { - lastWriteAt: number; - enterInputPending: boolean; - dirtyAllInRenderRange: boolean; - pendingRefreshReason: string | null; - }; - internals.lastWriteAt = performance.now() - 10_000; - - handlers.data?.("\r"); - handlers.writeParsed?.(); - assert.equal(internals.enterInputPending, true); - - // User scrolls while Enter protection is still active — must not scroll-prune, - // but must dirty the viewport so idle-clear catch-up can rescan. - internals.dirtyAllInRenderRange = false; - term.buffer.active.viewportY += 5; - handlers.scroll?.(); - assert.equal( - internals.dirtyAllInRenderRange, - true, - "scroll during Enter protection should mark the viewport dirty for catch-up", - ); - - await new Promise((resolve) => { setTimeout(resolve, 700); }); - assert.equal(internals.enterInputPending, false); - highlighter.dispose(); - } finally { - raf.restore(); - } -}); - -test("Enter cancels a queued user-scroll prune before it can flash keywords", async () => { - const raf = installAnimationFrameQueue(); - try { - const { term, decorationStates, handlers } = 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 }; - internals.lastWriteAt = performance.now() - 10_000; - - // Arm a user-scroll refresh, then press Enter before the debounce fires. - term.buffer.active.viewportY += 1; - handlers.scroll?.(); - handlers.data?.("\r"); - await new Promise((resolve) => { setTimeout(resolve, 150); }); - - assert.equal( - existingDecorations.filter(({ isDisposed }) => isDisposed).length, - 0, - "pressing Enter should cancel a pending scroll prune of keyword decorations", - ); - highlighter.dispose(); - } finally { - raf.restore(); - } -}); - test("pasted Enter remains protected when more input arrives before output", async () => { const raf = installAnimationFrameQueue(); try { @@ -1876,7 +2154,7 @@ test("wrapped highlight scanning stops before walking an oversized soft-wrapped } }); -test("scroll refresh reuses wrapped scan misses across visible rows", async () => { +test("scroll refresh reuses wrapped scan misses across visible rows", () => { const raf = installAnimationFrameQueue(); try { const lineText = "a".repeat(80); @@ -1908,7 +2186,6 @@ test("scroll refresh reuses wrapped scan misses across visible rows", async () = term.buffer.active.viewportY = 29_500; handlers.scroll?.(); - await new Promise((resolve) => { setTimeout(resolve, 130); }); assert.ok( getLineCount() < 1_000, diff --git a/components/terminal/keywordHighlight.ts b/components/terminal/keywordHighlight.ts index fdc130a33a..608a781c90 100644 --- a/components/terminal/keywordHighlight.ts +++ b/components/terminal/keywordHighlight.ts @@ -45,6 +45,7 @@ interface LineDecorationState { marker: IMarker; decorations: IDecoration[]; signature: string; + indexedLine: number; } type RefreshReason = "scroll" | "write" | "full"; @@ -75,15 +76,6 @@ interface WrappedBlockScanCache { cappedMiss: DirtyLineSegment | null; } -interface ScrollRefreshJob { - generation: number; - start: number; - end: number; - nextLine: number; - cursorAbsoluteY: number; - wrappedBlockCache: WrappedBlockScanCache; -} - /** Shared empty array for non-matching lines to avoid per-call allocations. */ const EMPTY_RANGES: readonly CachedDecorationRange[] = Object.freeze([]); @@ -93,13 +85,16 @@ const RE_ASCII_ONLY = /^[\x00-\x7f]*$/; /** * Manages terminal decorations for keyword highlighting. - * Uses xterm.js Decoration API to overlay styles without modifying the data stream. - * This ensures zero impact on scrolling performance ("lazy" highlighting). + * Uses persistent xterm.js markers so nearby indexed lines keep decorations + * across scrollback navigation without modifying the terminal data stream. + * Retention is bounded to protect xterm's marker listeners for broad rules. */ export class KeywordHighlighter implements IDisposable { private term: XTerm; private compiledRules: CompiledRule[] = []; private lineDecorations = new Map(); + private markerLineOffset = 0; + private lineDecorationIndexNeedsRebuild = false; private debounceTimer: NodeJS.Timeout | null = null; /** Single quiet-window catch-up after bulk dumps (no per-write schedule). */ private bulkPressureCatchUpTimer: NodeJS.Timeout | null = null; @@ -128,10 +123,7 @@ export class KeywordHighlighter implements IDisposable { private enterQueuedWriteCancellationPending = false; private enterViewportScanInProgress = false; private enterViewportScanNeedsRepeat = false; - private scrollRefreshJob: ScrollRefreshJob | null = null; - private scrollRefreshGeneration = 0; private static readonly DIRTY_SCAN_PADDING = XTERM_PERFORMANCE_CONFIG.highlighting.dirtyScanPadding; - private static readonly SCROLL_SETTLE_DEBOUNCE_MS = XTERM_PERFORMANCE_CONFIG.highlighting.scrollSettleDebounceMs; private static readonly INPUT_QUIET_MS = XTERM_PERFORMANCE_CONFIG.highlighting.inputQuietMs; private static readonly WRITE_BURST_INTERVAL_MS = 28; private static readonly WRITE_BURST_DECAY_MS = 80; @@ -150,7 +142,8 @@ export class KeywordHighlighter implements IDisposable { // Hook into terminal events to trigger highlighting this.disposables.push( // When user scrolls, refresh visible area - this.term.onScroll(() => { + this.term.onScroll((viewportY) => { + this.lastViewportY = viewportY; this.triggerViewportChangeRefresh(); }), // User input should keep terminal echo responsive; highlight can catch up @@ -164,13 +157,6 @@ export class KeywordHighlighter implements IDisposable { clearTimeout(this.enterInputIdleTimer); this.enterInputIdleTimer = null; } - // Drop any pending user-scroll refresh so Enter echo cannot finish a - // scroll pass that prunes still-visible keyword decorations. - this.cancelScrollRefresh(); - if (this.pendingRefreshReason === "scroll") { - this.cancelQueuedRefreshSchedule(); - this.pendingRefreshReason = "write"; - } } }), // When new data is written, refresh on the next frame so highlights land @@ -188,17 +174,13 @@ export class KeywordHighlighter implements IDisposable { const cancelQueuedWriteForEnter = this.enterQueuedWriteCancellationPending && this.pendingRefreshReason === "write"; - if (this.enterInputPending || outputDrivenPendingScroll) { - this.cancelScrollRefresh(); - if ( - this.pendingRefreshReason === "scroll" - || cancelQueuedWriteForEnter - ) { - this.cancelQueuedRefreshSchedule(); - } - if (this.pendingRefreshReason === "scroll") { - this.pendingRefreshReason = "write"; - } + // Convert output-driven auto-scroll to write refresh. Do not cancel a + // real user scrollback browse just because Enter write-path is active. + if (outputDrivenPendingScroll && this.pendingRefreshReason === "scroll") { + this.cancelQueuedRefreshSchedule(); + this.pendingRefreshReason = "write"; + } else if (cancelQueuedWriteForEnter) { + this.cancelQueuedRefreshSchedule(); } this.enterQueuedWriteCancellationPending = false; const pressure = getTerminalOutputPressure(this.term); @@ -260,7 +242,11 @@ export class KeywordHighlighter implements IDisposable { ); }), // Also refresh on resize as viewport content changes - this.term.onResize(() => this.triggerRefresh("debounced", "full")), + this.term.onResize(() => { + this.syncLineDecorationIndex(true); + this.lastRenderRange = null; + this.triggerRefresh("debounced", "full"); + }), // onRender fires after each render cycle - catch scrolls that onScroll might miss this.term.onRender(() => { // Only trigger refresh if viewport position changed @@ -346,7 +332,6 @@ export class KeywordHighlighter implements IDisposable { } public dispose() { - this.cancelScrollRefresh(); this.clearDecorations(); this.disposables.forEach(d => d.dispose()); this.disposables = []; @@ -412,7 +397,6 @@ export class KeywordHighlighter implements IDisposable { // Re-check state: may have changed since the refresh was scheduled if (!this.enabled || this.compiledRules.length === 0) return; if (this.term.buffer.active.type === 'alternate') { - this.cancelScrollRefresh(); if (this.lineDecorations.size > 0) this.clearDecorations(); return; } @@ -424,12 +408,13 @@ export class KeywordHighlighter implements IDisposable { } private clearDecorations() { - this.cancelScrollRefresh(); const hadDecorations = this.lineDecorations.size > 0; for (const [lineY, state] of this.lineDecorations) { this.disposeLineDecorations(lineY, state); } this.lineDecorations.clear(); + this.markerLineOffset = 0; + this.lineDecorationIndexNeedsRebuild = false; this.lastViewportRange = null; this.lastRenderRange = null; this.clearDirtySegments(); @@ -443,7 +428,7 @@ export class KeywordHighlighter implements IDisposable { } private disposeLineDecorations(lineY: number, state?: LineDecorationState) { - const target = state ?? this.lineDecorations.get(lineY); + const target = state ?? this.getLineDecorationState(lineY); if (!target) return; const removedLineY = this.removeLineDecorationState(target, lineY); const markerLineBeforeDispose = target.marker.isDisposed ? -1 : target.marker.line; @@ -454,17 +439,26 @@ export class KeywordHighlighter implements IDisposable { } private removeLineDecorationState(target: LineDecorationState, lineHint?: number): number | null { + const indexedState = this.lineDecorations.get(target.indexedLine); + if (indexedState === target) { + this.lineDecorations.delete(target.indexedLine); + return target.marker.isDisposed + ? (lineHint ?? null) + : target.marker.line; + } if (lineHint != null) { - const hinted = this.lineDecorations.get(lineHint); + const hinted = this.lineDecorations.get(this.toIndexedLine(lineHint)); if (hinted === target) { - this.lineDecorations.delete(lineHint); + this.lineDecorations.delete(hinted.indexedLine); return lineHint; } } for (const [mappedLineY, mappedState] of this.lineDecorations) { if (mappedState === target) { this.lineDecorations.delete(mappedLineY); - return mappedLineY; + return target.marker.isDisposed + ? (lineHint ?? null) + : target.marker.line; } } return null; @@ -488,7 +482,7 @@ export class KeywordHighlighter implements IDisposable { const offset = lineY - cursorAbsoluteY; const marker = this.term.registerMarker(offset); if (!marker) { - this.lineDecorations.delete(lineY); + this.lineDecorations.delete(this.toIndexedLine(lineY)); return; } @@ -507,15 +501,25 @@ export class KeywordHighlighter implements IDisposable { if (decorations.length === 0) { marker.dispose(); - this.lineDecorations.delete(lineY); + this.lineDecorations.delete(this.toIndexedLine(lineY)); return; } - this.lineDecorations.set(lineY, { + const state: LineDecorationState = { marker, decorations, signature, + indexedLine: this.toIndexedLine(lineY), + }; + marker.onDispose(() => { + this.removeLineDecorationState(state, lineY); + this.lastRenderRange = null; + for (const decoration of decorations) { + if (!decoration.isDisposed) decoration.dispose(); + } }); + this.lineDecorations.set(state.indexedLine, state); + this.prunePersistentDecorations(); this.markTerminalRefreshNeeded(lineY); } @@ -564,9 +568,6 @@ export class KeywordHighlighter implements IDisposable { private triggerRefresh(mode: "immediate" | "debounced" | "continuation", reason: RefreshReason = "full") { if (!this.enabled || this.compiledRules.length === 0) return; - if (reason !== "scroll") { - this.cancelScrollRefresh(); - } this.pendingRefreshReason = this.mergeRefreshReason(this.pendingRefreshReason, reason); // Optimization: Disable highlighting in Alternate Buffer (e.g. Vim, Htop) @@ -598,6 +599,22 @@ export class KeywordHighlighter implements IDisposable { return; } + // xterm emits onScroll synchronously before it queues the viewport refresh. + // Reconcile only the newly visible lines in that event so decorations are + // registered before the next frame, including for distant scrollbar jumps. + if (mode === "immediate" && reason === "scroll") { + if (this.animationFrameId !== null) { + cancelAnimationFrame(this.animationFrameId); + this.animationFrameId = null; + } + if (this.debounceTimer) { + clearTimeout(this.debounceTimer); + this.debounceTimer = null; + } + this.executeRefresh(); + return; + } + if (mode === "continuation") { if (this.animationFrameId !== null) { return; @@ -682,9 +699,7 @@ export class KeywordHighlighter implements IDisposable { const inputQuietDelay = reason === "write" ? this.getInputProtectionRemainingMs(performance.now()) : 0; - const delay = reason === "scroll" - ? KeywordHighlighter.SCROLL_SETTLE_DEBOUNCE_MS - : Math.max(this.getAdaptiveHighlightingProfile().debounceMs, inputQuietDelay); + const delay = Math.max(this.getAdaptiveHighlightingProfile().debounceMs, inputQuietDelay); this.debounceTimer = setTimeout(() => { this.debounceTimer = null; this.executeRefresh(); @@ -692,31 +707,20 @@ export class KeywordHighlighter implements IDisposable { } private triggerViewportChangeRefresh() { - this.cancelScrollRefresh(); - // Enter echo often emits onScroll before onWriteParsed. After an idle gap - // lastWriteAt looks stale, so the user-scroll path would prune overscan - // decorations and flash keywords still on screen. While Enter output is - // pending, skip scroll-prune but mark the viewport dirty so idle-clear / - // writeParsed can catch up (including user scroll during the protection window). - if (this.enterInputPending) { - if (this.pendingRefreshReason === "scroll") { - this.cancelQueuedRefreshSchedule(); - this.pendingRefreshReason = "write"; - } - this.markVisibleRangeDirty(); - return; - } const now = performance.now(); + const buffer = this.term.buffer.active; + const isBrowsingScrollback = buffer.viewportY < buffer.baseY; const isOutputDrivenViewportChange = + !isBrowsingScrollback && this.lastWriteAt > 0 && now - this.lastWriteAt <= KeywordHighlighter.WRITE_BURST_HIGHLIGHT_PAUSE_MS; - if (isOutputDrivenViewportChange || this.isWriteBurstActive(now)) { + if (isOutputDrivenViewportChange || (!isBrowsingScrollback && this.isWriteBurstActive(now))) { this.markVisibleRangeDirty(); this.triggerRefresh("debounced", "write"); return; } - this.triggerRefresh("debounced", "scroll"); + this.triggerRefresh("immediate", "scroll"); } private refreshViewport(reason: RefreshReason) { @@ -734,11 +738,11 @@ export class KeywordHighlighter implements IDisposable { const rangeStart = Math.max(0, viewportY - overscan); const rangeEnd = viewportEnd + overscan; - const previousRange = this.lastRenderRange; this.beginTerminalRefreshTracking(viewportStart, viewportEnd); let writeContinuationPending = false; try { - this.reindexLineDecorationsFromMarkers(); + this.syncLineDecorationIndex(); + const previousRange = this.lastRenderRange; if (reason === "write") { writeContinuationPending = this.processDirtyLinesInRange( @@ -748,8 +752,12 @@ export class KeywordHighlighter implements IDisposable { "write", ); } else if (reason === "scroll") { - this.startScrollRefresh(viewportStart, viewportEnd, cursorAbsoluteY); - return; + this.processScrollViewport( + viewportStart, + viewportEnd, + cursorAbsoluteY, + previousRange, + ); } else if (previousRange !== null && this.lineDecorations.size > 0) { if (rangeStart < previousRange.start) { this.processLineRange(rangeStart, Math.min(rangeEnd, previousRange.start - 1), cursorAbsoluteY); @@ -768,12 +776,6 @@ export class KeywordHighlighter implements IDisposable { viewportStart, viewportEnd, ); - } else { - for (const [lineY, state] of this.lineDecorations) { - if (lineY < rangeStart || lineY > rangeEnd || state.marker.isDisposed) { - this.disposeLineDecorations(lineY, state); - } - } } // `write` refresh only processes dirty lines and does NOT guarantee the whole @@ -782,6 +784,8 @@ export class KeywordHighlighter implements IDisposable { if (reason === "write") { this.lastViewportRange = null; this.lastRenderRange = null; + } else if (reason === "scroll") { + // processScrollViewport records the contiguous range already indexed. } else { this.lastViewportRange = { start: viewportStart, end: viewportEnd }; this.lastRenderRange = { start: rangeStart, end: rangeEnd }; @@ -839,8 +843,36 @@ export class KeywordHighlighter implements IDisposable { } } - private reindexLineDecorationsFromMarkers() { - if (this.lineDecorations.size === 0) return; + private toIndexedLine(lineY: number): number { + return lineY - this.markerLineOffset; + } + + private getLineDecorationState(lineY: number): LineDecorationState | undefined { + this.syncLineDecorationIndex(); + let state = this.lineDecorations.get(this.toIndexedLine(lineY)); + if (state && state.marker.line !== lineY) { + this.syncLineDecorationIndex(true); + state = this.lineDecorations.get(this.toIndexedLine(lineY)); + } + return state; + } + + private syncLineDecorationIndex(force = false) { + force = force || this.lineDecorationIndexNeedsRebuild; + if (this.lineDecorations.size === 0) { + this.markerLineOffset = 0; + this.lineDecorationIndexNeedsRebuild = false; + return; + } + + if (!force) { + const anchor = this.lineDecorations.values().next().value as LineDecorationState | undefined; + if (anchor && !anchor.marker.isDisposed && anchor.marker.line >= 0) { + this.markerLineOffset += anchor.marker.line - (anchor.indexedLine + this.markerLineOffset); + return; + } + } + const nextLineDecorations = new Map(); const staleStates = new Set(); @@ -849,12 +881,12 @@ export class KeywordHighlighter implements IDisposable { staleStates.add(state); continue; } - const markerLine = state.marker.line; - const existing = nextLineDecorations.get(markerLine); + state.indexedLine = state.marker.line; + const existing = nextLineDecorations.get(state.indexedLine); if (existing && existing !== state) { staleStates.add(existing); } - nextLineDecorations.set(markerLine, state); + nextLineDecorations.set(state.indexedLine, state); } for (const state of nextLineDecorations.values()) { @@ -862,6 +894,11 @@ export class KeywordHighlighter implements IDisposable { } this.lineDecorations = nextLineDecorations; + this.markerLineOffset = 0; + this.lineDecorationIndexNeedsRebuild = false; + if (staleStates.size > 0) { + this.lastRenderRange = null; + } for (const state of staleStates) { const markerLineBeforeDispose = state.marker.isDisposed ? -1 : state.marker.line; @@ -873,6 +910,23 @@ export class KeywordHighlighter implements IDisposable { } } + private prunePersistentDecorations() { + const config = XTERM_PERFORMANCE_CONFIG.highlighting; + const maxPersistentLines = Math.min( + config.maxPersistentDecorationLines, + Math.max( + config.minPersistentDecorationLines, + this.term.rows * config.persistentDecorationViewports, + ), + ); + + while (this.lineDecorations.size > maxPersistentLines) { + const oldest = this.lineDecorations.values().next().value as LineDecorationState | undefined; + if (!oldest) break; + this.disposeLineDecorations(oldest.marker.line, oldest); + } + } + private processDirtyLinesInRange( rangeStart: number, rangeEnd: number, @@ -952,7 +1006,9 @@ export class KeywordHighlighter implements IDisposable { // Decoration registration/removal makes xterm repaint the full viewport. // Prune in batches so ordinary one-line output keeps existing highlights // stable while long-running output remains bounded. - for (const [lineY, state] of this.lineDecorations) { + this.syncLineDecorationIndex(); + for (const state of [...this.lineDecorations.values()]) { + const lineY = state.marker.isDisposed ? -1 : state.marker.line; if (lineY < rangeStart || lineY > rangeEnd || state.marker.isDisposed) { this.disposeLineDecorations(lineY, state); } @@ -964,8 +1020,10 @@ export class KeywordHighlighter implements IDisposable { rangeEnd: number, targetSize: number, ): void { - for (const [lineY, state] of this.lineDecorations) { + this.syncLineDecorationIndex(); + for (const state of [...this.lineDecorations.values()]) { if (this.lineDecorations.size <= targetSize) return; + const lineY = state.marker.isDisposed ? -1 : state.marker.line; if (lineY < rangeStart || lineY > rangeEnd || state.marker.isDisposed) { this.disposeLineDecorations(lineY, state); } @@ -1004,10 +1062,6 @@ export class KeywordHighlighter implements IDisposable { this.enterInputIdleTimer = setTimeout(() => { this.enterInputIdleTimer = null; this.enterInputPending = false; - // Catch up any viewport motion deferred while Enter protection blocked - // scroll-prune (e.g. user scrolled during the post-Enter window). - this.markVisibleRangeDirty(); - this.triggerRefresh("debounced", "write"); }, KeywordHighlighter.WRITE_PRUNE_IDLE_MS); } @@ -1043,8 +1097,13 @@ export class KeywordHighlighter implements IDisposable { } private hasDecorationMarkerShiftSinceLastRefresh(): boolean { - for (const [lineY, state] of this.lineDecorations) { - if (state.marker.isDisposed || state.marker.line !== lineY) return true; + for (const state of this.lineDecorations.values()) { + if ( + state.marker.isDisposed + || state.marker.line !== state.indexedLine + this.markerLineOffset + ) { + return true; + } } return false; } @@ -1238,6 +1297,7 @@ export class KeywordHighlighter implements IDisposable { // Detect in-place ANSI redraw chunks (cursor returns near original line while // multiple viewport regions are actually rewritten). if (sameWindow && cursorSpan <= Math.max(1, padding * 2) && probeDiffCount >= 2) { + this.lineDecorationIndexNeedsRebuild = true; this.markVisibleRangeDirty(); return; } @@ -1441,7 +1501,7 @@ export class KeywordHighlighter implements IDisposable { } const signature = this.buildRangesSignature(cachedRanges); - const existing = this.lineDecorations.get(lineY); + const existing = this.getLineDecorationState(lineY); if ( existing && !existing.marker.isDisposed && @@ -1458,58 +1518,46 @@ export class KeywordHighlighter implements IDisposable { } } - private startScrollRefresh(start: number, end: number, cursorAbsoluteY: number) { - this.cancelScrollRefresh(); - const generation = this.scrollRefreshGeneration; - this.scrollRefreshJob = { - generation, - start, - end, - nextLine: start, - cursorAbsoluteY, - wrappedBlockCache: this.createWrappedBlockScanCache(), - }; - this.runScrollRefreshChunk(generation); - } - - private runScrollRefreshChunk(generation: number) { - const job = this.scrollRefreshJob; - if (!job || job.generation !== generation) return; - if (!this.enabled || this.compiledRules.length === 0 || this.term.buffer.active.type === "alternate") { - this.cancelScrollRefresh(); - return; - } - - this.beginTerminalRefreshTracking(job.start, job.end); - try { - this.reindexLineDecorationsFromMarkers(); - this.processLineRange( - job.nextLine, - job.end, - job.cursorAbsoluteY, - job.wrappedBlockCache, - ); - this.removeDirtyRange(job.nextLine, job.end); - job.nextLine = job.end + 1; - } finally { - this.flushTerminalRefresh(); + private processScrollViewport( + start: number, + end: number, + cursorAbsoluteY: number, + previousRange: DirtyLineSegment | null, + ) { + const wrappedBlockCache = this.createWrappedBlockScanCache(); + const overlapsPreviousRange = previousRange !== null + && start <= previousRange.end + 1 + && end + 1 >= previousRange.start; + + if (!overlapsPreviousRange || previousRange === null) { + this.processLineRange(start, end, cursorAbsoluteY, wrappedBlockCache); + this.lastRenderRange = { start, end }; + } else { + if (start < previousRange.start) { + this.processLineRange( + start, + Math.min(end, previousRange.start - 1), + cursorAbsoluteY, + wrappedBlockCache, + ); + } + if (end > previousRange.end) { + this.processLineRange( + Math.max(start, previousRange.end + 1), + end, + cursorAbsoluteY, + wrappedBlockCache, + ); + } + this.lastRenderRange = { + start: Math.min(start, previousRange.start), + end: Math.max(end, previousRange.end), + }; } - this.finishScrollRefresh(job); - } - - private finishScrollRefresh(job: ScrollRefreshJob) { - if (this.scrollRefreshJob !== job) return; - this.scrollRefreshJob = null; + this.removeDirtyRange(start, end); this.dirtyAllInRenderRange = false; - this.clearLineDecorationsOutsideRange(job.start, job.end); - this.lastViewportRange = { start: job.start, end: job.end }; - this.lastRenderRange = { start: job.start, end: job.end }; - } - - private cancelScrollRefresh() { - this.scrollRefreshJob = null; - this.scrollRefreshGeneration += 1; + this.lastViewportRange = { start, end }; } private cancelQueuedRefreshSchedule() { @@ -1523,15 +1571,6 @@ export class KeywordHighlighter implements IDisposable { } } - private clearLineDecorationsOutsideRange(start: number, end: number) { - if (this.lineDecorations.size === 0) return; - const entries = Array.from(this.lineDecorations.entries()); - for (const [lineY, state] of entries) { - if (lineY >= start && lineY <= end) continue; - this.disposeLineDecorations(lineY, state); - } - } - private getCachedRanges( line: IBufferLine, lineText: string, diff --git a/infrastructure/config/xtermPerformance.ts b/infrastructure/config/xtermPerformance.ts index 2a7d183a04..e6c5513ce5 100644 --- a/infrastructure/config/xtermPerformance.ts +++ b/infrastructure/config/xtermPerformance.ts @@ -110,6 +110,11 @@ export const XTERM_PERFORMANCE_CONFIG = { immediateMinIntervalMs: 16, // Number of unique line scan results to keep cached. cacheEntries: 1200, + // Retain matched-line markers across nearby scrollback without allowing a + // broad rule to register unbounded xterm trim listeners. + persistentDecorationViewports: 20, + minPersistentDecorationLines: 200, + maxPersistentDecorationLines: 1200, // Keep decorations for lines just outside the viewport so small scrolls // don't constantly dispose/recreate them. Scales with current terminal rows. overscanViewportRatio: 2.0, @@ -124,8 +129,6 @@ export const XTERM_PERFORMANCE_CONFIG = { writeRefreshBudgetMs: 4, // Process dirty contiguous lines in chunks so budget checks can preempt. dirtySegmentChunkSize: 48, - // User-scroll catch-up should be almost invisible to the renderer. - scrollSettleDebounceMs: 120, // Keep highlighting deprioritized briefly after a large output burst. // Longer quiet window lets xterm paint bulk dumps (cat/yes/tail) without // competing decoration scans every few hundred ms. From 7896667700dad5f731466fa1928de0f05aeddc9a Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 7 Aug 2026 09:54:37 +0000 Subject: [PATCH 4/5] fix(terminal): bound persistent keyword prune to scroll path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep Enter/write deferred pruning intact by pruning the retention cap only after scroll viewport indexing, not on every decoration apply. Co-authored-by: 陈大猫 --- components/terminal/keywordHighlight.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/terminal/keywordHighlight.ts b/components/terminal/keywordHighlight.ts index 608a781c90..129b409b36 100644 --- a/components/terminal/keywordHighlight.ts +++ b/components/terminal/keywordHighlight.ts @@ -519,7 +519,6 @@ export class KeywordHighlighter implements IDisposable { } }); this.lineDecorations.set(state.indexedLine, state); - this.prunePersistentDecorations(); this.markTerminalRefreshNeeded(lineY); } @@ -1558,6 +1557,7 @@ export class KeywordHighlighter implements IDisposable { this.removeDirtyRange(start, end); this.dirtyAllInRenderRange = false; this.lastViewportRange = { start, end }; + this.prunePersistentDecorations(); } private cancelQueuedRefreshSchedule() { From a10ad7716683b3f72ed66be0d33859dcf15c45ec Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 7 Aug 2026 10:01:18 +0000 Subject: [PATCH 5/5] fix(terminal): rescan dirty overlap on persistent scroll refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Overlapping scroll only scanned newly exposed rows then cleared the whole viewport dirty set, which could drop in-place redraw work when scroll outranked a pending write refresh. Rescan write-dirtied overlap lines and only clear dirty for ranges that were actually processed. Co-authored-by: 陈大猫 --- components/terminal/keywordHighlight.test.ts | 51 ++++++++++++++++++++ components/terminal/keywordHighlight.ts | 30 ++++++++++-- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/components/terminal/keywordHighlight.test.ts b/components/terminal/keywordHighlight.test.ts index 1744c372f2..ed3b5b557a 100644 --- a/components/terminal/keywordHighlight.test.ts +++ b/components/terminal/keywordHighlight.test.ts @@ -1025,6 +1025,57 @@ test("in-place redraw removes a persistent highlight when text stops matching", } }); +test("overlapping scroll rescans write-dirtied lines instead of clearing them", async () => { + const raf = installAnimationFrameQueue(); + try { + const { + term, + handlers, + getActiveDecorationCount, + setLineText, + } = createFakeTerminal("hello DEPLOY world", { lineCount: 12 }); + term.rows = 4; + term.buffer.active.viewportY = 0; + term.buffer.active.baseY = 0; + const highlighter = new KeywordHighlighter(term as never); + highlighter.setRules([{ + id: "deploy", + label: "Deploy", + patterns: ["DEPLOY"], + color: "#F87171", + enabled: true, + }], true); + raf.flush(); + assert.equal(getActiveDecorationCount(), 12); + + const internals = highlighter as unknown as { + lastRenderRange: { start: number; end: number } | null; + addDirtyRange: (start: number, end: number) => void; + dirtySegments: Array<{ start: number; end: number }>; + }; + assert.ok(internals.lastRenderRange); + + // Simulate an in-place redraw that dirtied an overlapping line, then a + // one-row scroll that cancels the pending write refresh. Scroll must still + // rescan that dirty overlap. + setLineText(1, "hello SAFE world 1"); + internals.addDirtyRange(1, 1); + assert.ok(internals.dirtySegments.length > 0); + term.buffer.active.viewportY = 1; + handlers.scroll?.(); + raf.flush(); + + assert.equal( + getActiveDecorationCount(), + 11, + "overlapping scroll should rescan write-dirtied lines in the overlap", + ); + highlighter.dispose(); + } finally { + raf.restore(); + } +}); + test("Enter-driven scroll does not dispose nearby keyword decorations", async () => { const raf = installAnimationFrameQueue(); try { diff --git a/components/terminal/keywordHighlight.ts b/components/terminal/keywordHighlight.ts index 129b409b36..6a2ff96af9 100644 --- a/components/terminal/keywordHighlight.ts +++ b/components/terminal/keywordHighlight.ts @@ -1531,31 +1531,53 @@ export class KeywordHighlighter implements IDisposable { if (!overlapsPreviousRange || previousRange === null) { this.processLineRange(start, end, cursorAbsoluteY, wrappedBlockCache); this.lastRenderRange = { start, end }; + this.removeDirtyRange(start, end); + this.dirtyAllInRenderRange = false; } else { if (start < previousRange.start) { + const exposedEnd = Math.min(end, previousRange.start - 1); this.processLineRange( start, - Math.min(end, previousRange.start - 1), + exposedEnd, cursorAbsoluteY, wrappedBlockCache, ); + this.removeDirtyRange(start, exposedEnd); } if (end > previousRange.end) { + const exposedStart = Math.max(start, previousRange.end + 1); this.processLineRange( - Math.max(start, previousRange.end + 1), + exposedStart, end, cursorAbsoluteY, wrappedBlockCache, ); + this.removeDirtyRange(exposedStart, end); + } + + // Overlap was previously indexed; only rescan lines still marked dirty by + // writes (in-place redraws). Never clear the whole viewport dirty set here — + // scroll can outrank/cancel a pending write refresh. + const overlapStart = Math.max(start, previousRange.start); + const overlapEnd = Math.min(end, previousRange.end); + if ( + overlapStart <= overlapEnd + && (this.dirtyAllInRenderRange || this.dirtySegments.length > 0) + ) { + this.processDirtyLinesInRange( + overlapStart, + overlapEnd, + cursorAbsoluteY, + "write", + ); } + this.lastRenderRange = { start: Math.min(start, previousRange.start), end: Math.max(end, previousRange.end), }; } - this.removeDirtyRange(start, end); - this.dirtyAllInRenderRange = false; this.lastViewportRange = { start, end }; this.prunePersistentDecorations(); }