Repository navigation
fix(cli): eliminate banner duplication and drag flicker on resize/wake #8831
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 12 commits
d69f7aa
6bea8b7
0ba19b4
cb1297a
2515655
575ad26
50d755b
c3229b2
dad05dc
c4b5468
4f7b4ba
15d4be6
d2e01e8
7584697
78469b4
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 |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ import { | |
| pushKittyProtocolFlags, | ||
| } from './utils/kittyProtocolDetector.js'; | ||
| import { installTerminalRedrawOptimizer } from './utils/terminalRedrawOptimizer.js'; | ||
| import { installTerminalResizeReflow } from './utils/terminal-resize-reflow.js'; | ||
|
chiga0 marked this conversation as resolved.
Resolved
|
||
| import { installSynchronizedOutput } from './utils/synchronizedOutput.js'; | ||
| import { | ||
| isInteractiveTerminal, | ||
|
|
@@ -164,6 +165,15 @@ export async function startInteractiveUI( | |
| isInteractiveTerminal(), | ||
| ); | ||
|
|
||
| // On width shrink the terminal reflows the printed frame into more physical | ||
| // rows than Ink's stale erase count (issue #8557); amplify the clear to the | ||
| // reflowed height. Installed before render() so the resize listener runs | ||
| // ahead of Ink's resized(). | ||
| const resizeReflow = | ||
|
Collaborator
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. [Suggestion] No regression test drives a real terminal through the shrink path. The 38 unit tests validate the erase-amplification model against the wrapper's own reflow assumption — Failure scenario: the width model diverges from a real terminal's reflow, and the issue's exact symptom (duplicated/stacked transcript on shrink) returns on real terminals while all unit tests stay green. Suggested fix: commit a PTY/tmux-based regression in the integration harness that shrinks a real terminal mid-session and asserts a single reprint; at minimum, capture the byte-level shrink rig output as a fixture. 中文说明没有任何回归测试驱动真实终端走缩窄路径:38 个单测用 FakeStdout + 与生产代码同一套打包逻辑验证放大模型,模型与测试共享任何宽度计算错误;#8557 线程里作者自己已记录过 Warp 上的真实分歧(CJK 字体回退把
Collaborator
Author
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. Accepted as a known limitation, deferred: a PTY/tmux shrink regression would not share the width-model assumption, but the repo's interactive tmux harness is flaky-prone in CI and the byte-level rig would still bind to one terminal's reflow semantics (Warp's CJK fallback diverges from any app-side model, as documented in this thread). Mitigations in place: 38+ unit tests pin the wrapper contract, and the fix was author-verified on Ghostty and Warp (live-view duplication gone; Warp block-history snapshots remain platform behavior, documented). Tracking a real-terminal shrink regression as follow-up work; not blocking this PR. |
||
| process.stdout.isTTY && !config.getScreenReader() | ||
| ? installTerminalResizeReflow(process.stdout, { virtualViewport: useVP }) | ||
| : { restore: () => {}, repaint: () => {} }; | ||
|
|
||
| // Create wrapper component to use hooks inside render | ||
| const AppWrapper = () => { | ||
| const kittyProtocolStatus = useKittyKeyboardProtocol(); | ||
|
|
@@ -195,6 +205,7 @@ export async function startInteractiveUI( | |
| initializationResult={initializationResult} | ||
| initialUseVirtualViewport={useVP} | ||
| extensionRefreshState={options.extensionRefreshState} | ||
| repaintViewport={resizeReflow.repaint} | ||
| /> | ||
| </BackgroundTaskViewProvider> | ||
| </AgentViewProvider> | ||
|
|
@@ -313,6 +324,10 @@ export async function startInteractiveUI( | |
| if (useVP) { | ||
| process.stdout.setMaxListeners(stdoutMaxListeners); | ||
| } | ||
| // Unwind the stdout.write wrapper stack in LIFO order (resizeReflow is | ||
| // installed last / outermost); the identity-guarded restores silently | ||
| // no-op and leak wrappers otherwise. | ||
| resizeReflow.restore(); | ||
| restoreSynchronizedOutput(); | ||
|
chiga0 marked this conversation as resolved.
Resolved
|
||
| restoreTerminalRedrawOptimizer(); | ||
| // If the ErrorBoundary caught a rendering error, echo it to stderr | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.