Repository navigation
Open terminal path:line links in the preferred editor - #7162
Eridanus117 wants to merge 9 commits into
Conversation
Terminal links of the form `path:line[:column]` (the convention Claude Code and Codex print for click-to-open) do not open at the referenced line today: absolute refs silently fail, relative refs are misrouted to the browser omnibar. Introduce the resolution/opening primitives as stubs and cover the target behavior with tests so this commit is red and the fix (next commit) turns it green: - `String.splitTerminalPathLineSuffix()` splits a trailing `:line[:column]`. - `TerminalPathResolver.resolveOpenURLFileReference(_:cwd:)` resolves a token to an existing file plus optional line/column, probing the literal path first and the line-stripped path second. - `TerminalFileReference.fileURL` carries line/column as a `#L…` fragment. - `PreferredEditorService.open` should honor that fragment and hand a `path:line:column` argument (plus `-g`) to VS Code-family editors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve terminal file links of the form `path:line[:column]` — the convention Claude Code and Codex print for click-to-open — to the referenced file and open it at that line in the configured editor: - `String.splitTerminalPathLineSuffix()` peels a trailing `:line[:column]`. - `TerminalPathResolver.resolveOpenURLFileReference(_:cwd:)` resolves a token to an existing file plus optional line/column, probing the literal path first (so a file that really ends in a colon-number wins) and the line-stripped path second; web URLs are never treated as file paths. - `TerminalFileReference.fileURL` carries line/column across the package boundary as a `#L<line>[:<column>]` fragment. - `PreferredEditorService.open` decodes that fragment and hands `path:line:column` (plus `-g`) to VS Code-family editors, falling back to the fragment-free file URL for other editors and the system opener. - The terminal open-URL handler routes any line reference to the editor, which also fixes relative `dir/file.swift:line` links that previously fell through to the browser omnibar. Verified with package-level swift test on CmuxTerminalCore and CmuxWorkspaces (red without the fix, green with it). No new user-facing strings. Supersedes manaflow-ai#3977 by @austinywang, which implemented the same user-facing behavior before manaflow-ai#5894 extracted the resolver into CmuxTerminalCore / CmuxWorkspaces; the `:line` split and editor `-g` dispatch are adapted from that PR onto the new package seams. Co-authored-by: austinpower1258 <austinwang115@gmail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@Eridanus117 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds terminal open-URL support for ChangesLine/column reference resolution and editor opening
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 25✅ Passed checks (25 passed)
✨ Finishing Touches🧪 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 |
Greptile SummaryThis PR fixes three broken terminal link behaviors: absolute
Confidence Score: 5/5The change is well-scoped with full package-level test coverage; bare-path links are unaffected; only the full-app Swift 6 build needs maintainer confirmation. All four new primitives have dedicated unit tests covering edge cases. The only unverified piece is the app-target build under Swift 6 strict isolation, noted in a previous thread. Sources/GhosttyTerminalView.swift — the @mainactor call site needs a full-app build confirmation with the Zig/Ghostty toolchain before merge. Important Files Changed
Reviews (4): Last reviewed commit: "Fail closed on malformed line-reference ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| guard !lastSuffix.isEmpty, | ||
| lastSuffix.allSatisfy(\.isNumber), | ||
| let lastNumber = Int(lastSuffix), | ||
| lastNumber > 0 else { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
isNumber includes non-ASCII numeral characters
Character.isNumber returns true for Unicode numeric glyphs beyond ASCII digits — vulgar fractions (½), superscripts (²), Devanagari digits (५), etc. The guard still behaves correctly overall because Int(lastSuffix) returns nil for any non-ASCII numeral and the outer guard fails, but the predicate passes for a moment before Int() rejects the value, making the intent harder to read. Use allSatisfy({ $0.isASCII && $0.isNumber }) (or the range comparison "0"..."9") to express "only decimal digits" precisely in both calls in this function.
| private static func commandTokens(_ command: String) -> [String] { | ||
| var tokens: [String] = [] | ||
| var current = "" | ||
| var quote: Character? | ||
| var hasToken = false | ||
|
|
||
| for character in command { | ||
| if let active = quote { | ||
| if character == active { | ||
| quote = nil | ||
| } else { | ||
| current.append(character) | ||
| } | ||
| hasToken = true | ||
| continue | ||
| } | ||
| if character == "'" || character == "\"" { | ||
| quote = character | ||
| hasToken = true | ||
| continue | ||
| } | ||
| if character.isWhitespace { | ||
| if hasToken { | ||
| tokens.append(current) | ||
| current = "" | ||
| hasToken = false | ||
| } | ||
| continue | ||
| } | ||
| current.append(character) | ||
| hasToken = true | ||
| } | ||
| if hasToken { | ||
| tokens.append(current) | ||
| } | ||
| return tokens | ||
| } |
There was a problem hiding this comment.
commandTokens handles quoted binary paths (single and double quotes) but explicitly skips backslash-escape interpretation. A user who configures their editor as /Applications/Visual\ Studio\ Code.app/Contents/Resources/app/bin/code (backslash-escaped spaces instead of quotes) would have the command split at each unescaped space, resulting in tokens.first being "Visual\\" — which is not in gotoEditorCommandNames. The file opens fine (the bare path is used), but goto navigation silently fails: the file opens at the top rather than at the referenced line. The code comment acknowledges this, but there is no test covering the degradation. A recognizesBackslashEscapedBinaryPath test (expecting gotoFlag == "" and argument == path) would document the boundary precisely and prevent a future fix from accidentally enabling partial tokenization.
| if reference.line != nil { | ||
| #if DEBUG | ||
| cmuxDebugLog("link.openURL resolvedAsFileReference=\(reference.fileURL)") | ||
| #endif | ||
| PreferredEditorService(defaults: .standard).open(reference.fileURL) | ||
| return (true, nil) | ||
| } |
There was a problem hiding this comment.
PreferredEditorService.open is @MainActor, and it is called here synchronously inside the performOnMain closure. The package-level tests pass, but the PR description explicitly notes the full app target was not built locally. Swift 6 strict isolation checking happens at compile time in the app target; if performOnMain's closure type does not carry a @MainActor annotation (or an equivalent @isolated(any) parameter) the compiler may reject this call. The existing closure already calls CommandClickFileOpenRouter.shouldRouteInCmux and other presumably @MainActor symbols without issues, which is a positive signal, but a maintainer with the full toolchain should confirm the app target builds without isolation errors before merging.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolver.swift`:
- Around line 169-191: Extract the shared token-to-standardized-path resolution
logic out of TerminalPathResolver so the duplicated pipeline in
existingFileReference and resolveQuicklookPath is centralized. Create a small
helper that handles expandingTildeInPath, the absolute-vs-cwd path decision,
standardizingPath, and seenPaths dedup, then have existingFileReference call it
and keep only the fileExists/TerminalFileReference creation step; leave
resolveQuicklookPath as a follow-up if needed. Use the existing
existingFileReference and resolveQuicklookPath symbols to keep the refactor
scoped and consistent.
🪄 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: 85da9610-ff35-418b-bf5b-0ad9e649d366
📒 Files selected for processing (6)
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/String+TerminalPathTokens.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolver.swiftPackages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/TerminalPathLineReferenceTests.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/FileOpen/PreferredEditorLineReferenceTests.swiftSources/GhosttyTerminalView.swift
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found across 6 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="Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swift">
<violation number="1" location="Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swift:127">
P2: Goto handling silently fails when the configured editor command includes a wrapper prefix (e.g., `env VAR=1 code`, `arch -arm64 code`, `nohup code`). The tokenizer produces the correct word list, but the code only inspects `tokens.first` to decide whether the editor supports goto arguments. Because the first token is the wrapper executable, the lookup against `gotoEditorCommandNames` misses the actual editor name and the function falls back to a plain file path, discarding the `#L...` line/column fragment. A safer approach is to scan all tokens for a recognized editor name rather than only the first one.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…, dedup - `splitTerminalPathLineSuffix` now requires ASCII digits, so a Unicode numeral suffix (`²`, full-width `４２`, Devanagari) is not read as a line number (previously relied on `Int()` rejecting it downstream). - `commandTokens` interprets backslash escapes, so an editor command with a backslash-escaped binary path (`/Applications/Visual\ Studio\ Code.app/…`) is still recognized as a goto editor and gets `-g` + `path:line:column`. - Extract the shared `~`-expand / cwd-join / standardize / dedup step into `standardizedExistingPath`, used by both `resolveQuicklookPath` and the new file-reference resolver (removes the duplicated candidate pipeline). Not changed: goto detection still keys off the command's first shell word, so a wrapper prefix (`env VAR=1 code`, `arch -arm64 code`) opens the file without `-g` — same first-word model as the configured-editor command elsewhere; wrapper-prefixed editor commands are uncommon and degrade to a plain open. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolver.swift (1)
147-155: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftDon’t exclude extensionless file references from open-url resolution.
Line 153 rejects bare extensionless files, so
Makefile:12,Dockerfile:7, orREADME:3returnnileven when the file exists and can still fall through to browser routing. Prefer an authoritative regular-file probe for the stripped candidate, rather than using slash/extension shape as the deciding signal. As per path instructions, correctness-critical routing should resolve from a trusted computation like “resolved & exists,” not string heuristics.Possible direction
- private let fileExists: `@Sendable` (String) -> Bool + private let fileStatus: `@Sendable` (String) -> FileStatus + private enum FileStatus: Sendable { + case missing + case file + case directory + }Use
fileStatussohost:portcan still fail closed when it resolves to a directory, while real extensionless files can open at the requested line.🤖 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 `@Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolver.swift` around lines 147 - 155, The open-url path check in TerminalPathResolver’s looksLikeFilePath is too restrictive because it rejects extensionless files like Makefile, Dockerfile, and README even when they exist. Update the resolution flow to use a trusted regular-file existence check on the stripped candidate (for example via fileStatus) instead of relying on slash or extension heuristics, so host:port cases still fail closed when they resolve to a directory while real extensionless files can still open at the requested line.Source: Path instructions
🤖 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.
Outside diff comments:
In
`@Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolver.swift`:
- Around line 147-155: The open-url path check in TerminalPathResolver’s
looksLikeFilePath is too restrictive because it rejects extensionless files like
Makefile, Dockerfile, and README even when they exist. Update the resolution
flow to use a trusted regular-file existence check on the stripped candidate
(for example via fileStatus) instead of relying on slash or extension
heuristics, so host:port cases still fail closed when they resolve to a
directory while real extensionless files can still open at the requested line.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: b62736f5-2734-418b-97a1-c7000b59f319
📒 Files selected for processing (5)
Packages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/String+TerminalPathTokens.swiftPackages/macOS/CmuxTerminalCore/Sources/CmuxTerminalCore/PathResolution/TerminalPathResolver.swiftPackages/macOS/CmuxTerminalCore/Tests/CmuxTerminalCoreTests/TerminalPathLineReferenceTests.swiftPackages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/FileOpen/PreferredEditorLineReferenceTests.swift
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
`commandTokens` interprets a backslash as escaping the next character even inside double quotes, so `code "\--goto"` tokenizes to `--goto` and the invocation drops the `-g` flag. But `/bin/sh -c` (which actually runs the command) preserves a literal backslash inside double quotes, passing `\--goto`. This test pins the correct behavior — `-g` must still be added — and is red against the current tokenizer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Match `/bin/sh -c` word-splitting: a backslash is literal inside single or double quotes and only escapes the next character when unquoted. Previously the escape fired inside double quotes too, so `code "\--goto"` tokenized to `--goto` and the invocation wrongly saw an existing goto flag, dropping the `-g` that a `path:line` link needs. Unquoted backslash-escaped binary paths (e.g. `/Applications/Visual\ Studio\ Code.app/…`) are unaffected — the escape still applies there. Turns the prior commit's test green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`editorInvocation` keys the goto-editor check off only the command's first shell word, so a realistic wrapper prefix (`arch -arm64 code` on Apple Silicon, `env VAR=1 code`, `nohup code`) is not recognized as a VS Code-family editor and the file opens without `-g` — no line jump. This test pins the expected `-g` and is red against the first-word check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Recognize a VS Code-family editor anywhere in the configured command so a wrapper prefix (`arch -arm64 code`, `env VAR=1 code`, `nohup code`) still gets `-g` + `path:line:column`. The first-word-only check missed these and degraded to a plain open with no line jump. Turns the prior commit's test green. The only cost is that a non-goto editor command whose bare argument is literally named `code`/`cursor`/… would be treated as goto-capable, but an editor command is `<editor> [flags]` (flags start with `-`), so a bare same-named argument is not a real configuration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review round 2Pushed
Package tests green on Xcode 26.5: Re: greptile's app-target actor-isolation note — I built the full app target locally (Xcode 26.5, plus a pinned zig 0.15.2 for the Ghostty CLI helper). The changed Swift — including the |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swift (1)
143-155: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRequire the
Lmarker and reject malformed columns.
#42,#L42:, and#L42:abccurrently become line-only references, which can open the wrong location. The fragment contract is#L<line>[:<column>]; fail closed when the marker is missing or a supplied column is invalid.As per path instructions, correctness-critical open/dispatch logic should use a single reliable signal and avoid best-effort guessing when it can route to the wrong target. <path_instructions>
Proposed fix
- if raw.first == "L" || raw.first == "l" { - raw.removeFirst() - } + guard raw.first == "L" || raw.first == "l" else { + return nil + } + raw.removeFirst() let parts = raw.split(separator: ":", maxSplits: 1, omittingEmptySubsequences: false) guard let linePart = parts.first, let line = Int(linePart), line > 0 else { return nil } - if parts.count == 2, let column = Int(parts[1]), column > 0 { + if parts.count == 2 { + guard let column = Int(parts[1]), column > 0 else { + return nil + } return (line, column) } return (line, nil)🤖 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 `@Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swift` around lines 143 - 155, The fragment parser in PreferredEditorService’s line/column handling is too permissive and accepts malformed references like missing the required L marker or invalid columns, which can route to the wrong location. Update the parsing logic that splits the fragment into line and column so it only succeeds for the exact `#L`<line>[:<column>] form, returning nil when the marker is absent, the line is invalid, or a supplied column is malformed or nonpositive. Keep the behavior localized to the existing fragment parsing path in PreferredEditorService so the open/dispatch flow uses one reliable signal instead of best-effort guessing.Source: Path instructions
🤖 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.
Outside diff comments:
In
`@Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swift`:
- Around line 143-155: The fragment parser in PreferredEditorService’s
line/column handling is too permissive and accepts malformed references like
missing the required L marker or invalid columns, which can route to the wrong
location. Update the parsing logic that splits the fragment into line and column
so it only succeeds for the exact `#L`<line>[:<column>] form, returning nil when
the marker is absent, the line is invalid, or a supplied column is malformed or
nonpositive. Keep the behavior localized to the existing fragment parsing path
in PreferredEditorService so the open/dispatch flow uses one reliable signal
instead of best-effort guessing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 47946bfd-ac66-471a-b018-c4487c132e4e
📒 Files selected for processing (2)
Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/FileOpen/PreferredEditorLineReferenceTests.swift
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swift (1)
152-161: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReject malformed column fragments instead of falling back to line-only.
When the fragment contains a colon but the column is empty/invalid (
L42:,L42:abc,L42:0), Line 158 falls through and returns(line, nil), so a malformed location still opens at a guessed line. Fail closed for malformed line-reference transport.Proposed fix
- if parts.count == 2, let column = Int(parts[1]), column > 0 { - return (line, column) + if parts.count == 2 { + guard let column = Int(parts[1]), column > 0 else { + return nil + } + return (line, column) } return (line, nil)As per path instructions, correctness-critical
path[:line[:column]]routing should avoid best-effort parsing fallbacks and fail closed when the reliable signal is missing.🤖 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 `@Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swift` around lines 152 - 161, The parsing in PreferredEditorService’s line-reference helper currently falls back to returning a line-only location when a colon is present but the column is missing or invalid. Update the parser so that any fragment containing a colon must only succeed when the column is present and a valid positive integer; otherwise return nil instead of `(line, nil)`. Keep the change localized to the raw split/Int parsing logic in the line-reference parsing path.Source: Path instructions
🤖 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.
Outside diff comments:
In
`@Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swift`:
- Around line 152-161: The parsing in PreferredEditorService’s line-reference
helper currently falls back to returning a line-only location when a colon is
present but the column is missing or invalid. Update the parser so that any
fragment containing a colon must only succeed when the column is present and a
valid positive integer; otherwise return nil instead of `(line, nil)`. Keep the
change localized to the raw split/Int parsing logic in the line-reference
parsing path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 51c9c48e-8472-44e1-a807-4e55980c02bd
📒 Files selected for processing (2)
Packages/macOS/CmuxWorkspaces/Sources/CmuxWorkspaces/FileOpen/PreferredEditorService.swiftPackages/macOS/CmuxWorkspaces/Tests/CmuxWorkspacesTests/FileOpen/PreferredEditorLineReferenceTests.swift
`lineColumn(fromFragment:)` accepts fragments outside the `L<line>[:<column>]` contract: a bare `manaflow-ai#42` (no `L` marker) becomes line 42, and a colon group with a missing/invalid/nonpositive column (`L42:`, `L42:abc`, `L42:0`) silently falls back to line-only. These tests pin fail-closed behavior and are red against the current permissive parser. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Parse only the exact `L<line>[:<column>]` fragment contract that `TerminalFileReference.fileURL` emits: require the `L` marker (so a bare `manaflow-ai#42` anchor is not read as a line) and require a positive-integer column when a colon group is present (so `L42:`, `L42:abc`, `L42:0` are rejected rather than silently opening at line-only). Matches the repo's fail-closed convention for open/dispatch routing. Turns the prior commit's tests green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review round 3 (CodeRabbit)Addressed both fail-closed findings on
The parser now accepts exactly the For context: this is defensive hardening rather than a live bug — the only fragment-bearing caller today is the terminal Package tests: |
|
You had path:line handling first! Main now carries the behavior in 8616849, so I’m closing this superseded PR :) |
2 participants
What
Terminal file links of the form
path:line[:column]— the convention Claude Code and Codex print for click-to-open — now open at the referenced line in the user's preferred editor.Before this change, only bare paths clicked through; the
:linesuffix broke three ways:/Users/me/app/main.swift:42URL(fileURLWithPath:)keeps:42, file doesn't exist)main.swiftat line 42src/App.swift:42:5(relative)https://src/App.swift…)App.swiftat line 42, col 5/Users/me/README.md(no line)How
The bug and fix are entirely in the Swift open/route layer (Ghostty already frames the full
path:line[:col]as one link token). Four small pieces on the existing package seams:String.splitTerminalPathLineSuffix()(CmuxTerminalCore) — peels a trailing:line/:line:column(positive integers only) off a token.TerminalPathResolver.resolveOpenURLFileReference(_:cwd:)— resolves a token to an existing file plus optional line/column. Each candidate spelling is probed literal-first (so a file that really ends in:42still wins) then line-stripped;http/httpstext is never treated as a file, and the file-existence probe gates everything else.TerminalFileReference.fileURL— carries line/column across the package boundary as a#L<line>[:<column>]URL fragment (single encode site).PreferredEditorService.open— decodes that fragment and handspath:line:column(plus-g) to VS Code-family editors (code,code-insiders,cursor,cursor-insiders,windsurf), respecting an existing-g/--gotoin the configured command. Other editors and the system-open fallback get the plain file path.The terminal open-URL handler routes any resolved line reference to the editor (in-app viewers can't jump to a line); this is also what fixes the relative-path→browser misroute.
Scope
Covers the terminal link click path (
GHOSTTY_ACTION_OPEN_URL), which is where all three reported failures occur. The editor-dispatch path is now the single fragment-aware primitive, so the cmd-click "word under cursor" fallback (bare filenames Ghostty doesn't underline, #2199 territory) can supply a line reference in a one-line follow-up without touching this logic again.Testing
Package-level
swift test(Xcode 26.5), red→green two-commit:swift test --package-path Packages/macOS/CmuxTerminalCore— 187 tests green (split, reference resolution incl. thehost:port-with-same-named-dir exclusion, fragment encoding).swift test --package-path Packages/macOS/CmuxWorkspaces— 106 tests green (editor-g/path:line:columndispatch — incl. quoted app-bundle binary detection and existing---gotohandling — end-to-end throughopen).The first commit adds the tests against stubs / current behavior and is red; the second implements and is green. No new user-facing strings (localization audit: n/a).
The four resolution/dispatch primitives are covered above; the ~15-line
GHOSTTY_ACTION_OPEN_URLwiring that calls them is a thin routing shim (line ref → editor; no-line → existing cmux/external routing) and was reviewed by inspection — I couldn't build the full app locally (the Ghostty CLI helper needs a zig toolchain I don't have installed), so the app-level build and the three repro tokens (/abs/readme.md:1, relativedir/App.swift:1,foo.swift:42:5) should be confirmed on CI / a maintainer machine.Credit
Supersedes #3977 by @austinywang, which delivered the same user-facing behavior. That PR predates #5894, which extracted the terminal resolver into
CmuxTerminalCore/CmuxWorkspaces; rebasing it onto the new package seams amounts to a rewrite, so this is a focused reimplementation (~280 lines vs 886) adapting its:linesplit and editor-gdispatch. Full credit to @austinywang viaCo-authored-by.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by cubic
Clicking terminal links like path:line[:column] now opens the file at that location in your preferred editor. This fixes broken absolute links and stops relative links from being sent to the browser.
New Features
path:line[:column]inCmuxTerminalCore, carrying line/column as a#L…fragment.CmuxWorkspacesconverts#L…topath:line[:column]for VS Code–family CLIs (code,code-insiders,cursor,cursor-insiders,windsurf) and adds-gunless present; other editors get the plain path.Bug Fixes
/app/main.swift:42open at line 42; relativesrc/App.swift:42:5opens in the editor, not the browser.L<line>[:<column>]is accepted — reject#42,L42:,L42:abc,L42:0); ignorehttp/httpsandhost:port; only treat line-stripped tokens that look like file paths./bin/sh -ctokenizing and detect VS Code–family CLIs anywhere in the command; do not treat"\--goto"as--goto, and add-gwhen needed.Written for commit c05260e. Summary will update on new commits.
Summary by CodeRabbit
path:line/path:line:column, targeting the specified location in the preferred editor.#L<line>[:<column>]fragments (and encodes them in produced file URLs) to enable goto-capable commands.word:number, and host:port-like strings from being misread as file references; trimmed/empty inputs don’t resolve.