Enable macOS Services menu on terminal right-click#2699
Enable macOS Services menu on terminal right-click#2699austinywang wants to merge 3 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughImplement macOS Services pasteboard integration on Ghostty's NSView for sending/receiving terminal selection; adjust Workspace appearance application to compute next appearance via Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant macOS as "macOS Services\nMenu System"
participant GhosttyNSView as "GhosttyNSView\n(NSServicesMenuRequestor)"
participant Pasteboard as Pasteboard
participant GhosttyTerminal as "Ghostty\nSurface"
User->>macOS: Invoke Services on selection
macOS->>GhosttyNSView: validRequestor(sendType: .string)
GhosttyNSView->>GhosttyTerminal: ghostty_surface_has_selection()
GhosttyTerminal-->>GhosttyNSView: hasSelection (true)
GhosttyNSView-->>macOS: return self
macOS->>GhosttyNSView: writeSelection(to: pasteboard)
GhosttyNSView->>GhosttyTerminal: snapshot selection
GhosttyTerminal-->>GhosttyNSView: selectedText
GhosttyNSView->>Pasteboard: setString(selectedText, forType: .string)
rect rgba(100,200,100,0.5)
Note over macOS,Pasteboard: External Service may process/return text
end
macOS->>GhosttyNSView: readSelection(from: pasteboard)
GhosttyNSView->>Pasteboard: GhosttyPasteboardHelper.stringContents
Pasteboard-->>GhosttyNSView: processedText
GhosttyNSView->>GhosttyTerminal: withExternalCommittedText { insertText(processedText) }
GhosttyTerminal-->>GhosttyNSView: done
GhosttyNSView-->>macOS: true
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Greptile SummaryThis PR adds Confidence Score: 5/5PR is safe to merge; all remaining findings are P2 style suggestions with no runtime impact. The NSServicesMenuRequestor implementation is correct and well-integrated with existing selection and text-input infrastructure. Both findings are minor protocol-contract style deviations that have no practical effect given AppKit's type equivalences and the existing guards in validRequestor. Sources/GhosttyTerminalView.swift — NSServicesMenuRequestor extension (lines 11712–11758) Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant AppKit
participant GhosttyNSView
participant GhosttyC as ghostty C API
participant Pasteboard
User->>AppKit: Right-click → Services
AppKit->>GhosttyNSView: validRequestor(forSendType:returnType:)
GhosttyNSView->>GhosttyC: ghostty_surface_has_selection(surface)
GhosttyC-->>GhosttyNSView: Bool
GhosttyNSView-->>AppKit: self (or super/nil)
User->>AppKit: Select service action
AppKit->>GhosttyNSView: writeSelection(to:types:)
GhosttyNSView->>GhosttyC: ghostty_surface_read_selection(surface, &text)
GhosttyC-->>GhosttyNSView: text data
GhosttyNSView->>Pasteboard: declareTypes + setString(selectedText)
AppKit->>AppKit: Service transforms text
AppKit->>GhosttyNSView: readSelection(from:)
GhosttyNSView->>Pasteboard: GhosttyPasteboardHelper.stringContents(from:)
Pasteboard-->>GhosttyNSView: transformed string
GhosttyNSView->>GhosttyNSView: withExternalCommittedText { insertText(...) }
Reviews (1): Last reviewed commit: "Enable terminal Services requestor" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| func writeSelection( | ||
| to pboard: NSPasteboard, | ||
| types: [NSPasteboard.PasteboardType] | ||
| ) -> Bool { | ||
| guard let snapshot = readSelectionSnapshot() else { return false } | ||
|
|
||
| pboard.declareTypes([.string], owner: nil) | ||
| pboard.setString(snapshot.string, forType: .string) | ||
| return true | ||
| } |
There was a problem hiding this comment.
writeSelection ignores the types parameter
The NSServicesMenuRequestor protocol contract requires writing only the types listed in types that the object supports, but the parameter is silently ignored and .string is always written. This is harmless in practice since validRequestor already gates on supported types, but it departs from the protocol contract.
| func writeSelection( | |
| to pboard: NSPasteboard, | |
| types: [NSPasteboard.PasteboardType] | |
| ) -> Bool { | |
| guard let snapshot = readSelectionSnapshot() else { return false } | |
| pboard.declareTypes([.string], owner: nil) | |
| pboard.setString(snapshot.string, forType: .string) | |
| return true | |
| } | |
| func writeSelection( | |
| to pboard: NSPasteboard, | |
| types: [NSPasteboard.PasteboardType] | |
| ) -> Bool { | |
| let supportedTypes: [NSPasteboard.PasteboardType] = [.string, .init("public.utf8-plain-text")] | |
| guard types.isEmpty || types.contains(where: { supportedTypes.contains($0) }) else { return false } | |
| guard let snapshot = readSelectionSnapshot() else { return false } | |
| pboard.declareTypes([.string], owner: nil) | |
| pboard.setString(snapshot.string, forType: .string) | |
| return true | |
| } |
| if (returnType == nil || supportedTypes.contains(returnType!)) && | ||
| (sendType == nil || supportedTypes.contains(sendType!)) { | ||
| if let sendType, supportedTypes.contains(sendType) { | ||
| guard let surface, ghostty_surface_has_selection(surface) else { | ||
| return super.validRequestor(forSendType: sendType, returnType: returnType) | ||
| } | ||
| } |
There was a problem hiding this comment.
validRequestor
The supportedTypes.contains(sendType) condition inside the if let sendType binding is always true at that point — the outer if already guarantees sendType == nil || supportedTypes.contains(sendType!), and since sendType is non-nil here (unwrapped by if let), it is already in supportedTypes.
| if (returnType == nil || supportedTypes.contains(returnType!)) && | |
| (sendType == nil || supportedTypes.contains(sendType!)) { | |
| if let sendType, supportedTypes.contains(sendType) { | |
| guard let surface, ghostty_surface_has_selection(surface) else { | |
| return super.validRequestor(forSendType: sendType, returnType: returnType) | |
| } | |
| } | |
| if (returnType == nil || supportedTypes.contains(returnType!)) && | |
| (sendType == nil || supportedTypes.contains(sendType!)) { | |
| if let sendType { | |
| guard let surface, ghostty_surface_has_selection(surface) else { | |
| return super.validRequestor(forSendType: sendType, returnType: returnType) | |
| } | |
| } | |
| return self | |
| } |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is kicking off a free cloud agent to fix this issue. This run is complimentary, but you can enable autofix for all future PRs in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3340e71. Configure here.
| let supportedTypes: [NSPasteboard.PasteboardType] = [ | ||
| .string, | ||
| .init("public.utf8-plain-text") | ||
| ] |
There was a problem hiding this comment.
Duplicate entries in supported pasteboard types array
Low Severity
NSPasteboard.PasteboardType.string has the raw value "public.utf8-plain-text", so .string and .init("public.utf8-plain-text") are the same type. The supportedTypes array contains two identical entries. If the intent was to also cover the legacy Objective-C "NSStringPboardType" pasteboard type, the wrong string was used. The codebase already defines a constant for this at GhosttyPasteboardHelper.utf8PlainTextType.
Reviewed by Cursor Bugbot for commit 3340e71. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/Workspace.swift`:
- Around line 6872-6875: The initial Bonsplit appearance is created with
bonsplitAppearance(from:backgroundOpacity:) which defaults tabTitleFontSize to
11, so update the call that currently uses
GhosttyApp.shared.defaultBackgroundColor and
GhosttyApp.shared.defaultBackgroundOpacity to also pass the configured font size
(GhosttyConfig.surfaceTabBarFontSize) into bonsplitAppearance's tabTitleFontSize
parameter so new workspaces start with the correct tab font size; locate the
call to bonsplitAppearance(…) and add the tabTitleFontSize argument set to
GhosttyConfig.surfaceTabBarFontSize.
In `@vendor/bonsplit`:
- Line 1: The parent repo updated the vendor/bonsplit submodule to commit
d65b4a8e1a733fa6ea7dac2d591289081b0dc2ef but that commit is not on bonsplit's
origin/main; push that commit to the bonsplit remote first, verify it is an
ancestor of origin/main (e.g., fetch and check merge-base for commit
d65b4a8e1a733fa6ea7dac2d591289081b0dc2ef), then update the parent repo's
vendor/bonsplit pointer to the pushed commit so the submodule reference is not
dangling.
🪄 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: 1325a357-47e4-435d-94a1-ac063e15e628
📒 Files selected for processing (2)
Sources/Workspace.swiftvendor/bonsplit
| @@ -1 +1 @@ | |||
| Subproject commit 098d9fa00e2b1d4712f1a46b818ee7d53d4aa31f | |||
| Subproject commit d65b4a8e1a733fa6ea7dac2d591289081b0dc2ef | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that the new bonsplit submodule commit exists on origin/main.
# Expected: Command exits 0 if commit is an ancestor of origin/main.
cd vendor/bonsplit
git fetch origin main --quiet
if git merge-base --is-ancestor d65b4a8e1a733fa6ea7dac2d591289081b0dc2ef origin/main; then
echo "✅ Commit d65b4a8 is on origin/main"
else
echo "❌ Commit d65b4a8 is NOT on origin/main — submodule pointer may be dangling"
exit 1
fiRepository: manaflow-ai/cmux
Length of output: 323
Push submodule commit to bonsplit's origin/main before updating the parent repo pointer.
Verification confirms that the new submodule commit d65b4a8e1a733fa6ea7dac2d591289081b0dc2ef does not exist on origin/main. Per repo policy, submodule commits must be pushed to the remote main branch before the parent repo updates its pointer to avoid dangling references and ensure reproducible builds.
Steps:
- Push the commit to bonsplit's
origin/main - Verify with:
cd vendor/bonsplit && git fetch origin main && git merge-base --is-ancestor d65b4a8e1a733fa6ea7dac2d591289081b0dc2ef origin/main - Update the submodule pointer in the parent repo
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@vendor/bonsplit` at line 1, The parent repo updated the vendor/bonsplit
submodule to commit d65b4a8e1a733fa6ea7dac2d591289081b0dc2ef but that commit is
not on bonsplit's origin/main; push that commit to the bonsplit remote first,
verify it is an ancestor of origin/main (e.g., fetch and check merge-base for
commit d65b4a8e1a733fa6ea7dac2d591289081b0dc2ef), then update the parent repo's
vendor/bonsplit pointer to the pushed commit so the submodule reference is not
dangling.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/Workspace.swift (1)
6786-6812: Use a consistent epsilon for tab font-size change detection.Line 6791 uses
0.001, which is looser than the0.0001tolerance used in nearby float change checks and can skip small real updates. Consider aligning this threshold.Proposed adjustment
- let fontSizeChanged = abs(currentAppearance.tabTitleFontSize - nextAppearance.tabTitleFontSize) > 0.001 + let fontSizeChanged = abs(currentAppearance.tabTitleFontSize - nextAppearance.tabTitleFontSize) > 0.0001Also applies to: 6818-6818
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/Workspace.swift` around lines 6786 - 6812, The tab font-size comparison in Workspace (in the block using Self.bonsplitAppearance and bonsplitController.configuration.appearance) currently uses an epsilon of 0.001; change that to the consistent 0.0001 used elsewhere so fontSizeChanged is computed with abs(currentAppearance.tabTitleFontSize - nextAppearance.tabTitleFontSize) > 0.0001, and update any other identical comparisons (e.g., the second occurrence around the same workspace appearance update) to the same 0.0001 tolerance to keep behavior consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@Sources/Workspace.swift`:
- Around line 6786-6812: The tab font-size comparison in Workspace (in the block
using Self.bonsplitAppearance and bonsplitController.configuration.appearance)
currently uses an epsilon of 0.001; change that to the consistent 0.0001 used
elsewhere so fontSizeChanged is computed with
abs(currentAppearance.tabTitleFontSize - nextAppearance.tabTitleFontSize) >
0.0001, and update any other identical comparisons (e.g., the second occurrence
around the same workspace appearance update) to the same 0.0001 tolerance to
keep behavior consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4633abd9-95e8-4b83-988a-1af5e7f0168b
📒 Files selected for processing (2)
Sources/GhosttyTerminalView.swiftSources/Workspace.swift
1 participant
Summary
GhosttyNSViewconform toNSServicesMenuRequestorso AppKit can expose native Services for selected terminal textBonsplitConfiguration.Appearance.tabTitleFontSizeusage so this checkout builds against the currentvendor/bonsplitAPIVerification
./scripts/reload.sh --tag issue-2698-services-menu --launchCloses #2698
Note
Medium Risk
Adds macOS Services pasteboard integration to terminal selection/input and tweaks workspace appearance updates; regressions could affect selection availability, text insertion, or tab-strip styling but scope is limited to UI/input paths.
Overview
Enables native macOS Services for terminal selections by making
GhosttyNSViewanNSServicesMenuRequestor, exporting the current selection to the Services pasteboard and accepting returned text back into the terminal via the existingsendText/paste path (including multiline payloads).Updates workspace chrome application to derive the next
BonsplitConfiguration.AppearanceviabonsplitAppearance(from: config)and only apply background/tab-title font size when they materially change, aligning with the current bonsplit API and tightening related debug logging/thresholds.Reviewed by Cursor Bugbot for commit d277da6. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Adds native macOS Services to the terminal so you can right-click selected text, run a Service, and have the result pasted back into the terminal. Also fixes tab title font sizing/divider regressions and aligns appearance updates with the current
vendor/bonsplitAPI. Closes #2698.New Features
GhosttyNSViewnow conforms toNSServicesMenuRequestorto expose Services for selected text.Bug Fixes
bonsplitAppearance(from:), apply only when changed, and initialize new workspaces from cached config to avoid startup mismatch.vendor/bonsplitto include the divider fix.Written for commit d277da6. Summary will update on new commits.
Summary by CodeRabbit
New Features
Refactor
Chore