fix: adapt keyboard shortcut display for macOS by Br1an67 · Pull Request #2484 · QwenLM/qwen-code

Br1an67 · 2026-03-19T05:50:51Z

TLDR

On macOS, keyboard shortcut hints now use native modifier symbols (⌃ for Ctrl, ⌘ for Cmd, ⌥ for Alt, ⇧ for Shift) instead of the generic text format.

Closes #2227

Dive Deeper

The CLI keyboard shortcut panel and error retry hints displayed shortcuts like ctrl+y, cmd+v regardless of platform. On macOS, the convention is to use symbolic modifiers (⌃Y, ⌘V).

Changes:

packages/cli/src/ui/utils/shortcutFormatter.ts — New formatShortcut() utility that converts modifier names to macOS symbols when process.platform === "darwin"
packages/cli/src/ui/components/KeyboardShortcuts.tsx — Apply formatShortcut() to shortcut key display
packages/cli/src/ui/hooks/useGeminiStream.ts — Apply formatShortcut() to "Press Ctrl+Y to retry" hint messages
packages/cli/src/ui/utils/shortcutFormatter.test.ts — Tests for Mac symbol conversion, multi-part shortcuts, and non-Mac passthrough

Non-macOS platforms are completely unaffected — the formatter is a no-op on Linux/Windows.

Reviewer Test Plan

Run on macOS — verify shortcut panel shows ⌃Y, ⌃C, ⌘V etc.
Trigger an API error — verify retry hint shows ⌃Y instead of Ctrl+Y
Run on Linux/Windows — verify shortcuts display unchanged (ctrl+y, ctrl+c)

Testing Matrix

	🍏	🪟	🐧
npm run	✅	❓	❓

Linked issues / bugs

Closes #2227

wenshao · 2026-04-18T03:05:31Z

@copilot resolve the merge conflicts in this pull request

wenshao

No issues found. LGTM! ✅ — gpt-5.4 via Qwen Code /review

Closes QwenLM#2227 On macOS, keyboard shortcut hints now use native modifier symbols (⌃ for Ctrl, ⌘ for Cmd, ⌥ for Alt, ⇧ for Shift) instead of the generic "ctrl+", "cmd+" text format. - Add formatShortcut() utility in shortcutFormatter.ts - Apply to KeyboardShortcuts panel and retry hint messages - Non-macOS platforms are unaffected

wenshao

No issues found. LGTM! ✅ — gpt-5.5 via Qwen Code /review

wenshao · 2026-04-27T09:35:10Z

@Br1an67 friendly ping — CI is currently red on all 9 Test (...) jobs, but the failures are not in the code this PR changes. They surface in packages/core/src/core/coreToolScheduler.test.ts with:

TypeError: this.toolRegistry.ensureTool is not a function

Root cause: the PR branch is based on main from 2026-04-18, and shortly after that point #3415 — test(core): update scheduler registry mock and a few related PRs (#3313, #3505) updated the scheduler tests to mock the new ensureTool API. Without those updates, the mock is missing the method and the test crashes.

Fix: rebase (or merge) latest main into this branch and push — no code changes needed on your side. Once CI is green I can take another look and merge.

Thanks for the contribution!

wenshao · 2026-04-28T01:18:42Z

Reviewed the change. Direction is correct (native symbols on Mac, full no-op elsewhere keeps the blast radius tiny), but a few points worth addressing:

Main issues

1. `.replace('Ctrl+Y', ...)` couples the formatter to translation content

const retryHint = t('Press Ctrl+Y to retry').replace('Ctrl+Y', retryKey);

I checked all 5 locales that define this key (en/zh/zh-TW/fr/ca) and every one happens to contain the literal substring Ctrl+Y, so today it works. But this turns "every translation must contain the exact substring Ctrl+Y" into an implicit contract — the moment a translator writes Ctrl-Y, control+y, or transliterates the modifier, .replace silently no-ops and the raw Ctrl+Y survives on screen.

Note also that de.js / ja.js / pt.js / ru.js don't have this key at all — they fall back to the English key, which coincidentally still contains Ctrl+Y. That's accidental, not designed.

