Skip to content

fix(cli): show ⌥T instead of alt+T on macOS for thinking expansion #5802

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.

Sign up for GitHub

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

Merged
wenshao merged 5 commits into from
Jun 25, 2026
Merged

fix(cli): show ⌥T instead of alt+T on macOS for thinking expansion #5802

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ffde6e5
fix(cli): show ⌥T instead of alt+T on macOS for thinking expansion
Alex-ai-future Jun 24, 2026
e9b75c6
Merge branch 'main' into fix/alt-t
wenshao Jun 24, 2026
350c94c
fix(cli): address review feedback on macOS key hint PR
Alex-ai-future Jun 24, 2026
ebe604d
fix(cli): move toggleKeyHint to module scope
Alex-ai-future Jun 24, 2026
b82e70c
fix(cli): export toggleKeyHint as shared constant
Alex-ai-future Jun 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions packages/cli/src/ui/components/HistoryItemDisplay.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ vi.mock('../hooks/useMouseEvents.js', () => ({
useMouseEvents: vi.fn(),
}));

const toggleKeyHint = process.platform === 'darwin' ? 'option+t' : 'alt+t';

@wenshao wenshao Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Same cross-platform test gap as ConversationMessages.test.tsx — this constant mirrors the component's conditional, so only the host platform's branch is exercised. Consider mocking process.platform to verify both branches.

— qwen3.7-max via Qwen Code /review


describe('<HistoryItemDisplay />', () => {
const mockConfig = {
getChatRecordingService: () => undefined,
Expand Down Expand Up @@ -358,7 +360,7 @@ describe('<HistoryItemDisplay />', () => {

const output = lastFrame() ?? '';
expect(output).toContain('Thought for');
expect(output).toContain('alt+t to expand');
expect(output).toContain(`${toggleKeyHint} to expand`);
expect(output).not.toContain('Inspecting the repository');
});

Expand Down Expand Up @@ -394,7 +396,7 @@ describe('<HistoryItemDisplay />', () => {

const output = lastFrame() ?? '';
expect(output).toContain('Thought for');
expect(output).toContain('alt+t to expand');
expect(output).toContain(`${toggleKeyHint} to expand`);
expect(output).not.toContain('Inspecting the repository');
});

Expand All @@ -414,7 +416,7 @@ describe('<HistoryItemDisplay />', () => {

const output = lastFrame() ?? '';
expect(output).toContain('Thought for');
expect(output).toContain('alt+t to collapse');
expect(output).toContain(`${toggleKeyHint} to collapse`);
expect(output).toContain('Inspecting the repository');
});

Expand Down
10 changes: 6 additions & 4 deletions packages/cli/src/ui/components/messages/ConversationMessages.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import { render } from 'ink-testing-library';
import { ThinkMessage, ThinkMessageContent } from './ConversationMessages.js';

const toggleKeyHint = process.platform === 'darwin' ? 'option+t' : 'alt+t';

@wenshao wenshao Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Tests mirror the component's process.platform conditional, so only the host platform's branch is ever exercised in CI. Other tests in the project (e.g., detect-terminal-theme.test.ts, clipboardUtils.test.ts) mock process.platform to cover both branches.

Consider adding a test that stubs process.platform to the opposite value and asserts the other key hint appears. This would catch a regression that hardcodes the wrong key on the non-host platform.

Additionally, the "should render full text when committed and expanded" test (line 37) asserts content is shown but doesn't verify the collapse hint text (to collapse). The collapse interpolation at ConversationMessages.tsx:376 is untested in this direct unit test — adding expect(output).toContain(\${toggleKeyHint} to collapse`)` would close that gap.

— qwen3.7-max via Qwen Code /review


describe('<ThinkMessage />', () => {
const defaultProps = {
text: 'Analyzing the code structure',
Expand All @@ -19,7 +21,7 @@ describe('<ThinkMessage />', () => {
);
const output = lastFrame();
expect(output).toContain('Thinking');
expect(output).not.toContain('alt+t to expand');
expect(output).not.toContain(`${toggleKeyHint} to expand`);
});

it('should render collapsed line when committed and not expanded', () => {
Expand All @@ -28,7 +30,7 @@ describe('<ThinkMessage />', () => {
);
const output = lastFrame();
expect(output).toContain('Thinking');
expect(output).toContain('alt+t to expand');
expect(output).toContain(`${toggleKeyHint} to expand`);
expect(output).not.toContain('Analyzing the code structure');
});

Expand All @@ -45,7 +47,7 @@ describe('<ThinkMessage />', () => {
<ThinkMessage {...defaultProps} isPending={false} />,
);
const output = lastFrame();
expect(output).toContain('alt+t to expand');
expect(output).toContain(`${toggleKeyHint} to expand`);
expect(output).not.toContain('Analyzing the code structure');
});

Expand All @@ -61,7 +63,7 @@ describe('<ThinkMessage />', () => {
const output = lastFrame();
expect(output).toContain('Thought for');
expect(output).toContain('15s');
expect(output).toContain('alt+t to expand');
expect(output).toContain(`${toggleKeyHint} to expand`);
});

it('should show present-tense duration while pending (streaming)', () => {
Expand Down
7 changes: 5 additions & 2 deletions packages/cli/src/ui/components/messages/ConversationMessages.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import { formatDuration } from '../../utils/displayUtils.js';

export const THINKING_ICON = '∴ ';

const toggleKeyHint = process.platform === 'darwin' ? 'option+t' : 'alt+t';

@wenshao wenshao Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] toggleKeyHint is independently defined in 3 files — this source file plus both test files (ConversationMessages.test.tsx:10 and HistoryItemDisplay.test.tsx:32) each copy-paste the identical process.platform === 'darwin' ? 'option+t' : 'alt+t' expression. If the platform logic or label ever changes, it must be updated in 3 places, with silent divergence risk (a test file could pass on one platform and fail on another with a stale expectation).

Consider exporting the constant from this module (same pattern as THINKING_ICON) and importing it in the test files:

Suggested change
const toggleKeyHint = process.platform === 'darwin' ? 'option+t' : 'alt+t';
export const toggleKeyHint = process.platform === 'darwin' ? 'option+t' : 'alt+t';

Then in both test files:

import { toggleKeyHint } from './messages/ConversationMessages.js';

— qwen3.7-max via Qwen Code /review


interface UserMessageProps {
text: string;
}
Expand Down Expand Up @@ -329,7 +331,7 @@ export const ThinkMessage: React.FC<ThinkMessageProps> = ({
return (
<Text dimColor italic>
{THINKING_ICON}
{label} {t('(alt+t to expand)')}
{label} {t('({{keyHint}} to expand)', { keyHint: toggleKeyHint })}
</Text>
);
}
Expand Down Expand Up @@ -370,7 +372,8 @@ export const ThinkMessage: React.FC<ThinkMessageProps> = ({
<Box flexDirection="column">
<Text dimColor italic>
{THINKING_ICON}
{expandedLabel} {t('(alt+t to collapse)')}
{expandedLabel}{' '}
{t('({{keyHint}} to collapse)', { keyHint: toggleKeyHint })}
</Text>
<Box paddingLeft={2} flexDirection="column">
<MarkdownDisplay
Expand Down
Loading