-
Notifications
You must be signed in to change notification settings - Fork 495
fix(terminal): stop idle-Enter keyword highlight flash after #2880 hole #2931
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
cd12b04
9cd756f
7408e96
60820ac
d1cd072
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -123,6 +123,12 @@ 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. */ | ||
| private enterWriteParsedSeen = false; | ||
| /** 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 +160,11 @@ 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; | ||
| // 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 +235,24 @@ export class KeywordHighlighter implements IDisposable { | |
| const inputProtectionActive = this.isInputProtectionActive(performance.now()); | ||
| if (inputProtectionActive || this.enterInputPending) { | ||
| if (this.enterInputPending) { | ||
| if (this.enterViewportScanInProgress) { | ||
| if (this.enterWriteParsedSeen) { | ||
| // Sustained Enter output: allow decoration catch-up so new matches | ||
| // are not postponed until the stream goes idle. Do not re-add the | ||
| // whole viewport dirty range — that rewinds multi-frame scans. | ||
| this.enterSuppressDecorationMutation = false; | ||
| this.updateWriteBurst(); | ||
| this.enterViewportScanNeedsRepeat = true; | ||
| 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 +290,7 @@ export class KeywordHighlighter implements IDisposable { | |
| }) | ||
| ); | ||
| this.lastBufferSnapshot = this.readBufferSnapshot(); | ||
| this.wasBrowsingScrollback = this.isBrowsingScrollback(); | ||
| } | ||
|
|
||
| public setRules(rules: readonly RuntimeKeywordHighlightRule[], enabled: boolean) { | ||
|
|
@@ -430,6 +455,8 @@ export class KeywordHighlighter implements IDisposable { | |
| this.enterQueuedWriteCancellationPending = false; | ||
| this.enterViewportScanInProgress = false; | ||
| this.enterViewportScanNeedsRepeat = false; | ||
| this.enterSuppressDecorationMutation = false; | ||
| this.enterWriteParsedSeen = false; | ||
| if (hadDecorations) { | ||
| this.term.refresh(0, this.term.rows - 1); | ||
| } | ||
|
|
@@ -715,16 +742,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 +1120,8 @@ export class KeywordHighlighter implements IDisposable { | |
| this.enterInputIdleTimer = setTimeout(() => { | ||
| this.enterInputIdleTimer = null; | ||
| this.enterInputPending = false; | ||
| this.enterSuppressDecorationMutation = false; | ||
| this.enterWriteParsedSeen = false; | ||
| // Catch up any viewport motion deferred while Enter protection blocked | ||
| // scroll refresh (e.g. user scrolled during the post-Enter window). | ||
| this.markVisibleRangeDirty(); | ||
|
|
@@ -1519,13 +1555,17 @@ export class KeywordHighlighter implements IDisposable { | |
| 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 +1576,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 +1595,12 @@ export class KeywordHighlighter implements IDisposable { | |
| continue; | ||
| } | ||
|
|
||
| // Idle Enter: keep prior decorations mounted and skip new ones until the | ||
| // guard clears or sustained Enter output opts back into mutation. | ||
| if (this.enterSuppressDecorationMutation) { | ||
| continue; | ||
| } | ||
|
Comment on lines
+1638
to
+1645
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a user presses Enter and immediately scrolls to previously unindexed scrollback, the synchronous scroll refresh reaches this guard and skips every new decoration because Enter suppression is still active. Useful? React with 👍 / 👎. |
||
|
|
||
| this.disposeLineDecorations(lineY, existing); | ||
| this.applyLineDecorations(lineY, cachedRanges, signature, cursorAbsoluteY); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a single idle Enter's echo and prompt arrive in two
onWriteParsedbatches, the second callback reaches this branch and clears suppression even though both writes belong to the same idle submission. The queued scan then disposes or registers prompt-row decorations—for example with custom~/#rules—causing the exact full-viewport repaint flash this change is intended to prevent. A second callback alone is not a reliable signal of sustained command output, so suppression should remain until an actual output/quiet criterion is met.Useful? React with 👍 / 👎.