fix(terminal): stop idle-Enter keyword highlight flash after #2880 hole - #2931
Conversation
netcatty-bot
added
automation:bot-pr
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd12b04647
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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; |
There was a problem hiding this comment.
When a single idle Enter's echo and prompt arrive in two onWriteParsed batches, 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 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cd756f44b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (this.enterWriteParsedCount >= KeywordHighlighter.ENTER_SUPPRESS_LIFT_WRITE_COUNT) { | ||
| return true; | ||
| } | ||
| return this.isWriteBurstActive(performance.now()); |
There was a problem hiding this comment.
When a prompt is emitted in three onWriteParsed batches—for example separate control-sequence, prompt-text, and mode-setting writes—this fixed callback threshold lifts suppression before the final prompt batch is scanned, allowing matching ~/# decorations to trigger the same full-viewport repaint. The fresh evidence in this revision is that the previous second-write trigger was replaced by an equally non-semantic third-write trigger; callback count still does not prove command output, so suppression should remain until an actual output or quiet criterion distinguishes the prompt.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7408e960cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (this.shouldLiftEnterDecorationSuppression()) { | ||
| this.enterSuppressDecorationMutation = false; |
There was a problem hiding this comment.
When Enter follows another write burst within the burst-decay window, recentWriteBurst and lastWriteAt still describe the earlier output because the Enter handler does not reset them. A split echo/prompt can therefore make this predicate true on its second batch, lift suppression, and recreate the prompt-decoration viewport flash. The fresh evidence in this revision is that the new burst-based signal remains global rather than being scoped to writes observed after the current Enter.
Useful? React with 👍 / 👎.
| private shouldLiftEnterDecorationSuppression(): boolean { | ||
| if (!this.enterSuppressDecorationMutation) return false; | ||
| return this.isWriteBurstActive(performance.now()); |
There was a problem hiding this comment.
When a post-Enter command emits small writes every roughly 30–500 ms, updateWriteBurst() never reaches the six-write threshold because each interval exceeds 28 ms, while every callback rearms the 600 ms Enter guard. Consequently tail -f, progress output, and other steady but non-bursty commands can keep decoration mutation suppressed indefinitely, leaving new matches unhighlighted and removed matches stale until the stream pauses; the catch-up criterion needs to distinguish or periodically service these slower streams too.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60820acf9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (this.enterSuppressDecorationMutation) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
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. processScrollViewport nevertheless records the range as covered, so matching lines remain visibly unhighlighted until the 600 ms Enter guard plus debounce expires; suppression should preserve existing prompt-area decorations without blocking decoration creation for a real scrollback browse.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
netcatty-bot
removed
the
automation:codex-loop
2 participants
Summary
onScrollbefore dims change).~/#rules) cannot force an xterm full-viewport repaint that flashes still-visible matches.Why
#2880 deferred bottom-pinned Enter scroll, then a follow-up required
hasOutputDrivenViewportChange()so End stayed responsive. That gate reopened the flash: after an idle gap, Enter echo can scroll while the last snapshot still matches, so highlight treated it as a user scroll and rescanned. Separately, applying or disposing decorations on the new prompt line makes xterm repaint the whole viewport, which is what users see as existing keywords “闪” even when those decorations were never disposed.Changes
components/terminal/keywordHighlight.ts: bottom Enter deferral useswasBrowsingScrollback/ return-to-bottom; idle Enter mutes decoration mutation; second write re-enables mutation without rewinding dirty ranges.components/terminal/keywordHighlight.test.ts: coverage for scroll-before-buffer-dim update and idle prompt redraw; Enter redraw / full-refresh catch-up waits for the guard window.Testing
node --test --import tsx components/terminal/keywordHighlight.test.ts(52 pass)~/#rule optional, idle Enter — existing on-screen matches should not flash.Fixes #2879
Automation
@codex review(own/bot PRs only)