Repository navigation
fix(cli): default to virtualized terminal history #5738
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -69,7 +69,7 @@ This document lists the available keyboard shortcuts in Qwen Code. | |||||
|
|
||||||
| ## History scrollback | ||||||
|
|
||||||
| Active when `ui.useTerminalBuffer` is enabled (Settings → UI → Virtualized History) and screen reader mode is off, which is the default for non-screen-reader sessions. In that mode conversation history is rendered inside an in-app viewport instead of the host terminal scrollback, so the keys below replace the terminal's native scroll. | ||||||
|
Collaborator
There was a problem hiding this comment. [Suggestion] This activation description omits the terminal-compatibility gate. Even with
Suggested change
— Codex $qreview via Qwen Code /review
Contributor
Author
There was a problem hiding this comment. Addressed in cef51a6. The shortcut documentation now states the interactive-terminal gate explicitly, including TTY stdout, inactive CI, and TERM != dumb. |
||||||
|
|
||||||
| | Shortcut | Description | | ||||||
| | --------------- | ---------------------------------------------------- | | ||||||
|
|
@@ -87,7 +87,7 @@ When `ui.useTerminalBuffer` is on, the terminal forwards mouse events to qwen-co | |||||
|
|
||||||
| Inside tmux, some terminals translate trackpad or wheel gestures into plain `Up Arrow` and `Down Arrow` sequences before qwen-code sees them. Those bytes are identical to real arrow-key presses, so qwen-code cannot tell whether you meant to scroll the viewport or navigate prompt history. | ||||||
|
|
||||||
| If trackpad scrolling changes the prompt history in tmux, make sure `ui.useTerminalBuffer` is enabled; then use `Shift+Up` / `Shift+Down`, or the mouse wheel when tmux forwards wheel events to the app. If you prefer host scrollback, adjust your tmux mouse bindings for wheel events. | ||||||
|
|
||||||
| ## IDE Integration | ||||||
|
|
||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1005,10 +1005,10 @@ const SETTINGS_SCHEMA = { | |
| type: 'boolean', | ||
| label: 'Virtualized History (reduces flicker on long sessions)', | ||
| category: 'UI', | ||
| requiresRestart: true, | ||
| default: true, | ||
|
Contributor
Author
There was a problem hiding this comment. Fixed in ddc0c31. InputPrompt, BaseSelectionList, and useMouseEvents now prefer the frozen VP decision from UIStateContext, with raw settings only as a fallback for usage outside UIStateContext. Added regression coverage for the unset raw setting across all three mouse paths. |
||
| description: | ||
| 'Render conversation history in an in-app scrollable viewport instead of the terminal scrollback buffer. Enabled by default to avoid flicker, scroll-storm, and interface freeze on long sessions, after Ctrl+O, after Ctrl+E / Ctrl+F (expand), after window resize, or when alt-tabbing back. Screen reader mode uses append-only terminal output instead. Scroll with Shift+↑/↓ (line), PgUp/PgDn (page), Ctrl+Home/End (top/bottom), or the mouse wheel. Does NOT use the host terminal scrollback while enabled; for native text selection, hold Shift (or Option on macOS) while dragging.', | ||
| showInDialog: true, | ||
| }, | ||
| shellOutputMaxLines: { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1253,8 +1253,22 @@ describe('startInteractiveUI', () => { | |
| render: vi.fn().mockReturnValue({ unmount: vi.fn() }), | ||
| })); | ||
|
|
||
| let initialExitListeners: NodeJS.ExitListener[] = []; | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| initialExitListeners = process.listeners('exit') as NodeJS.ExitListener[]; | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| const currentExitListeners = process.listeners( | ||
| 'exit', | ||
| ) as NodeJS.ExitListener[]; | ||
| for (const listener of currentExitListeners) { | ||
| if (!initialExitListeners.includes(listener)) { | ||
| process.removeListener('exit', listener); | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| it('should render the UI with proper React context and exitOnCtrlC disabled', async () => { | ||
|
|
@@ -1284,13 +1298,116 @@ describe('startInteractiveUI', () => { | |
| expect(options).toEqual({ | ||
| exitOnCtrlC: false, | ||
| isScreenReaderEnabled: false, | ||
| alternateScreen: true, | ||
| }); | ||
|
|
||
| // Verify React element structure is valid (but don't deep dive into JSX internals) | ||
| expect(reactElement).toBeDefined(); | ||
| }); | ||
|
|
||
| it('should not use alternate screen when VP mode is explicitly disabled', async () => { | ||
| const { render } = await import('ink'); | ||
| const renderSpy = vi.mocked(render); | ||
| const legacySettings = { | ||
| ...mockSettings, | ||
| merged: { | ||
| ...mockSettings.merged, | ||
| ui: { | ||
| ...mockSettings.merged.ui, | ||
| useTerminalBuffer: false, | ||
| }, | ||
| }, | ||
| } as LoadedSettings; | ||
|
|
||
| const mockInitializationResult = { | ||
| authError: null, | ||
| themeError: null, | ||
| shouldOpenAuthDialog: false, | ||
| geminiMdFileCount: 0, | ||
| }; | ||
|
|
||
| await startInteractiveUI( | ||
| mockConfig, | ||
| legacySettings, | ||
| mockStartupWarnings, | ||
| mockWorkspaceRoot, | ||
| mockInitializationResult, | ||
| ); | ||
|
|
||
| const [, options] = renderSpy.mock.calls[0]; | ||
| expect(options).toMatchObject({ alternateScreen: false }); | ||
| }); | ||
|
|
||
| it('should not use alternate screen in screen reader mode when VP mode is unset', async () => { | ||
|
Collaborator
There was a problem hiding this comment. [Suggestion] Test gap: the screen-reader guard is tested when it('should not use alternate screen in screen reader mode even when VP mode is explicitly enabled', async () => {
// useTerminalBuffer: true, getScreenReader: () => true
// expect alternateScreen: false
});— qwen3.7-max via Qwen Code /review
Contributor
Author
There was a problem hiding this comment. Covered in 8f5eb98. Added the explicit useTerminalBuffer: true + screen-reader case for startInteractiveUI, asserting alternateScreen remains false. |
||
| const { render } = await import('ink'); | ||
| const renderSpy = vi.mocked(render); | ||
| const screenReaderConfig = { | ||
| ...mockConfig, | ||
| getScreenReader: () => true, | ||
|
Contributor
Author
There was a problem hiding this comment. Added in a22120a. startInteractiveUI now has a real-process-env integration test with stdout.isTTY=true and CI=true, asserting alternateScreen=false. I also corrected teardown ordering so vi.stubEnv values are removed before the original CI environment snapshot is restored, preventing cross-test env leakage on GitHub Actions. |
||
| } as Config; | ||
|
|
||
| const mockInitializationResult = { | ||
| authError: null, | ||
| themeError: null, | ||
| shouldOpenAuthDialog: false, | ||
| geminiMdFileCount: 0, | ||
| }; | ||
|
|
||
| await startInteractiveUI( | ||
| screenReaderConfig, | ||
| mockSettings, | ||
| mockStartupWarnings, | ||
| mockWorkspaceRoot, | ||
| mockInitializationResult, | ||
| ); | ||
|
|
||
| const [, options] = renderSpy.mock.calls[0]; | ||
| expect(options).toMatchObject({ | ||
| isScreenReaderEnabled: true, | ||
| alternateScreen: false, | ||
| }); | ||
| }); | ||
|
|
||
| it('should not use alternate screen in screen reader mode even when VP mode is explicitly enabled', async () => { | ||
| const { render } = await import('ink'); | ||
| const renderSpy = vi.mocked(render); | ||
| const screenReaderConfig = { | ||
| ...mockConfig, | ||
| getScreenReader: () => true, | ||
| } as Config; | ||
| const vpSettings = { | ||
| ...mockSettings, | ||
| merged: { | ||
| ...mockSettings.merged, | ||
| ui: { | ||
| ...mockSettings.merged.ui, | ||
| useTerminalBuffer: true, | ||
| }, | ||
| }, | ||
| } as LoadedSettings; | ||
|
|
||
| const mockInitializationResult = { | ||
| authError: null, | ||
| themeError: null, | ||
| shouldOpenAuthDialog: false, | ||
| geminiMdFileCount: 0, | ||
| }; | ||
|
|
||
| await startInteractiveUI( | ||
| screenReaderConfig, | ||
| vpSettings, | ||
| mockStartupWarnings, | ||
| mockWorkspaceRoot, | ||
| mockInitializationResult, | ||
| ); | ||
|
|
||
| const [, options] = renderSpy.mock.calls[0]; | ||
| expect(options).toMatchObject({ | ||
| isScreenReaderEnabled: true, | ||
| alternateScreen: false, | ||
| }); | ||
| }); | ||
|
|
||
| it('should perform all startup tasks in correct order', async () => { | ||
| const { getCliVersion } = await import('./utils/version.js'); | ||
| const { checkForUpdates } = await import('./ui/utils/updateCheck.js'); | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,7 +32,7 @@ import { | |
| type Mock, | ||
| } from 'vitest'; | ||
| import { render, cleanup } from 'ink-testing-library'; | ||
| import { useContext, useState, act } from 'react'; | ||
| import { | ||
| AppContainer, | ||
| dedupeNewestFirst, | ||
|
|
@@ -396,6 +396,7 @@ describe('AppContainer State Management', () => { | |
| ui: { | ||
| showStatusInTitle: false, | ||
| hideWindowTitle: false, | ||
| useTerminalBuffer: false, | ||
| }, | ||
| }, | ||
| setValue: vi.fn(), | ||
|
|
@@ -713,6 +714,107 @@ describe('AppContainer State Management', () => { | |
| ); | ||
| }); | ||
|
|
||
| it('defaults to VP mode when useTerminalBuffer is unset', () => { | ||
|
Contributor
Author
There was a problem hiding this comment. Added in a22120a. AppContainer now has an integration test that sets process.stdout.isTTY=false with useTerminalBuffer unset and asserts the frozen UI state stays on the Static path. The test environment restores the original TTY descriptor afterward. |
||
| const defaultSettings = { | ||
| merged: { | ||
| hideTips: false, | ||
| theme: 'default', | ||
| ui: { | ||
| showStatusInTitle: false, | ||
| hideWindowTitle: false, | ||
| }, | ||
| }, | ||
| setValue: vi.fn(), | ||
| } as unknown as LoadedSettings; | ||
|
|
||
| render( | ||
| <AppContainer | ||
| config={mockConfig} | ||
| settings={defaultSettings} | ||
| version="1.0.0" | ||
| initializationResult={mockInitResult} | ||
| />, | ||
| ); | ||
|
|
||
| expect(capturedUIState.useTerminalBuffer).toBe(true); | ||
| }); | ||
|
|
||
| it('keeps screen reader mode on the Static path when useTerminalBuffer is unset', () => { | ||
| vi.spyOn(mockConfig, 'getScreenReader').mockReturnValue(true); | ||
| const defaultSettings = { | ||
| merged: { | ||
| hideTips: false, | ||
| theme: 'default', | ||
| ui: { | ||
| showStatusInTitle: false, | ||
| hideWindowTitle: false, | ||
| }, | ||
| }, | ||
| setValue: vi.fn(), | ||
| } as unknown as LoadedSettings; | ||
|
|
||
| render( | ||
| <AppContainer | ||
| config={mockConfig} | ||
| settings={defaultSettings} | ||
| version="1.0.0" | ||
| initializationResult={mockInitResult} | ||
| />, | ||
| ); | ||
|
|
||
| expect(capturedUIState.useTerminalBuffer).toBe(false); | ||
| }); | ||
|
|
||
| it('locks terminal buffer mode for the running session', () => { | ||
| const vpSettings = { | ||
| merged: { | ||
| hideTips: false, | ||
| theme: 'default', | ||
| ui: { | ||
| showStatusInTitle: false, | ||
| hideWindowTitle: false, | ||
| useTerminalBuffer: true, | ||
| }, | ||
| }, | ||
| setValue: vi.fn(), | ||
| } as unknown as LoadedSettings; | ||
| const legacySettings = { | ||
| merged: { | ||
| hideTips: false, | ||
| theme: 'default', | ||
| ui: { | ||
| showStatusInTitle: false, | ||
| hideWindowTitle: false, | ||
| useTerminalBuffer: false, | ||
| }, | ||
| }, | ||
| setValue: vi.fn(), | ||
| } as unknown as LoadedSettings; | ||
|
|
||
| vi.spyOn(mockConfig, 'initialize').mockResolvedValue(undefined); | ||
| let updateSettings!: (settings: LoadedSettings) => void; | ||
| function Wrapper() { | ||
| const [settings, setSettings] = useState(vpSettings); | ||
| updateSettings = setSettings; | ||
| return ( | ||
| <AppContainer | ||
| config={mockConfig} | ||
| settings={settings} | ||
| version="1.0.0" | ||
| initializationResult={mockInitResult} | ||
| /> | ||
| ); | ||
| } | ||
|
|
||
| render(<Wrapper />); | ||
|
|
||
| expect(capturedUIState.useTerminalBuffer).toBe(true); | ||
|
|
||
| act(() => updateSettings(legacySettings)); | ||
|
|
||
| expect(capturedUIState.useTerminalBuffer).toBe(true); | ||
| }); | ||
|
|
||
| // #4891 changed the resize contract: width changes now trigger ONE full | ||
| // clearTerminal after RESIZE_REPAINT_SETTLE_MS (trailing-edge debounce), | ||
| // instead of never (#3967) or per-event (pre-#3967). This test pins the | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This snippet was updated for the new default, but it no longer matches the implementation this same PR ships:
MainContent.tsxdoesn't read settings here anymore — it reads the startup-frozenuiState.useTerminalBuffer(const useVirtualScroll = uiState.useTerminalBuffer;), whichAppContainercomputes once viashouldUseVirtualViewport(settings.merged.ui?.useTerminalBuffer, config.getScreenReader())in auseStateinitializer.As written, the snippet still documents a live, ungated settings read — the exact shape this PR deliberately removes (no screen-reader gate, mid-session flippable). A contributor following the doc would reintroduce a read that can desync from the
alternateScreendecision made once atrender().Suggest updating the snippet to the real shape (AppContainer's frozen
shouldUseVirtualViewport(...)+ MainContent readinguiState.useTerminalBuffer), and mentioning the screen-reader gate and restart-only semantics.🤖 Generated with Claude Code — Claude Fable 5
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.
Fixed in 8632a62. The design doc now shows the actual startup-frozen AppContainer decision using shouldUseVirtualViewport(..., isInteractiveTerminal()), and MainContent reading uiState.useTerminalBuffer. It also documents the screen-reader/non-interactive fallback and restart-only behavior.