Add customizable Markdown preview keybindings - #4423
lawrencecchen wants to merge 16 commits into
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds focused-panel Markdown preview navigation and in-preview search: new shortcut actions allowing optional bare first strokes, recorder/settings wiring, chord-capable resolver, WKWebView interception forwarding commands to page JS, localized strings, docs/schema updates, and tests. ChangesMarkdown Preview Keyboard Shortcuts
Sequence Diagram(s)sequenceDiagram
participant User
participant WebView as MarkdownWebView
participant Coordinator as MarkdownWebRenderer.Coordinator
participant Resolver as MarkdownPreviewKeyboardShortcutResolver
participant JS as window.__cmuxMarkdownPreviewHandleKeyCommand
User->>WebView: Press key
WebView->>Coordinator: onKeyboardShortcut(event)
alt Chord first stroke detected
Coordinator->>Coordinator: Buffer pendingShortcutChord
Coordinator-->>WebView: Return handled (true)
else Single stroke or chord second stroke
Coordinator->>Resolver: Resolve command(for:event, pendingFirstStroke)
Resolver-->>Coordinator: MarkdownPreviewKeyCommand
Coordinator->>JS: Forward command via JS invocation
JS->>JS: Perform scroll/find action in page
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (1 error, 1 warning)
✅ Passed checks (15 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR adds panel-local Markdown Preview keyboard shortcuts (Vim-style
Confidence Score: 4/5Safe to merge with the understanding that open localization gaps (already tracked in prior review threads) will be addressed as follow-up; the shortcut dispatch guards are solid and the default keybindings will not affect terminal or browser panels. The new dispatch pipeline is well-guarded and the test coverage is thorough. The open items from prior review threads — missing xcstrings translations across 17 locales, the chord cancellation dual-dispatch edge case for custom chord users, and the find-shortcut being silently consumed before the viewer is ready — remain unresolved in this diff and affect production user experience. Resources/Localizable.xcstrings (13 new keys, only en/ja translations) and Sources/Panels/MarkdownWebSupport.swift (doCommand override and hardcoded keyCode interception for Ctrl+P/N). Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AppKit
participant AppDelegate
participant MarkdownWebView
participant Coordinator
participant JS as shell.html JS
User->>AppKit: Key event
AppKit->>AppDelegate: sendEvent(_:)
AppDelegate->>AppDelegate: handleFocusedMarkdownPreviewShortcut
Note over AppDelegate: Guards: main window, no chord prefix,<br/>no command palette/terminal/text input
alt "Markdown panel focused & preview mode"
AppDelegate->>Coordinator: handleKeyboardShortcut(event)
alt Pending chord
Coordinator->>Coordinator: clear pendingShortcutChord
Coordinator->>Coordinator: resolve second stroke
else Single-stroke or chord prefix
Coordinator->>Coordinator: resolve command / set pendingShortcutChord
end
Coordinator->>Coordinator: performKeyboardCommand(command)
Coordinator->>JS: evaluateJavaScript(__cmuxMarkdownPreviewHandleKeyCommand)
JS->>JS: scroll / window.find / window.prompt
JS-->>AppKit: WKUIDelegate: runJavaScriptTextInputPanel
AppKit-->>User: NSAlert search dialog
AppDelegate-->>AppKit: return true (event consumed)
else Fallback: MarkdownWebView is first responder
AppKit->>MarkdownWebView: performKeyEquivalent / keyDown / doCommand
MarkdownWebView->>Coordinator: onKeyboardShortcut(event)
Coordinator->>JS: evaluateJavaScript(...)
MarkdownWebView-->>AppKit: return true
else Non-markdown / browser
AppDelegate->>AppDelegate: matchConfiguredShortcut(.find/.findNext/...)
AppDelegate->>AppDelegate: tabManager.findNext() → focusedBrowserPanel
end
Reviews (15): Last reviewed commit: "Tighten markdown preview shortcut dispat..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| "surfaces": "Surfaces", | ||
| "surfacesBlurb": "Surfaces are tabs inside a pane.", | ||
| "markdownPreview": "Markdown Preview", | ||
| "markdownPreviewBlurb": "Markdown preview shortcuts are local to the focused Markdown panel, so bare Vim-style keys do not affect terminals.", |
There was a problem hiding this comment.
The new markdownPreview and markdownPreviewBlurb message keys are only present in en.json and ja.json. web/i18n/routing.ts defines 20 supported locales (en, ja, zh-CN, zh-TW, ko, de, es, fr, it, da, pl, ru, bs, ar, no, pt-BR, th, tr, km, uk), so 18 locale files — ar.json, zh-CN.json, ko.json, de.json, and 14 others — are not updated. Users in those locales will silently receive the English fallback for the Markdown Preview section heading and blurb on the keyboard-shortcuts page.
Rule Used: Flag production user-facing text that is not fully... (source)
| "shortcutFirstStroke": { | ||
| "allOf": [ | ||
| { | ||
| "$ref": "#/$defs/shortcutStroke" | ||
| }, | ||
| { | ||
| "anyOf": [ | ||
| { | ||
| "pattern": "^(?:(?:[cC][mM][dD]|[cC][oO][mM][mM][aA][nN][dD]|[sS][hH][iI][fF][tT]|[oO][pP][tT]|[oO][pP][tT][iI][oO][nN]|[aA][lL][tT]|[cC][tT][rR][lL]|[cC][oO][nN][tT][rR][oO][lL]|[cC][tT][lL]|⌘|⇧|⌥|⌃)\\+)+" | ||
| }, | ||
| { | ||
| "pattern": "^(?: |[sS][pP][aA][cC][eE]|[sS][pP][aA][cC][eE][bB][aA][rR]|<[sS][pP][aA][cC][eE]>)$" | ||
| } | ||
| ] | ||
| } | ||
| ], | ||
| "description": "First or only shortcut stroke. Must include a modifier unless the key is Space." | ||
| "$ref": "#/$defs/shortcutStroke", | ||
| "description": "First or only shortcut stroke. Most actions require a modifier unless the key is Space. Markdown preview actions also accept bare first strokes such as h, j, k, l, /, and n." | ||
| }, |
There was a problem hiding this comment.
The previous shortcutFirstStroke definition enforced that a modifier key (or Space) must be present via an anyOf pattern constraint. Removing it means JSON-schema-aware editors and validators will accept bare-key first strokes for any action — including actions like toggleSidebar or openBrowser where the Swift runtime will still reject bare keys at load time. Users misconfiguring non-markdown actions with bare keys will get no schema feedback, only a silent runtime rejection.
| "shortcutFirstStroke": { | |
| "allOf": [ | |
| { | |
| "$ref": "#/$defs/shortcutStroke" | |
| }, | |
| { | |
| "anyOf": [ | |
| { | |
| "pattern": "^(?:(?:[cC][mM][dD]|[cC][oO][mM][mM][aA][nN][dD]|[sS][hH][iI][fF][tT]|[oO][pP][tT]|[oO][pP][tT][iI][oO][nN]|[aA][lL][tT]|[cC][tT][rR][lL]|[cC][oO][nN][tT][rR][oO][lL]|[cC][tT][lL]|⌘|⇧|⌥|⌃)\\+)+" | |
| }, | |
| { | |
| "pattern": "^(?: |[sS][pP][aA][cC][eE]|[sS][pP][aA][cC][eE][bB][aA][rR]|<[sS][pP][aA][cC][eE]>)$" | |
| } | |
| ] | |
| } | |
| ], | |
| "description": "First or only shortcut stroke. Must include a modifier unless the key is Space." | |
| "$ref": "#/$defs/shortcutStroke", | |
| "description": "First or only shortcut stroke. Most actions require a modifier unless the key is Space. Markdown preview actions also accept bare first strokes such as h, j, k, l, /, and n." | |
| }, | |
| "shortcutFirstStroke": { | |
| "oneOf": [ | |
| { | |
| "allOf": [ | |
| { "$ref": "#/$defs/shortcutStroke" }, | |
| { | |
| "anyOf": [ | |
| { "pattern": "^(?:(?:[cC][mM][dD]|[cC][oO][mM][mM][aA][nN][dD]|[sS][hH][iI][fF][tT]|[oO][pP][tT]|[oO][pP][tT][iI][oO][nN]|[aA][lL][tT]|[cC][tT][rR][lL]|[cC][oO][nN][tT][rR][oO][lL]|[cC][tT][lL]|⌘|⇧|⌥|⌃)\\+)+" }, | |
| { "pattern": "^(?: |[sS][pP][aA][cC][eE]|[sS][pP][aA][cC][eE][bB][aA][rR]|<[sS][pP][aA][cC][eE]>)$" } | |
| ] | |
| } | |
| ], | |
| "description": "First stroke with a required modifier (or Space). Used by most actions." | |
| }, | |
| { | |
| "$ref": "#/$defs/shortcutStroke", | |
| "description": "Bare first stroke accepted only for Markdown preview actions (e.g. h, j, k, l, /, n)." | |
| } | |
| ], | |
| "description": "First or only shortcut stroke. Most actions require a modifier unless the key is Space. Markdown preview actions also accept bare first strokes such as h, j, k, l, /, and n." | |
| }, |
| onPointerDown?() | ||
| super.mouseDown(with: event) | ||
| } | ||
|
|
||
| override func keyDown(with event: NSEvent) { | ||
| if onKeyboardShortcut?(event) == true { | ||
| return | ||
| } | ||
| super.keyDown(with: event) | ||
| } | ||
|
|
||
| override func performKeyEquivalent(with event: NSEvent) -> Bool { | ||
| if onKeyboardShortcut?(event) == true { | ||
| return true | ||
| } | ||
| return super.performKeyEquivalent(with: event) |
There was a problem hiding this comment.
Both performKeyEquivalent and keyDown invoke onKeyboardShortcut. When a chord prefix (e.g., a user-configured ctrl+b) has been captured and the user then presses a key with modifier flags (e.g., ctrl+d), performKeyEquivalent is called first: handleKeyboardShortcut enters the pending-chord branch, the second stroke fails to match, the defer clears pendingShortcutChord, and the method returns false. AppKit then calls keyDown with the same event. With pendingShortcutChord now nil, handleKeyboardShortcut re-evaluates the event as a fresh single-stroke and may match markdownPageDown (⌃D), firing an unintended command. This only affects users who configure chord-based markdown shortcuts; the shipping defaults are all single-stroke.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/KeyboardShortcutSettings.swift (1)
2331-2348:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSupport literal
?and uppercase single-key tokens incmux.json.Line 2331 routes Markdown preview overrides through
parseConfig, butShortcutStroke.parseConfig(_:)still treats"N"as plainnand"?"as a literal?key with noshift. That makes the obvious Vim-style config values either wrong or unmappable, so the new settings-file customization path cannot faithfully express?/N.💡 Proposed fix
extension ShortcutStroke { static func parseConfig(_ rawValue: String) -> ShortcutStroke? { guard !rawValue.isEmpty else { return nil } let rawParts = rawValue.split(separator: "+", omittingEmptySubsequences: false) .map(String.init) let parts = rawParts.map { $0.trimmingCharacters(in: .whitespacesAndNewlines) } guard !parts.isEmpty, let lastRawPart = rawParts.last, !lastRawPart.isEmpty else { return nil } var command = false var shift = false var option = false var control = false for modifier in parts.dropLast() { switch modifier.lowercased() { case "cmd", "command", "⌘": command = true case "shift", "⇧": shift = true case "opt", "option", "alt", "⌥": option = true case "ctrl", "control", "ctl", "⌃": control = true default: return nil } } - guard let key = parseConfigKeyToken(lastRawPart) else { return nil } + let trimmedKeyToken = lastRawPart.trimmingCharacters(in: .whitespacesAndNewlines) + guard let parsedKey = parseConfigKeyToken(lastRawPart) else { return nil } + + let inferredShiftFromUppercase = + trimmedKeyToken.count == 1 && + trimmedKeyToken.lowercased() != trimmedKeyToken && + trimmedKeyToken.uppercased() == trimmedKeyToken + + let inferredShiftFromSymbol = trimmedKeyToken == "?" + let key = trimmedKeyToken == "?" ? "/" : parsedKey + return ShortcutStroke( key: key, command: command, - shift: shift, + shift: shift || inferredShiftFromUppercase || inferredShiftFromSymbol, option: option, control: control ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/KeyboardShortcutSettings.swift` around lines 2331 - 2348, The parser currently normalizes tokens so uppercase letters and shift-required symbols (like "?") lose their shift semantics; update ShortcutStroke.parseConfig(_:) to detect single-character tokens: if the token is an uppercase ASCII letter, set the key to the lowercase character and add the Shift modifier to modifierFlags; if the token is a symbol that normally requires Shift (e.g., "?"), set the key to the unshifted equivalent ("/") or represent the symbol as the key while adding Shift to modifierFlags so the literal "?" is preserved as a shifted stroke; ensure StoredShortcut.parseConfig(strokes:allowBareFirstStroke:) continues to call ShortcutStroke.parseConfig(_:) and relies on the updated modifierFlags behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 208: The table row showing the search shortcuts is ambiguous because it
repeats "/" as both the shortcut and the column separator; update the cell
content string currently showing "| / / ⇧ / | Search forward/backward |" to use
the Vim convention for backward search by replacing the middle "/" with "?" so
it reads something like "| / ? ⇧ / | Search forward/backward |", ensuring the
forward search symbol "/" and backward search symbol "?" (Shift+/) are clearly
indicated.
In `@Resources/Localizable.xcstrings`:
- Around line 113626-113709: The new localization keys (e.g.,
"shortcut.markdownScrollLeft.label", "shortcut.markdownScrollDown.label",
"shortcut.markdownScrollUp.label", "shortcut.markdownScrollRight.label",
"shortcut.markdownPageUp.label", "shortcut.markdownPageDown.label",
"shortcut.markdownFindForward.label", "shortcut.markdownFindBackward.label",
"shortcut.markdownFindNext.label", "shortcut.markdownFindPrevious.label",
"markdown.web.searchPromptForward", "markdown.web.searchPromptBackward") were
added only for en and ja; update each of these entries to include translated
values for every locale already present in Resources/Localizable.xcstrings
(following the repository’s fallback convention where applicable) so the catalog
has full locale coverage and no partial localizations for production-facing
strings.
In `@web/app/`[locale]/docs/configuration/page.tsx:
- Around line 371-372: The new English copy in
web/app/[locale]/docs/configuration/page.tsx (the inline sentence starting with
"Bare first strokes...") must be moved into next-intl messages: replace the
hard-coded text with a t('docs.configuration.bareStrokes') (or similar unique
key) call in the page component, add that key to web/i18n/routing.ts for this
route, and add corresponding entries in each web/messages/* locale file
(including the English file) so all locales are covered; ensure the key name
matches across the page, routing, and every messages file.
In `@web/data/cmux.schema.json`:
- Around line 936-937: The new English description added to the schema under
"$ref": "`#/`$defs/shortcutStroke" is user-facing and must be localized; remove or
replace the hard-coded description text in the schema and instead reference a
localization key (e.g., shortcutStroke.description) that pulls from the i18n
message bundle, then add the corresponding entries to every locale in
web/messages/ and update web/i18n/routing.ts if needed to include the key —
locate the description in cmux.schema.json (the $defs/shortcutStroke entry) and
ensure the schema only references the i18n key rather than embedding English
text.
---
Outside diff comments:
In `@Sources/KeyboardShortcutSettings.swift`:
- Around line 2331-2348: The parser currently normalizes tokens so uppercase
letters and shift-required symbols (like "?") lose their shift semantics; update
ShortcutStroke.parseConfig(_:) to detect single-character tokens: if the token
is an uppercase ASCII letter, set the key to the lowercase character and add the
Shift modifier to modifierFlags; if the token is a symbol that normally requires
Shift (e.g., "?"), set the key to the unshifted equivalent ("/") or represent
the symbol as the key while adding Shift to modifierFlags so the literal "?" is
preserved as a shifted stroke; ensure
StoredShortcut.parseConfig(strokes:allowBareFirstStroke:) continues to call
ShortcutStroke.parseConfig(_:) and relies on the updated modifierFlags behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2c3c940e-b13f-44da-9dc3-136c036b97cc
📒 Files selected for processing (21)
README.mdResources/Localizable.xcstringsResources/markdown-viewer/shell.htmlSources/AppDelegate.swiftSources/KeyboardShortcutContext.swiftSources/KeyboardShortcutRecorder.swiftSources/KeyboardShortcutSettings.swiftSources/KeyboardShortcutSettingsControls.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/Panels/MarkdownPreviewKeyboardShortcuts.swiftSources/Panels/MarkdownViewerAssets.swiftSources/Panels/MarkdownWebRenderer.swiftSources/Panels/MarkdownWebSupport.swiftcmux.xcodeproj/project.pbxprojcmuxTests/MarkdownPanelTests.swiftweb/app/[locale]/docs/configuration/page.tsxweb/app/[locale]/docs/keyboard-shortcuts/page.tsxweb/data/cmux-shortcuts.tsweb/data/cmux.schema.jsonweb/messages/en.jsonweb/messages/ja.json
| <code>9</code>. Bare first strokes such as <code>j</code> and <code>/</code> are accepted | ||
| for Markdown preview actions because they are only active inside the focused Markdown panel. |
There was a problem hiding this comment.
Move new Markdown bare-stroke explanation to next-intl messages.
Lines 371-372 introduce new user-facing English copy in a [locale] docs page; this should be translated message content (e.g., t(...)) instead of inline text.
As per coding guidelines: “Web UI/copy… must be consumed from next-intl… and represented across all locales in web/i18n/routing.ts and every matching file in web/messages/.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/app/`[locale]/docs/configuration/page.tsx around lines 371 - 372, The new
English copy in web/app/[locale]/docs/configuration/page.tsx (the inline
sentence starting with "Bare first strokes...") must be moved into next-intl
messages: replace the hard-coded text with a t('docs.configuration.bareStrokes')
(or similar unique key) call in the page component, add that key to
web/i18n/routing.ts for this route, and add corresponding entries in each
web/messages/* locale file (including the English file) so all locales are
covered; ensure the key name matches across the page, routing, and every
messages file.
| "$ref": "#/$defs/shortcutStroke", | ||
| "description": "First or only shortcut stroke. Most actions require a modifier unless the key is Space. Markdown preview actions also accept bare first strokes such as h, j, k, l, /, and n." |
There was a problem hiding this comment.
Localize or relocate new schema description copy before exposing it in localized docs.
Line 937 adds new user-facing English text in schema metadata, and this description is rendered directly in locale docs; that bypasses per-locale translations.
As per coding guidelines: “Flag any user-facing text that isn’t fully localized across all supported locales… and update every locale listed in web/i18n/routing.ts with matching web/messages/ entries.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/data/cmux.schema.json` around lines 936 - 937, The new English
description added to the schema under "$ref": "`#/`$defs/shortcutStroke" is
user-facing and must be localized; remove or replace the hard-coded description
text in the schema and instead reference a localization key (e.g.,
shortcutStroke.description) that pulls from the i18n message bundle, then add
the corresponding entries to every locale in web/messages/ and update
web/i18n/routing.ts if needed to include the key — locate the description in
cmux.schema.json (the $defs/shortcutStroke entry) and ensure the schema only
references the i18n key rather than embedding English text.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 21 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="web/messages/en.json">
<violation number="1" location="web/messages/en.json:553">
P1: New i18n keys `markdownPreview` and `markdownPreviewBlurb` are missing from 18 of 20 supported locale files.</violation>
</file>
<file name="Resources/markdown-viewer/shell.html">
<violation number="1" location="Resources/markdown-viewer/shell.html:1459">
P1: The new markdown find-forward/find-backward commands call `window.prompt`, but the markdown WKWebView delegate does not implement JavaScript text-input handling, so prompt behaves like Cancel and the search command never starts.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "workspacesBlurb": "Workspaces live in the sidebar. Each workspace has its own set of panes and surfaces.", | ||
| "surfaces": "Surfaces", | ||
| "surfacesBlurb": "Surfaces are tabs inside a pane.", | ||
| "markdownPreview": "Markdown Preview", |
There was a problem hiding this comment.
P1: New i18n keys markdownPreview and markdownPreviewBlurb are missing from 18 of 20 supported locale files.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At web/messages/en.json, line 553:
<comment>New i18n keys `markdownPreview` and `markdownPreviewBlurb` are missing from 18 of 20 supported locale files.</comment>
<file context>
@@ -550,6 +550,8 @@
"workspacesBlurb": "Workspaces live in the sidebar. Each workspace has its own set of panes and surfaces.",
"surfaces": "Surfaces",
"surfacesBlurb": "Surfaces are tabs inside a pane.",
+ "markdownPreview": "Markdown Preview",
+ "markdownPreviewBlurb": "Markdown preview shortcuts are local to the focused Markdown panel, so bare Vim-style keys do not affect terminals.",
"splitPanes": "Split Panes",
</file context>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/Panels/MarkdownWebRenderer.swift (1)
293-301:⚠️ Potential issue | 🟠 Major | ⚡ Quick winConsume Escape when cancelling a pending preview chord.
After a first chord stroke is armed, Line 300 returns
falseforShortcutStroke.isEscapeCancelEvent(event). That clearspendingShortcutChord, but still lets Escape fall through to app-level handlers, so a local chord cancel can trigger unrelated global Escape behavior.Suggested fix
guard let command = MarkdownPreviewKeyboardShortcutResolver.command( for: event, pendingFirstStroke: pendingShortcutChord ) else { - return !ShortcutStroke.isEscapeCancelEvent(event) + return true }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/Panels/MarkdownWebRenderer.swift` around lines 293 - 301, In handleKeyboardShortcut, when a pendingShortcutChord exists you currently clear pendingShortcutChord and, if MarkdownPreviewKeyboardShortcutResolver.command(...) returns nil, return !ShortcutStroke.isEscapeCancelEvent(event) which lets Escape propagate; change this so that after clearing pendingShortcutChord you still detect a cancel Escape via ShortcutStroke.isEscapeCancelEvent(event) and return true to consume it (otherwise return false for non-Escape events). Keep the call to MarkdownPreviewKeyboardShortcutResolver.command(...) and the pendingShortcutChord clearing logic (function: handleKeyboardShortcut, symbol: pendingShortcutChord, helper: ShortcutStroke.isEscapeCancelEvent, resolver: MarkdownPreviewKeyboardShortcutResolver.command).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/Panels/MarkdownPreviewKeyboardShortcuts.swift`:
- Around line 43-45: configuredCommandActions currently mixes markdown-local
actions with globalFindCommandActions causing global find shortcuts to start
local chord prefixes; change the chord-matching to use only the markdown-local
list (commandActions) instead of configuredCommandActions: keep
configuredCommandActions as the combined list for single-stroke resolution but
update chordPrefix(...) and chordCommand(...) to iterate over commandActions (or
a new local variable like localChordActions = commandActions) so
globalFindCommandActions are excluded from prefix/chord matching; apply the same
fix wherever chordPrefix/chordCommand currently iterate the combined list.
---
Outside diff comments:
In `@Sources/Panels/MarkdownWebRenderer.swift`:
- Around line 293-301: In handleKeyboardShortcut, when a pendingShortcutChord
exists you currently clear pendingShortcutChord and, if
MarkdownPreviewKeyboardShortcutResolver.command(...) returns nil, return
!ShortcutStroke.isEscapeCancelEvent(event) which lets Escape propagate; change
this so that after clearing pendingShortcutChord you still detect a cancel
Escape via ShortcutStroke.isEscapeCancelEvent(event) and return true to consume
it (otherwise return false for non-Escape events). Keep the call to
MarkdownPreviewKeyboardShortcutResolver.command(...) and the
pendingShortcutChord clearing logic (function: handleKeyboardShortcut, symbol:
pendingShortcutChord, helper: ShortcutStroke.isEscapeCancelEvent, resolver:
MarkdownPreviewKeyboardShortcutResolver.command).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 63b8aea9-e9ac-436e-a3c3-a04e004d9474
📒 Files selected for processing (9)
README.mdSources/AppDelegate.swiftSources/Panels/MarkdownPanel.swiftSources/Panels/MarkdownPreviewKeyboardShortcuts.swiftSources/Panels/MarkdownWebRenderer.swiftSources/Panels/MarkdownWebSupport.swiftcmuxTests/AppDelegateShortcutRoutingTests.swiftcmuxTests/MarkdownPanelTests.swiftweb/data/cmux-shortcuts.ts
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Sources/AppDelegate.swift">
<violation number="1" location="Sources/AppDelegate.swift:14180">
P2: Markdown preview shortcuts are routed without verifying the first responder belongs to the markdown preview, so keys can be consumed while another input is focused.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmuxTests/MarkdownPanelTests.swift (1)
413-417:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThis test doesn't actually cover the chord-miss fallback.
Line 417 uses
j, butjis already a valid standalone markdown-preview shortcut. That means this still returnstrueeven if the pendingctrl+bchord is not cleared correctly, so the test can pass while the "miss should fall through" contract is broken. Use an unbound second key and assertfalse, then optionally sendjafterward to prove the miss cleared the pending chord.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmuxTests/MarkdownPanelTests.swift` around lines 413 - 417, The test currently uses a standalone shortcut ("j") which masks whether a pending chord from MarkdownWebRenderer.Coordinator.handleKeyboardShortcut(try keyDownEvent("b", ...)) is cleared; change the second keyDownEvent call to an unbound key (e.g., use a character not mapped to any shortcut via keyDownEvent("x", ...)) and assert that coordinator.handleKeyboardShortcut(...) returns false to verify the chord miss falls through, then (optionally) send the original bound keyDownEvent("j", ...) and assert true to prove the pending chord was cleared.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Resources/markdown-viewer/shell.html`:
- Around line 1513-1564: The code currently hardcodes default key mappings
inside markdownPreviewCommandForKeyboardEvent (used by document.addEventListener
and invoking window.__cmuxMarkdownPreviewHandleKeyCommand), which bypasses user
remaps; change it to delegate shortcut resolution to the configurable resolver
instead of in-page logic: remove the hardcoded mapping switch/if blocks in
markdownPreviewCommandForKeyboardEvent and instead call the existing resolver
API (e.g., a function exposed on window or the settings-backed resolver) with
the event/key/modifier info and use its returned command (so
markdownPreviewShouldHandleKeyboardEvent still guards events but all mapping
logic lives in the resolver); ensure document.addEventListener uses that
resolver result and no longer contains built-in defaults.
In `@Sources/Panels/MarkdownWebSupport.swift`:
- Around line 48-55: The override of doCommand(by:) must not call
super.doCommand(by:) due to the prohibited_super_call rule; instead, when the
keyboard shortcut check falls through, forward the command along the responder
chain (e.g., call nextResponder?.doCommand(by: selector) or the project-approved
fallback helper) rather than super.doCommand(by:). Update the method containing
Self.isControlNavigationCommand(...), onKeyboardShortcut?(event) and the current
super.doCommand(by: selector) call to use nextResponder?.doCommand(by: selector)
or the repository's documented fallback function per the cmux lint rules in
.github/review-bot-rules/.
---
Outside diff comments:
In `@cmuxTests/MarkdownPanelTests.swift`:
- Around line 413-417: The test currently uses a standalone shortcut ("j") which
masks whether a pending chord from
MarkdownWebRenderer.Coordinator.handleKeyboardShortcut(try keyDownEvent("b",
...)) is cleared; change the second keyDownEvent call to an unbound key (e.g.,
use a character not mapped to any shortcut via keyDownEvent("x", ...)) and
assert that coordinator.handleKeyboardShortcut(...) returns false to verify the
chord miss falls through, then (optionally) send the original bound
keyDownEvent("j", ...) and assert true to prove the pending chord was cleared.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: e8004286-3195-4e6b-8e48-589b5ec05e82
📒 Files selected for processing (8)
Resources/markdown-viewer/shell.htmlSources/AppDelegate.swiftSources/Panels/MarkdownPanel.swiftSources/Panels/MarkdownWebRenderer.swiftSources/Panels/MarkdownWebSupport.swiftSources/TabManager.swiftcmuxTests/AppDelegateShortcutRoutingTests.swiftcmuxTests/MarkdownPanelTests.swift
| function markdownPreviewShouldHandleKeyboardEvent(ev) { | ||
| if (!ev || ev.defaultPrevented || ev.isComposing) { return false; } | ||
| var target = ev.target; | ||
| if (target && target.closest && target.closest('input, textarea, select, [contenteditable=""], [contenteditable="true"]')) { | ||
| return false; | ||
| } | ||
| return true; | ||
| } | ||
|
|
||
| function markdownPreviewCommandForKeyboardEvent(ev) { | ||
| if (!markdownPreviewShouldHandleKeyboardEvent(ev)) { return null; } | ||
| var key = String(ev.key || '').toLowerCase(); | ||
| var hasCommand = !!ev.metaKey; | ||
| var hasControl = !!ev.ctrlKey; | ||
| var hasOption = !!ev.altKey; | ||
| var hasShift = !!ev.shiftKey; | ||
|
|
||
| if (hasCommand && !hasControl && !hasOption && !hasShift && key === 'f') { return 'findForward'; } | ||
| if (hasCommand && !hasControl && !hasOption && !hasShift && key === 'g') { return 'findNext'; } | ||
| if (hasCommand && !hasControl && hasOption && !hasShift && key === 'g') { return 'findPrevious'; } | ||
| if (!hasCommand && hasControl && !hasOption && !hasShift && key === 'n') { return 'findNext'; } | ||
| if (!hasCommand && hasControl && !hasOption && !hasShift && key === 'p') { return 'findPrevious'; } | ||
| if (!hasCommand && hasControl && !hasOption && !hasShift && key === 'u') { return 'pageUp'; } | ||
| if (!hasCommand && hasControl && !hasOption && !hasShift && key === 'd') { return 'pageDown'; } | ||
| if (!hasCommand && !hasControl && !hasOption && !hasShift) { | ||
| switch (key) { | ||
| case 'h': return 'scrollLeft'; | ||
| case 'j': return 'scrollDown'; | ||
| case 'k': return 'scrollUp'; | ||
| case 'l': return 'scrollRight'; | ||
| case '/': return 'findForward'; | ||
| case 'n': return 'findNext'; | ||
| default: break; | ||
| } | ||
| } | ||
| if (!hasCommand && !hasControl && !hasOption && hasShift) { | ||
| switch (key) { | ||
| case '?': return 'findBackward'; | ||
| case 'n': return 'findPrevious'; | ||
| default: break; | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| document.addEventListener('keydown', function(ev) { | ||
| var command = markdownPreviewCommandForKeyboardEvent(ev); | ||
| if (!command) { return; } | ||
| ev.preventDefault(); | ||
| ev.stopPropagation(); | ||
| window.__cmuxMarkdownPreviewHandleKeyCommand(command); | ||
| }, true); |
There was a problem hiding this comment.
Keep shortcut resolution in the configurable resolver, not in the page.
This re-encodes the default preview bindings a second time. If a user remaps or removes h/j/k/l, /, n/N, Cmd-F, Cmd-G, Ctrl-N/P, or Ctrl-U/D, Line 1558 will still consume the old defaults whenever the event reaches the DOM, so the new Settings/cmux.json customization does not actually take effect in the preview.
Suggested direction
- function markdownPreviewShouldHandleKeyboardEvent(ev) {
- if (!ev || ev.defaultPrevented || ev.isComposing) { return false; }
- var target = ev.target;
- if (target && target.closest && target.closest('input, textarea, select, [contenteditable=""], [contenteditable="true"]')) {
- return false;
- }
- return true;
- }
-
- function markdownPreviewCommandForKeyboardEvent(ev) {
- if (!markdownPreviewShouldHandleKeyboardEvent(ev)) { return null; }
- ...
- }
-
- document.addEventListener('keydown', function(ev) {
- var command = markdownPreviewCommandForKeyboardEvent(ev);
- if (!command) { return; }
- ev.preventDefault();
- ev.stopPropagation();
- window.__cmuxMarkdownPreviewHandleKeyCommand(command);
- }, true);
+ // Keep only the semantic command handler here.
+ // Let the native markdown-preview shortcut resolver decide which
+ // keys/chords are active so user overrides and disabled bindings
+ // are honored consistently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Resources/markdown-viewer/shell.html` around lines 1513 - 1564, The code
currently hardcodes default key mappings inside
markdownPreviewCommandForKeyboardEvent (used by document.addEventListener and
invoking window.__cmuxMarkdownPreviewHandleKeyCommand), which bypasses user
remaps; change it to delegate shortcut resolution to the configurable resolver
instead of in-page logic: remove the hardcoded mapping switch/if blocks in
markdownPreviewCommandForKeyboardEvent and instead call the existing resolver
API (e.g., a function exposed on window or the settings-backed resolver) with
the event/key/modifier info and use its returned command (so
markdownPreviewShouldHandleKeyboardEvent still guards events but all mapping
logic lives in the resolver); ensure document.addEventListener uses that
resolver result and no longer contains built-in defaults.
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="Resources/markdown-viewer/shell.html">
<violation number="1" location="Resources/markdown-viewer/shell.html:1433">
P2: Per-keystroke scrolling uses smooth animation, which can drop/flatten repeated scroll steps when keys are pressed quickly.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| var nextLeft = Math.max(0, (window.scrollX || scroller.scrollLeft || 0) + deltaX); | ||
| var nextTop = clampScrollY((window.scrollY || scroller.scrollTop || 0) + deltaY); | ||
| try { | ||
| window.scrollTo({ left: nextLeft, top: nextTop, behavior: 'smooth' }); |
There was a problem hiding this comment.
P2: Per-keystroke scrolling uses smooth animation, which can drop/flatten repeated scroll steps when keys are pressed quickly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Resources/markdown-viewer/shell.html, line 1433:
<comment>Per-keystroke scrolling uses smooth animation, which can drop/flatten repeated scroll steps when keys are pressed quickly.</comment>
<file context>
@@ -1429,16 +1429,11 @@
- var previousBehavior = root.style.scrollBehavior;
- root.style.scrollBehavior = 'auto';
try {
+ window.scrollTo({ left: nextLeft, top: nextTop, behavior: 'smooth' });
+ } catch (e) {
window.scrollTo(nextLeft, nextTop);
</file context>
| window.scrollTo({ left: nextLeft, top: nextTop, behavior: 'smooth' }); | |
| window.scrollTo({ left: nextLeft, top: nextTop, behavior: 'auto' }); |
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="web/messages/en.json">
<violation number="1" location="web/messages/en.json:553">
P1: New i18n keys `markdownPreview` and `markdownPreviewBlurb` are missing from 18 of 20 supported locale files.</violation>
</file>
<file name="Resources/markdown-viewer/shell.html">
<violation number="1" location="Resources/markdown-viewer/shell.html:1433">
P2: Per-keystroke scrolling uses smooth animation, which can drop/flatten repeated scroll steps when keys are pressed quickly.</violation>
</file>
<file name="Sources/Panels/MarkdownWebRenderer.swift">
<violation number="1" location="Sources/Panels/MarkdownWebRenderer.swift:423">
P2: Add full locale coverage for `markdown.preview.dialog.title` in `Localizable.xcstrings`; it currently has only `en` and `ja` translations.
(Based on your team's feedback about Localizable.xcstrings locale coverage.) [FEEDBACK_USED]</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| if !title.isEmpty { | ||
| return title | ||
| } | ||
| return String(localized: "markdown.preview.dialog.title", defaultValue: "Markdown Preview") |
There was a problem hiding this comment.
P2: Add full locale coverage for markdown.preview.dialog.title in Localizable.xcstrings; it currently has only en and ja translations.
(Based on your team's feedback about Localizable.xcstrings locale coverage.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/Panels/MarkdownWebRenderer.swift, line 423:
<comment>Add full locale coverage for `markdown.preview.dialog.title` in `Localizable.xcstrings`; it currently has only `en` and `ja` translations.
(Based on your team's feedback about Localizable.xcstrings locale coverage.) </comment>
<file context>
@@ -384,6 +384,59 @@ struct MarkdownWebRenderer: NSViewRepresentable {
+ if !title.isEmpty {
+ return title
+ }
+ return String(localized: "markdown.preview.dialog.title", defaultValue: "Markdown Preview")
+ }
+
</file context>
| } | ||
| } | ||
| }, | ||
| "markdown.preview.dialog.title": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Markdown Preview" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Markdown プレビュー" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "markdown.mode.showPreview": { |
There was a problem hiding this comment.
markdown.preview.dialog.title missing translations for 17 locales
The new "markdown.preview.dialog.title" entry, used as the messageText of the NSAlert search dialog in MarkdownWebRenderer.Coordinator.markdownDialogTitle(), only ships English and Japanese translations. The existing string catalog already supports ar, bs, da, de, es, fr, it, ko, nb, pl, pt-BR, ru, th, tr, uk, zh-Hans, and zh-Hant — confirmed by adjacent entries like shortcut.markdownScrollLeft.label and markdown.web.searchPromptForward, which both carry all 19 locales. Users in those 17 locales will see "Markdown Preview" (English) as the alert window title whenever they invoke a find command in the Markdown preview.
Rule Used: Flag production user-facing text that is not fully... (source)
…review-keybindings # Conflicts: # Sources/TabManager.swift
| default: | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
Identical computed properties duplicate the same switch logic
Low Severity
allowsBareFirstStroke and isSurfaceLocalShortcutAction have byte-for-byte identical switch implementations covering the same 10 markdown cases. One could delegate to the other (or both to a shared isMarkdownPreviewAction property) to avoid maintaining two identical switch blocks that must be kept in sync when new panel-local actions are added.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4b58f65. Configure here.
| var query = window.prompt(cmuxString(promptKey, fallback), markdownPreviewSearchQuery); | ||
| if (query == null) { return false; } | ||
| markdownPreviewSearchQuery = String(query); | ||
| return markdownPreviewSearch(markdownPreviewSearchQuery, backwards); |
There was a problem hiding this comment.
markdownPreviewSearchQuery is overwritten with the empty string before markdownPreviewSearch validates it. If the user opens the prompt (pressing / or ?) and then submits without typing anything, the module-level variable is set to "" and the previous query is gone. Any subsequent n / N keystroke will call markdownPreviewSearch("", …), hit the if (!value) guard immediately, and do nothing until a fresh search is initiated. The fix is to only commit the new query when it is non-empty.
| var query = window.prompt(cmuxString(promptKey, fallback), markdownPreviewSearchQuery); | |
| if (query == null) { return false; } | |
| markdownPreviewSearchQuery = String(query); | |
| return markdownPreviewSearch(markdownPreviewSearchQuery, backwards); | |
| var query = window.prompt(cmuxString(promptKey, fallback), markdownPreviewSearchQuery); | |
| if (query == null) { return false; } | |
| var trimmedQuery = String(query); | |
| if (trimmedQuery) { markdownPreviewSearchQuery = trimmedQuery; } | |
| return markdownPreviewSearch(markdownPreviewSearchQuery, backwards); |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 08d9328. Configure here.
|
|
||
| static var actions: [KeyboardShortcutSettings.Action] { | ||
| commandActions.map(\.1) | ||
| } |
There was a problem hiding this comment.
Unused actions property is dead code
Low Severity
The MarkdownPreviewKeyboardShortcutResolver.actions computed property is defined but never referenced anywhere in the codebase. A grep for this property across all source and test files returns zero matches. This is unused dead code introduced by the PR.
Reviewed by Cursor Bugbot for commit 08d9328. Configure here.
…review-keybindings # Conflicts: # web/messages/ar.json # web/messages/bs.json # web/messages/da.json # web/messages/de.json # web/messages/en.json # web/messages/es.json # web/messages/fr.json # web/messages/it.json # web/messages/ja.json # web/messages/km.json # web/messages/ko.json # web/messages/no.json # web/messages/pl.json # web/messages/pt-BR.json # web/messages/ru.json # web/messages/th.json # web/messages/tr.json # web/messages/uk.json # web/messages/zh-CN.json # web/messages/zh-TW.json
| func handlePreviewKeyboardShortcut(_ event: NSEvent) -> Bool { | ||
| guard displayMode == .preview else { return false } | ||
| return rendererSession.handleKeyboardShortcut(event) | ||
| } | ||
|
|
||
| @discardableResult | ||
| func performPreviewKeyboardCommand(_ command: MarkdownPreviewKeyCommand) -> Bool { | ||
| guard displayMode == .preview else { return false } |
There was a problem hiding this comment.
performPreviewKeyboardCommand returns true whenever displayMode == .preview, but rendererSession.performKeyboardCommand calls evaluateJavaScript without guarding on whether the shell has loaded. When the WKWebView is nil (coordinator closed) or the page is still loading (isLoaded == false), the JS guard window.__cmuxMarkdownPreviewHandleKeyCommand && short-circuits to false and nothing executes — yet the function still returns true, consuming the shortcut. A user who presses / or Cmd-F the moment a Markdown panel opens will get no search dialog and no feedback, and the fallback to focusedBrowserPanel?.startFind() in performFindShortcutInActiveMainWindow is never reached. The same applies to the AppDelegate path through handlePreviewKeyboardShortcut. Returning false when !coordinator.isLoaded (or surfacing the loaded state through MarkdownRendererSession) would let the event fall through to other handlers.
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
2 participants
Summary
~/.config/cmux/cmux.json, including bare panel-local keys.Testing
xcodebuildmcp macos test --project-path cmux.xcodeproj --scheme cmux-unit --configuration Debug --derived-data-path /tmp/cmux-mdkeys-test --extra-args=-only-testing:cmuxTests/MarkdownPanelTests/testMarkdownPreviewKeyboardShortcutsUseVimDefaultsxcodebuildmcp macos test --project-path cmux.xcodeproj --scheme cmux-unit --configuration Debug --derived-data-path /tmp/cmux-mdkeys-test --extra-args=-only-testing:cmuxTests/MarkdownPanelTests/testMarkdownPreviewKeyboardShortcutsCanUseBareCmuxJSONBindingsxcodebuildmcp macos test --project-path cmux.xcodeproj --scheme cmux-unit --configuration Debug --derived-data-path /tmp/cmux-mdkeys-test --extra-args=-only-testing:cmuxTests/MarkdownPanelTests/testMarkdownPreviewKeyCommandHandlersScrollAndSearchxcodebuildmcp macos test --project-path cmux.xcodeproj --scheme cmux-unit --configuration Debug --derived-data-path /tmp/cmux-mdkeys-test --extra-args=-only-testing:cmuxTests/MarkdownPanelTests/testMarkdownRenderKeepsVisibleHeadingPositionAfterContentUpdatejq empty Resources/Localizable.xcstrings web/data/cmux.schema.json web/messages/en.json web/messages/ja.json./scripts/reload.sh --tag mdkeysLocal note: the full
MarkdownPanelTestsclass still crashes in existing WebKit image tests on this machine, while the two image tests pass when run individually.Closes #4140
Need help on this PR? Tag
@codesmithwith what you need.Note
Medium Risk
Medium risk: adds new shortcut actions and routing logic (including bare-key handling) plus new WebKit prompt handling, which could affect keyboard event dispatch and shortcut conflict resolution across panels/windows.
Overview
Adds a dedicated set of Markdown Preview keyboard shortcut actions (vim-style scroll/page and in-preview find) that are panel-local, customizable via Settings and
cmux.json, and allowed to use bare first strokes without modifiers.Updates shortcut parsing/recording and conflict resolution to support these bare-key Markdown actions, introduces a markdown-specific shortcut context, and routes the Find family (
⌘F/⌘Getc.) to the focused Markdown preview when applicable without stealing input from text fields/auxiliary windows.Extends the markdown WKWebView shell to execute smooth scrolling and
window.findsearch (with localized prompts and dialog title), adds extensive unit tests, and updates README/docs/schema/translations to document the new bindings.Reviewed by Cursor Bugbot for commit e4c3439. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds configurable, panel‑local Markdown Preview keybindings for Vim‑style scroll, half‑page moves, and search. Shortcuts are surface‑local to the focused preview in main windows and now own the Find family without breaking global chords.
New Features
~/.config/cmux/cmux.json.window.find) with localized prompts and a “Markdown Preview” dialog title; README/docs/schema/translations updated.Bug Fixes
Written for commit e4c3439. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Documentation
Localization
Tests