A safer pattern is interpolation:

t('Press {shortcut} to retry', { shortcut: formatShortcut('ctrl+y') })

with the locale strings updated accordingly (e.g. '按 {shortcut} 重试。').

2. Column-width calculation in `KeyboardShortcuts.tsx` wasn't updated

getShortcutWidth still uses shortcut.key.length (KeyboardShortcuts.tsx:72), but rendering goes through formatShortcut(shortcut.key):

'ctrl+y'.length === 6,  '⌃Y'.length === 2

On Mac this overestimates widths, so a layout that could fit in 3 columns may get downgraded to 2. The display still looks correct, but part of the UX win this PR is trying to deliver is being eaten by stale layout math. Either run formatShortcut inside getShortcutWidth too, or precompute a displayKey field on the shortcut.

3. Module-level `isMac` + `vi.resetModules()` is unnecessarily complicated

const isMac = process.platform === 'darwin';   // captured once at import time

The test has to use vi.resetModules() + dynamic import() to flip platforms. Reading process.platform inside the function would let the test be a plain synchronous case. This isn't on a hot path so the perf difference is negligible.

4. Two unrelated comments deleted in `useGeminiStream.ts`

The diff drops // Store error with hint as a pending item (not in history). and // Store error with hint as a pending item (same as handleErrorEvent). They look like incidental losses while editing nearby lines — unrelated to this PR's purpose and worth restoring.

Minor suggestions

