Repository navigation
Add opt-in setting to open Cmd-clicked markdown files in cmux viewer - #2904
Conversation
|
@claude is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a persisted setting and UI toggle to route Cmd-clicked Markdown file paths from the terminal into a cmux Markdown viewer panel (open/focus), with routing guards for remote workspaces, workspace split creation, and localization entries. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Terminal as GhosttyTerminalView
participant Settings as CmdClickMarkdownRouteSettings
participant Workspace as Workspace
participant Viewer as MarkdownPanel
participant Editor as PreferredEditorSettings
User->>Terminal: Cmd-click markdown file path
Terminal->>Terminal: Parse URL / determine local vs remote
Terminal->>Settings: shouldRoute(path)?
alt Remote workspace or routing disabled
Terminal->>Editor: Open with preferred editor (NSWorkspace)
Editor-->>User: File opened externally
else Local & routing enabled
Settings->>Settings: Resolve symlinks & verify readable file
alt Routable markdown file
Terminal->>Workspace: openOrFocusMarkdownSplit(from, filePath)
Workspace->>Viewer: Focus existing or create new markdown panel
Viewer-->>User: Rendered markdown panel
else Not routable
Terminal->>Editor: Open with preferred editor
Editor-->>User: File opened externally
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Sources/GhosttyTerminalView.swift (1)
3260-3299: Extract the markdown viewer open/focus flow into one helper.Both handlers duplicate the same
MarkdownPanellookup/focus/create sequence. A shared helper would keep the reuse logic, split parameters, and caller-specific fallback behavior from drifting again.Also applies to: 7876-7897
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 3260 - 3299, Extract the duplicated MarkdownPanel lookup/focus/create flow into a single helper (e.g. a function like openOrFocusMarkdown(filePath: String, termSurface: TerminalSurface, workspace: Workspace, fallback: (URL) -> Void) -> Bool) and call it from both handlers instead of repeating the block inside performOnMain. The helper should perform the CmdClickMarkdownRouteSettings.shouldRoute check, guard against remote terminals via termSurface/owningWorkspace/isRemoteTerminalSurface, look up an existing panel by casting panels values to MarkdownPanel and comparing filePath, call workspace.focusPanel(existingId) if found, or call workspace.newMarkdownSplit(...) to create and focus the new panel; return true on handled and call the provided fallback (e.g. PreferredEditorSettings.open) when shouldRoute fails. Replace the two duplicated sequences with calls to this helper, passing the file URL/path and the appropriate fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/cmuxApp.swift`:
- Around line 3995-4007: The helper should return or propagate the canonicalized
(resolved) path instead of only a Bool so downstream reuse checks use the same
normalized value; update shouldRoute(path:) (or the callsite immediately after
it) to produce and return the resolved path used for validation (the `resolved`
variable created via (path as NSString).resolvingSymlinksInPath) and then use
that resolved string for the MarkdownPanel reuse lookup and any comparisons
(replace usages of raw fileURL.path or the original path with the resolved path
when comparing against (panel as? MarkdownPanel)?.filePath and when
creating/finding viewers so all consumers compare the same canonical path, e.g.
return the canonical path from shouldRoute or canonicalize right after
validation before the viewer lookup/creation flow.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 3260-3299: The branch handling local file URLs currently always
returns true after PreferredEditorSettings.open(fileURL), causing the open_url
action to be consumed even when the markdown viewer is not actually used; update
the performOnMain closure so that when
CmdClickMarkdownRouteSettings.shouldRoute(path:) is false (the guard else path
where PreferredEditorSettings.open(fileURL) is called) you do not consume the
action—return false instead—while keeping the existing returns of true when you
actually focus an existing MarkdownPanel (workspace.focusPanel), or create/open
one (workspace.newMarkdownSplit); this ensures only successful viewer routing
(via CmdClickMarkdownRouteSettings.shouldRoute, existingId handling, or
newMarkdownSplit) consumes the action and everything else falls through to the
original NSWorkspace path.
---
Nitpick comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 3260-3299: Extract the duplicated MarkdownPanel
lookup/focus/create flow into a single helper (e.g. a function like
openOrFocusMarkdown(filePath: String, termSurface: TerminalSurface, workspace:
Workspace, fallback: (URL) -> Void) -> Bool) and call it from both handlers
instead of repeating the block inside performOnMain. The helper should perform
the CmdClickMarkdownRouteSettings.shouldRoute check, guard against remote
terminals via termSurface/owningWorkspace/isRemoteTerminalSurface, look up an
existing panel by casting panels values to MarkdownPanel and comparing filePath,
call workspace.focusPanel(existingId) if found, or call
workspace.newMarkdownSplit(...) to create and focus the new panel; return true
on handled and call the provided fallback (e.g. PreferredEditorSettings.open)
when shouldRoute fails. Replace the two duplicated sequences with calls to this
helper, passing the file URL/path and the appropriate fallback.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5c232d9e-4035-436a-869d-ea15c33ca45c
📒 Files selected for processing (5)
Resources/Localizable.xcstringsSources/GhosttyTerminalView.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/cmuxApp.swiftweb/data/cmux-settings.schema.json
Greptile SummaryAdds an opt-in Confidence Score: 5/5Safe to merge — opt-in feature with correct fallback behavior in all identified failure paths. All prior P0/P1 concerns (silent nil drop on split creation failure, missing PreferredEditorSettings fallback in handleCommandClickRelease) are addressed. Remaining findings are P2 style nits. The feature is default-off, remote workspace guard is in place, and both integration points fall back cleanly to the system opener when routing is inapplicable or fails. Sources/GhosttyTerminalView.swift — main-thread file I/O in shouldRoute (network-volume hang risk, already noted in prior threads but not yet moved off-main). Important Files Changed
Sequence DiagramsequenceDiagram
participant T as Terminal (I/O thread)
participant M as Main Thread
participant W as Workspace
T->>T: GHOSTTY_ACTION_OPEN_URL fires
T->>T: isEnabled() [UserDefaults, thread-safe]
T->>T: isMarkdownPath(path) [cheap ext check]
alt toggle off OR non-markdown
T->>M: performOnMain: NSWorkspace.open(url)
else toggle on AND markdown ext matches
T->>M: performOnMain: shouldRoute(path)
M->>M: isReadableFile + attributesOfItem
alt not readable / not regular / remote workspace
M-->>T: false → fall through to NSWorkspace.open
else passes all guards
M->>W: openOrFocusMarkdownSplit(from:filePath:)
W->>W: resolvingSymlinksInPath → scan panels
alt existing viewer found
W-->>M: MarkdownPanel (focused)
else no existing viewer
W->>W: newMarkdownSplit(...)
W-->>M: MarkdownPanel? (nil on failure)
end
alt split succeeded
M-->>T: routed=true → return consumed
else split failed (nil)
M-->>T: routed=false → fall through to NSWorkspace.open
end
end
end
Note over T,W: handleCommandClickRelease (main thread)
M->>M: shouldRoute(path)
alt routing succeeds
M->>W: openOrFocusMarkdownSplit(from:filePath:)
W-->>M: MarkdownPanel?
end
alt nil or any guard failed
M->>M: PreferredEditorSettings.open(url)
end
Reviews (4): Last reviewed commit: "Route Cmd-clicked .md files to cmux mark..." | Re-trigger Greptile |
There was a problem hiding this comment.
3 issues found across 5 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="Sources/GhosttyTerminalView.swift">
<violation number="1" location="Sources/GhosttyTerminalView.swift:3282">
P1: Don’t consume the open_url action when markdown routing is disabled; returning true here bypasses the existing `NSWorkspace.open` path and changes the default-off behavior for local markdown file URLs.</violation>
<violation number="2" location="Sources/GhosttyTerminalView.swift:3291">
P2: Check `newMarkdownSplit` result and fall back to the preferred editor on failure; otherwise this path can silently do nothing when split creation fails.</violation>
<violation number="3" location="Sources/GhosttyTerminalView.swift:7881">
P2: Markdown open-or-focus routing logic is duplicated across two click handlers, increasing drift risk and maintenance cost.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -3257,6 +3257,48 @@ class GhosttyApp { | |||
| #endif | |||
There was a problem hiding this comment.
P2: Markdown open-or-focus routing logic is duplicated across two click handlers, increasing drift risk and maintenance cost.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Sources/GhosttyTerminalView.swift, line 7881:
<comment>Markdown open-or-focus routing logic is duplicated across two click handlers, increasing drift risk and maintenance cost.</comment>
<file context>
@@ -7831,6 +7873,28 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
+ if let termSurface = terminalSurface,
+ let workspace = termSurface.owningWorkspace(),
+ !workspace.isRemoteTerminalSurface(termSurface.id),
+ CmdClickMarkdownRouteSettings.shouldRoute(path: resolution.path) {
+ if let existingId = workspace.panels.first(where: { _, panel in
+ (panel as? MarkdownPanel)?.filePath == resolution.path
</file context>
|
Addressed all three bots' review feedback in the updated force-push:
Please re-review: @codex review |
f4ba1eb to
bda43bf
Compare
@SeongJaeSong I have started the AI code review. It will take a few minutes to complete. |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Re-triggering reviews for commit @cubic-dev-ai review |
@SeongJaeSong I have started the AI code review. It will take a few minutes to complete. |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
Adds an opt-in setting that intercepts Cmd-clicked markdown paths (.md, .markdown, .mkd, .mdx) in the terminal and routes them to the built-in cmux markdown viewer panel (with live reload) instead of the preferred editor. Default: off (opt-in). Toggle available under Settings > App. Implementation: - New `CmdClickMarkdownRouteSettings` enum in `cmuxApp.swift` with UserDefaults-backed toggle (`openMarkdownInCmuxViewer`) and a path extension matcher. - `GhosttyApp.action(for:)` branches before `NSWorkspace.shared.open` in the `.external(url)` case of `GHOSTTY_ACTION_OPEN_URL` when the resolved URL is a local markdown file and the toggle is enabled. Uses the existing `Workspace.newMarkdownSplit` API (same entry point as `cmux markdown open`) to split a viewer panel to the right of the source terminal. Reuses an existing viewer panel when one already displays the same file (matches browser link reuse behavior). - `GhosttyNSView.handleCommandClickRelease` mirrors the same branching for the fallback path used when Ghostty does not consume the click (e.g. snapshot-resolved tokens). - `CmuxSettingsFileStore` registers the new key in `supportedSettingsJSONPaths`, parses `app.openMarkdownInCmuxViewer` from settings.json, and includes it in the default settings template. - Settings UI adds a toggle row under `Open Files With` in the App section. Closes manaflow-ai#1778
bda43bf to
63180ed
Compare
|
Rebased onto latest main ( Please re-review: @cubic-dev-ai review |
@SeongJaeSong I have started the AI code review. It will take a few minutes to complete. |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
|
@austinywang all bots green (Codex / CodeRabbit / Greptile / cubic), ready for review whenever you have a moment. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
…markdown-viewer Add opt-in setting to open Cmd-clicked markdown files in cmux viewer
Summary
app.openMarkdownInCmuxViewer, default off) that routes Cmd-clicked markdown paths (.md,.markdown,.mkd,.mdx) in the terminal to the built-in cmux markdown viewer panel (with live reload) instead of the preferred editor. Reuses an existing viewer panel when one already shows the same file.Behavior
PreferredEditorSettings.open/NSWorkspace.openbehavior.md*filefile://host/...(non-local), or URL with#fragment/?queryImplementation notes
CmdClickMarkdownRouteSettingsenum inSources/cmuxApp.swift:isMarkdownPath(_:)— cheap extension check, safe off main thread.shouldRoute(path:)— full readiness check (toggle + extension + readable + regular file viaattributesOfItem). Mirrors the existingmarkdown.opensocket path'sisReadableFileguard.Workspace.newMarkdownSplitAPI that powerscmux markdown open:GHOSTTY_ACTION_OPEN_URLinGhosttyTerminalView.swiftfor Ghostty-consumed clicks (URL-formed targets, OSC-8 hyperlinks). Runs before the browser-link early return so the setting works regardless ofbrowser.openTerminalLinksInCmuxBrowser.handleCommandClickReleasefallback path for word-under-cursor resolutions.CmuxSettingsFileStore.supportedSettingsJSONPaths, parsed fromsettings.json, included in the default settings template, and published inweb/data/cmux-settings.schema.json.Localizable.xcstrings(en / ja / ko / zh-Hans / zh-Hant / de / es / fr / it).Testing
reload.sh --tag pr-1778 --launch) — toggled the setting off/on and Cmd-clicked the paths below in a terminal panel..mdCmd-click opens in the preferred editor — no regression..md: splits a viewer panel to the right with live reload..mdclicked again: second click focuses the existing panel (no duplicate split)..md: new split is created..zshrc, etc.): opens in preferred editor (no false-positive routing).supportedSettingsJSONPaths+ parser + default template.file://URLs with#fragment/?query— covered by the guard.isReadableFile+typeRegularcheck.Happy to expand coverage or add an automated test if there's a harness suggestion for the cmd-click fallback paths.
Demo Video
I don't have a suitable recording setup here — happy to add a short clip if it helps. In the meantime, the behavior is reproducible in any Debug build by toggling Settings > App > Open Markdown in cmux Viewer and Cmd-clicking an
.mdpath in a terminal.Screen.Cast.2026-04-15.at.12.39.43.PM.mp4
Review Trigger (Copy/Paste as PR comment)
Checklist
Summary by CodeRabbit