feat(terminal): smart links + reorganized right-click menu#5491
feat(terminal): smart links + reorganized right-click menu#5491Supersynergy wants to merge 1 commit into
Conversation
|
@Supersynergy is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR enhances the terminal right-click context menu with single-pass token detection (filesystem path, URL, or GitHub shorthand) and adds Open/Reveal/Copy actions, a Find item, and workspace-aware actions (Open Folder in Finder, Copy Working Directory). New menu items are localized into English, Japanese, and Korean. ChangesSmart Terminal Context Menu
Sequence Diagram(s)sequenceDiagram
participant GhosttyNSView
participant TokenResolver
participant TerminalSurface
participant Finder
participant Browser
participant Pasteboard
GhosttyNSView->>TerminalSurface: request tokenUnderCursor
GhosttyNSView->>TokenResolver: resolve token + workingDirectory
TokenResolver-->>GhosttyNSView: resolvedPath or smartURL
GhosttyNSView->>Finder: open/reveal path (open/reveal path action)
GhosttyNSView->>Browser: open URL (open link action)
GhosttyNSView->>Pasteboard: write string (copy path or link)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (19 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 implements P1 of #5482: a reorganized terminal right-click menu that surfaces contextual actions (open/reveal/copy path, open/copy link) based on what sits under the cursor at click time, plus workspace-level "Open Folder in Finder" and "Copy Working Directory" items. The file-path recognizer delegates to the existing
Confidence Score: 5/5Safe to merge; the menu-construction logic, actor threading model, and action handlers are all correct on the happy paths. The new right-click menu wiring is correctly isolated to
Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User (right-click)
participant V as GhosttyNSView.menu(for:)
participant R as resolveWordUnderCursorPath
participant G as ghostty_surface_quicklook_word
participant B as browserURL(forToken:)
participant WD as surfaceWorkingDirectory
participant CFF as CommandClickFileOpenRouter
U->>V: right-click event
V->>R: resolveWordUnderCursorPath(at: point)
R->>G: ghostty_surface_quicklook_word (C API)
G-->>R: text token + filesystem stat
R-->>V: WordPathResolution? (.path)
alt "resolvedWordPath != nil"
V->>V: add Open/Reveal/Copy Path items
else "resolvedWordPath == nil"
V->>G: wordUnderCursorToken() → ghostty_surface_quicklook_word again
G-->>V: raw token
V->>B: browserURL(forToken:)
B-->>V: URL? (http/https or github shorthand)
V->>V: add Open Link / Copy Link items
end
V->>WD: surfaceWorkingDirectory()
WD->>CFF: resolveWorkingDirectory (already resolved above)
CFF-->>WD: String?
WD-->>V: workingDirectory?
alt "workingDirectory != nil"
V->>V: add Open Folder / Copy CWD items
end
V->>V: add Copy/Paste/Find, Layout, Reset, IDs sections
V-->>U: NSMenu displayed
Reviews (6): Last reviewed commit: "feat(terminal): smart links + reorganize..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let allowed = CharacterSet( | ||
| charactersIn: "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._" | ||
| ) | ||
| guard !owner.isEmpty, !repo.isEmpty, | ||
| owner.count <= 39, repo.count <= 100, | ||
| !owner.hasPrefix("-"), !owner.hasPrefix("."), | ||
| owner.unicodeScalars.allSatisfy(allowed.contains), | ||
| repo.unicodeScalars.allSatisfy(allowed.contains) else { return nil } | ||
| return URL(string: "https://github.com/\(owner)/\(repo)\(suffixPath)") |
There was a problem hiding this comment.
The allowed charset permits . and _ in both owner and repo names, but GitHub usernames and org slugs only allow [a-zA-Z0-9-]. Any slash-separated terminal token that fails file-path resolution — such as src/index.ts, dist/bundle.js, or a MIME type like text/html — will pass the owner/repo guards and produce a "Open Link in Browser" menu item pointing at a URL that almost certainly does not exist. Restricting the owner segment to [a-zA-Z0-9-] only would match GitHub's actual username rules and eliminate most false positives.
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!
| // MARK: - Smart context-menu helpers | ||
|
|
||
| /// The surface's current working directory, if known (local terminals only). | ||
| private func surfaceWorkingDirectory() -> String? { | ||
| guard let termSurface = terminalSurface, | ||
| let workspace = termSurface.owningWorkspace(), | ||
| !workspace.isRemoteTerminalSurface(termSurface.id) else { return nil } | ||
| return resolvedWordPathWorkingDirectory(workspace: workspace, terminalSurface: termSurface) | ||
| } | ||
|
|
||
| /// Raw token under the mouse cursor (Ghostty quicklook word), trimmed. | ||
| private func wordUnderCursorToken() -> String? { | ||
| guard let surface = surface else { return nil } | ||
| var text = ghostty_text_s() | ||
| guard ghostty_surface_quicklook_word(surface, &text) else { return nil } | ||
| defer { ghostty_surface_free_text(surface, &text) } | ||
| guard text.text_len > 0, let ptr = text.text else { return nil } | ||
| let data = Data(bytes: ptr, count: Int(text.text_len)) | ||
| let token = String(data: data, encoding: .utf8)? | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard let token, !token.isEmpty else { return nil } | ||
| return token | ||
| } | ||
|
|
||
| /// Recognize a browser-openable target: an explicit http(s) URL, or GitHub | ||
| /// `owner/repo`, `owner/repo#123`, `owner/repo@<sha>` shorthand. | ||
| static func browserURL(forToken token: String) -> URL? { | ||
| let trimmed = token.trimmingCharacters(in: CharacterSet(charactersIn: "()[]{}<>\"'`.,;:")) | ||
| let lower = trimmed.lowercased() | ||
| if lower.hasPrefix("http://") || lower.hasPrefix("https://") { | ||
| return URL(string: trimmed) | ||
| } | ||
| return githubShorthandURL(forToken: trimmed) | ||
| } | ||
|
|
||
| /// Map a bare GitHub `owner/repo[#issue][@sha]` token to its github.com URL. | ||
| /// Returns nil for anything that is not a plausible `owner/repo` slug. | ||
| static func githubShorthandURL(forToken token: String) -> URL? { | ||
| var slug = token | ||
| var suffixPath = "" | ||
| if let hash = slug.firstIndex(of: "#") { | ||
| let issue = slug[slug.index(after: hash)...] | ||
| guard !issue.isEmpty, issue.allSatisfy(\.isNumber) else { return nil } | ||
| suffixPath = "/issues/\(issue)" | ||
| slug = String(slug[..<hash]) | ||
| } else if let at = slug.firstIndex(of: "@") { | ||
| let sha = slug[slug.index(after: at)...] | ||
| guard sha.count >= 7, sha.allSatisfy(\.isHexDigit) else { return nil } | ||
| suffixPath = "/commit/\(sha)" | ||
| slug = String(slug[..<at]) | ||
| } | ||
| let parts = slug.split(separator: "/", omittingEmptySubsequences: false) | ||
| guard parts.count == 2 else { return nil } | ||
| let owner = String(parts[0]) | ||
| let repo = String(parts[1]) | ||
| let allowed = CharacterSet( | ||
| charactersIn: "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._" | ||
| ) | ||
| guard !owner.isEmpty, !repo.isEmpty, | ||
| owner.count <= 39, repo.count <= 100, | ||
| !owner.hasPrefix("-"), !owner.hasPrefix("."), | ||
| owner.unicodeScalars.allSatisfy(allowed.contains), | ||
| repo.unicodeScalars.allSatisfy(allowed.contains) else { return nil } | ||
| return URL(string: "https://github.com/\(owner)/\(repo)\(suffixPath)") | ||
| } |
There was a problem hiding this comment.
GhosttyTerminalView.swift is already 16,479 lines before this PR, and the cmux file-package-boundary rule flags large additions to existing oversized files. browserURL(forToken:) and githubShorthandURL(forToken:) are pure static funcs with zero UI or AppKit dependency — the PR even notes they are "ready for cmux-unit". Adding them directly to this file keeps independently testable parsing logic inside the app target alongside 16K lines of UI code. The natural fix is to extract them to a small SwiftPM package and import it here.
Rule Used: Flag Swift changes that add too much unrelated res... (source)
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!
| }, | ||
| "terminalContextMenu.copyLink": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "Copy Link" | ||
| } | ||
| }, | ||
| "ja": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "リンクをコピー" | ||
| } | ||
| }, | ||
| "ko": { | ||
| "stringUnit": { | ||
| "state": "translated", | ||
| "value": "링크 복사" | ||
| } | ||
| } | ||
| } | ||
| }, | ||
| "terminalContextMenu.copyPath": { | ||
| "extractionState": "manual", | ||
| "localizations": { | ||
| "en": { | ||
| "stringUnit": { |
There was a problem hiding this comment.
Localizable.xcstrings carries 17 locales (ar, bs, da, de, en, es, fr, it, ja, km, ko, nb, pl, ru, th, tr, uk). All 8 new terminalContextMenu.* entries are translated only for en, ja, ko. This matches the translation depth of the existing terminalContextMenu.* keys, but those entries may themselves be backlog debt — catalog additions should cover every locale already present in the touched file, or the gap should be tracked as an explicit follow-up issue.
Rule Used: Flag production user-facing text that is not fully... (source)
Owner segment now matches GitHub's user/org rules (ASCII alphanumerics + single hyphens, no dots/underscores, no leading/trailing/consecutive hyphen, <=39 chars) instead of the lax repo charset. Prevents slash-separated terminal tokens that fail path resolution (text/html, foo.bar/baz, my_org/x) from producing bogus 'Open Link in Browser' items. Repo names stay laxer (dots/underscores) to match real repos like mrdoob/three.js. Addresses Greptile P2 on manaflow-ai#5491.
Supersynergy
force-pushed
the
feat/smart-terminal-context-menu
branch
from
c4eae8d to
39b4595
Compare
Supersynergy
changed the title
|
Re-cut update:
Remaining non-code blocker: Vercel still requires Manaflow team authorization. |
Supersynergy
force-pushed
the
feat/smart-terminal-context-menu
branch
2 times, most recently
from
c4146c0 to
e278a52
Compare
Add context-aware terminal right-click actions and regroup the menu: - Path under cursor: Open (editor), Reveal in Finder, Copy Path - URL / GitHub owner/repo[#issue][@sha] under cursor: Open Link in Browser, Copy Link - Find… entry (was not reachable from the menu) - Working directory: Open Folder in Finder, Copy Working Directory - Regroup: contextual > edit/find > cwd/repo > layout > surface-admin (Trigger Flash moved out of the top slot) Reuses existing resolveWordUnderCursorPath, PreferredEditorSettings.open, activateFileViewerSelecting, SurfaceSearchOverlay. New owner/repo recognizer. Localized en/ja/ko. Relates to manaflow-ai#5482.
Supersynergy
force-pushed
the
feat/smart-terminal-context-menu
branch
from
e278a52 to
62462fe
Compare
|
Update on the latest push (
Remaining blocker is still non-code: fork PR checks/artifacts need maintainer approval / Vercel team authorization. |
1 participant
Summary
Re-cut on current
mainas a single clean commit. Implements P1 of #5482: smart terminal right-click actions plus a grouped menu that puts the thing you clicked first.Smart link recognition
Open <name>,Reveal in Finder,Copy Path.http(s)://...URL or GitHub shorthand (owner/repo,owner/repo#123,owner/repo@<sha>) ->Open Link in Browser,Copy Link.src/index.tsortext/html.Menu layout
Review fixes in this re-cut
Open Folder in Findernow usesNSWorkspace.shared.activateFileViewerSelecting(...), matching the Finder-specific menu label instead of the default folder-handler app.terminalContextMenu.*strings includeen,ja, andko, matching the existing terminal context menu localization depth in this section.origin/main, not the old pre-revert stack.Verification
swiftc -parse Sources/GhosttyTerminalView.swift: passpython3 -m json.tool Resources/Localizable.xcstrings: passen/ja/ko): passgit diff --check HEAD~1..HEAD: pass./scripts/check-pbxproj.sh: pass./scripts/lint-pbxproj-test-wiring.sh: passgrepgod review HEAD~1..HEAD: semgrep diff-aware produced no findingsBlocked locally: full
reload.sh --tag pr5491-menureachesxcodebuildand then fails because this machine only has CommandLineTools selected (/Library/Developer/CommandLineTools) and no Xcode.app.Expected external noise: Vercel checks require Manaflow team authorization and are not code failures.
Relates to #5482.
Summary by CodeRabbit
New Features
Localization