Mac convention for Tab is ⇥: the test asserts 'shift+tab' → '⇧TAB', but Apple HIG would be ⇧⇥. Same for Esc → ⎋ and Return → ⏎. If we're going for "native symbols," extending the map to cover tab/esc/return/enter would be a natural follow-up. Not a blocker.
Test coverage gaps: multi-modifier combos (ctrl+shift+a → ⌃⇧A), uppercase input (Ctrl+Y, CMD+V), single-symbol keys (!///@ — current toLowerCase→toUpperCase path returns them unchanged, but no assertion locks that in).

Not blocking

The MAC_MODIFIERS[lower] truthy check is fine here (all values are non-empty strings); no need for hasOwnProperty.
Non-Mac is a complete no-op, so regression risk is low.
getExternalEditorKey returning 'ctrl+x' on both branches is a pre-existing oddity, not this PR's concern.

I'd suggest prioritizing #1 (i18n contract) and #2 (width calc); the rest are suggestions.

Dismissing the prior auto-LGTM: CI is failing on all platforms (build error) and a fresh review surfaced i18n / layout-width / minor issues that need addressing.

wenshao · 2026-04-28T01:21:35Z

Two follow-ups on the review state:

Dismissed the prior auto-LGTM — the earlier /review bot approval ("No issues found. LGTM!") didn't catch the i18n / layout-width issues in the comment above, so I've dismissed it to reflect that the PR still has open comments to address.

CI is red, but it's not from this change. The failing test is packages/core/src/core/coreToolScheduler.test.ts with TypeError: this.toolRegistry.ensureTool is not a function. This PR only touches packages/cli/src/ui/.... ToolRegistry.ensureTool was added on main in #3297 and the scheduler mock was updated in #3415 — both landed after this branch's last CI run (2026-04-18). A rebase onto current main should clear it.

DragonnZhang

Downgraded from Approve to Comment: CI has test failures (pre-existing base branch issues in coreToolScheduler.test.ts, not caused by this PR).

[Suggestion] Help.tsx (lines 110-155) still renders shortcuts as hardcoded strings (Ctrl+C, Ctrl+L, Ctrl+O, Alt+Left/Right) without going through formatShortcut. On macOS, the bottom shortcut bar will show symbols like ⌃C, while the Help dialog shows Ctrl+C for the exact same shortcuts — a user-visible inconsistency.

Suggested fix: also route Help.tsx shortcut keys through formatShortcut().

Overall: clean implementation, no blocking issues. The i18n coupling and layout-width concerns previously raised by @wenshao remain valid and are not repeated here. — Qwen Code /review

DragonnZhang · 2026-06-05T08:16:05Z

+
+  return shortcut
+    .split(/\s+/)
+    .map((combo) => {


[Nice to have] combo.split('+') cannot represent the + key itself (e.g., ctrl++ for zoom-in would silently mangle). No current shortcut uses + as a key, but consider documenting this limitation or switching to a regex-based parser that matches known modifiers and leaves the remainder as the key.

— Qwen Code /review

DragonnZhang · 2026-06-05T08:16:05Z

+  it('converts modifier keys to Mac symbols on darwin', async () => {
+    Object.defineProperty(process, 'platform', { value: 'darwin' });
+    const { formatShortcut } = await import('./shortcutFormatter.js');
+    expect(formatShortcut('ctrl+y')).toBe('⌃Y');


[Nice to have] Consider adding test cases for combined modifiers (ctrl+shift+y → ⌃⇧Y), single-character keys (!, /, @), and mixed-case input (Ctrl+Y) to catch regressions if future shortcuts use these patterns.

— Qwen Code /review

Br1an67 requested review from DennisYu07, DragonnZhang, LaZzyMan, Mingholy, gwinthis, pomelo-nwu and tanzhenxin as code owners March 19, 2026 05:50

Br1an67 mentioned this pull request Mar 19, 2026

提示快捷键没有适配不同型号的电脑 #2227

Open

wenshao previously approved these changes Apr 18, 2026

View reviewed changes

wenshao dismissed their stale review via aa8428d April 18, 2026 03:11

wenshao force-pushed the fix/mac-keyboard-shortcut-display branch from bc3eb5e to aa8428d Compare April 18, 2026 03:11

BingqingLyu mentioned this pull request Apr 27, 2026

fix: adapt keyboard shortcut display for macOS BingqingLyu/qwen-code#55

Open

wenshao previously approved these changes Apr 27, 2026

View reviewed changes

DragonnZhang reviewed Jun 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: adapt keyboard shortcut display for macOS#2484

fix: adapt keyboard shortcut display for macOS#2484
Br1an67 wants to merge 1 commit into
QwenLM:mainfrom
Br1an67:fix/mac-keyboard-shortcut-display

Br1an67 commented Mar 19, 2026

Uh oh!

wenshao commented Apr 18, 2026

Uh oh!

wenshao left a comment

Uh oh!

wenshao left a comment

Uh oh!

wenshao commented Apr 27, 2026

Uh oh!

wenshao commented Apr 28, 2026 •

edited

Loading

Uh oh!

wenshao commented Apr 28, 2026

Uh oh!

DragonnZhang left a comment

Uh oh!

DragonnZhang Jun 5, 2026

Uh oh!

DragonnZhang Jun 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Br1an67 commented Mar 19, 2026

TLDR

Dive Deeper

Reviewer Test Plan

Testing Matrix

Linked issues / bugs

Uh oh!

wenshao commented Apr 18, 2026

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

wenshao left a comment

Choose a reason for hiding this comment

Uh oh!

wenshao commented Apr 27, 2026

Uh oh!

wenshao commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Main issues

1. .replace('Ctrl+Y', ...) couples the formatter to translation content

2. Column-width calculation in KeyboardShortcuts.tsx wasn't updated

3. Module-level isMac + vi.resetModules() is unnecessarily complicated

4. Two unrelated comments deleted in useGeminiStream.ts

Minor suggestions

Not blocking

Uh oh!

wenshao commented Apr 28, 2026

Uh oh!

DragonnZhang left a comment

Choose a reason for hiding this comment

Uh oh!

DragonnZhang Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

DragonnZhang Jun 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

wenshao commented Apr 28, 2026 •

edited

Loading

1. `.replace('Ctrl+Y', ...)` couples the formatter to translation content

2. Column-width calculation in `KeyboardShortcuts.tsx` wasn't updated

3. Module-level `isMac` + `vi.resetModules()` is unnecessarily complicated

4. Two unrelated comments deleted in `useGeminiStream.ts`