Repository navigation
Make terminal Cmd+A use command input selection - #3897
austinywang wants to merge 22 commits into
Conversation
Terminal-focused Cmd+A currently reaches the menu-miss Ghostty binding path, where Ghostty's select_all action selects the entire terminal screen. The regression locks the expected responder boundary: cmux should route Cmd+A to the terminal responder's selectAll action so the responder can apply command-line text semantics instead of viewport semantics. Constraint: Do not run local tests for this issue; CI owns test execution. Rejected: Testing the Ghostty select_all implementation directly | that would only prove the existing viewport behavior, not the cmux routing contract that lets the terminal text engine own Cmd+A. Confidence: high Scope-risk: narrow Tested: Not run locally per issue instructions. Not-tested: CI execution of the new regression test.
Terminal Cmd+A now bypasses stale menu and Ghostty binding fallback so the focused terminal responder owns the text-selection intent. GhosttyNSView implements selectAll by using Ghostty's existing cursor/OSC-133-aware line selection path at the insertion point, falling back to the old select_all binding only when semantic cursor selection is unavailable. Constraint: User required origin-only cmux changes; avoided Ghostty submodule and xcframework changes. Rejected: Adding a new Ghostty C API in this PR | would require a Ghostty submodule commit and artifact path outside the user's push constraint. Rejected: Parsing prompt text in Swift | would duplicate Ghostty's existing OSC-133 semantic source of truth and break across shells. Confidence: medium Scope-risk: moderate Directive: Replace the synthetic triple-click bridge with a Ghostty C API once submodule/artifact changes are permitted; keep OSC-133 semantics as the source of truth. Tested: git diff --check Not-tested: Local tests/build not run per issue instructions; CI must run the regression.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR implements terminal Cmd+A select-all routing to respect command-line boundaries via new Ghostty cursor-line selection API, updates fork pinning and build infrastructure to conditionally handle crash-report-subdir based on Ghostty submodule capabilities, adds test coverage for routing behavior, and fixes AppDelegate preprocessor scope. ChangesTerminal Select-All and Build Configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 14 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (14 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 SummaryRoutes terminal Cmd+A through the
Confidence Score: 5/5Safe to merge — the Cmd+A routing change is narrowly scoped to the Ghostty first-responder path, the new direct API replaces the previously fragile synthetic-click side channel, and a regression test covers the key invariant. The core Swift change is a clean two-layer implementation: try the direct cursor-line API, fall back to the existing No files require special attention; Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant NSWindow
participant AppDelegate as AppDelegate.performKeyEquivalent
participant SRS as ShortcutRoutingSupport
participant GNV as GhosttyNSView.selectAll
participant Ghostty as ghostty_surface_select_cursor_line
User->>NSWindow: Cmd+A keyDown
NSWindow->>AppDelegate: performKeyEquivalent(event)
AppDelegate->>AppDelegate: firstResponderGhosttyView?
AppDelegate->>SRS: shouldRouteTerminalSelectAllToNaturalTextEngine(event)
SRS-->>AppDelegate: true (Cmd+A, no other modifiers)
AppDelegate->>GNV: selectAll(nil)
GNV->>GNV: ensureSurfaceReadyForInput()
GNV->>Ghostty: ghostty_surface_select_cursor_line_compat(surface)
alt cursor line selected (OSC 133 boundary available)
Ghostty-->>GNV: true
GNV->>GNV: invalidateTextInputCoordinates(selectionChanged: true)
else no semantic boundary
Ghostty-->>GNV: false
GNV->>GNV: performBindingAction("select_all")
end
AppDelegate-->>NSWindow: true (event consumed)
Reviews (17): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @IBAction override func selectAll(_ sender: Any?) { | ||
| _ = selectCommandLineAtCursorOrAll() | ||
| } | ||
|
|
||
| @discardableResult | ||
| private func selectCommandLineAtCursorOrAll() -> Bool { | ||
| guard let surface = ensureSurfaceReadyForInput() else { | ||
| requestInputRecoveryAfterSurfaceMiss(reason: "selectAll.missingSurface") | ||
| return false | ||
| } | ||
|
|
||
| if selectCommandLineAtCursor(surface: surface) { | ||
| invalidateTextInputCoordinates(selectionChanged: true) | ||
| return true | ||
| } | ||
|
|
||
| let selected = performBindingAction("select_all") | ||
| if selected { | ||
| invalidateTextInputCoordinates(selectionChanged: true) | ||
| } | ||
| return selected | ||
| } | ||
|
|
||
| private func selectCommandLineAtCursor(surface: ghostty_surface_t) -> Bool { | ||
| guard !ghostty_surface_mouse_captured(surface) else { return false } | ||
| guard bounds.width > 0, bounds.height > 0 else { return false } | ||
|
|
||
| var x: Double = 0 | ||
| var y: Double = 0 | ||
| var width: Double = cellSize.width | ||
| var height: Double = cellSize.height | ||
| ghostty_surface_ime_point(surface, &x, &y, &width, &height) | ||
|
|
||
| let clickX = clampSurfaceClickCoordinate( | ||
| x + max(width, cellSize.width, 1) * 0.5, | ||
| upperBound: bounds.width | ||
| ) | ||
| let clickY = clampSurfaceClickCoordinate( | ||
| y + max(height, cellSize.height, 1) * 0.5, | ||
| upperBound: bounds.height | ||
| ) | ||
| let resetX = clickX > max(cellSize.width * 2, 2) ? 0 : max(bounds.width - 1, 0) | ||
| let resetY = clickY > max(cellSize.height * 2, 2) ? 0 : max(bounds.height - 1, 0) | ||
|
|
||
| let mods = ghostty_input_mods_e(rawValue: GHOSTTY_MODS_NONE.rawValue) ?? GHOSTTY_MODS_NONE | ||
| _ = ghostty_surface_clear_selection_compat(surface) | ||
|
|
||
| // Force Ghostty's multi-click state to restart before issuing the triple | ||
| // click that reaches Screen.selectLine and its OSC 133 input boundaries. | ||
| guard sendSurfaceLeftClick(surface: surface, x: resetX, y: resetY, mods: mods) else { | ||
| return false | ||
| } | ||
|
|
||
| for _ in 0 ..< 3 { | ||
| guard sendSurfaceLeftClick(surface: surface, x: clickX, y: clickY, mods: mods) else { | ||
| return false | ||
| } | ||
| } | ||
|
|
||
| return ghostty_surface_has_selection(surface) | ||
| } |
There was a problem hiding this comment.
selectAll via 4 synthetic mouse clicks is a fragile side-channel that patches the symptom rather than naming the invariant
The implementation fires one reset click to a corner (to move Ghostty's internal click-position state away) and then three rapid clicks to the cursor cell (to trigger triple-click line selection). This path works because Ghostty's multi-click counter resets when the click position changes significantly, and three consecutive same-position clicks counts as a triple-click. Any change to Ghostty's click-state management — e.g., adding click-position hysteresis, changing how synchronous synthetic events are timestamped, or gating triple-click on a timer — silently breaks the feature without a test failure.
A direct surface API for "select the input line at the cursor" (or "emit a triple-click at the cursor cell") would be the correct contract. If that API doesn't exist yet in ghostty_surface_*, it would be worth requesting or stubbing so the click-simulation path can be removed once it lands.
Rule Used: Flag Swift fixes that patch symptoms while leaving... (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!
There was a problem hiding this comment.
Addressed by replacing the synthetic-click selection path with the direct Ghostty cursor-line selection API, so the fragile mouse side channel is no longer used.
— Claude Code
There was a problem hiding this comment.
Addressed by removing the synthetic click path. Terminal select-all now calls Ghostty's direct cursor-line selection API, with the existing select_all binding only as the fallback path when that API cannot select.
— Claude Code
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?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 OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d71fbb0. Configure here.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@docs/ghostty-fork.md`:
- Around line 15-22: Update the pinned-head metadata so the downstream
release/tag and checksum entries match the new fork head 7e4cf8a2f: locate the
section that still references the older xcframework-fe972c095 artifact chain and
either replace that release-tag/checksum with the values corresponding to commit
7e4cf8a2f (release tag, artifact name, and checksum) or add an explicit sentence
stating why the downstream artifact intentionally remains on the prior
xcframework-fe972c095 chain; ensure references to PRs (manaflow-ai/ghostty#53
and `#56`) and the cmux theme picker/Metal renderer notes remain intact.
In `@ghostty`:
- Line 1: The pinned ghostty submodule was bumped to commit
7e4cf8a2fd2539d68240aa046e2cc892d21d2e89 but the checksum file
ghosttykit-checksums.txt is missing an entry for that exact revision; add a
checksum line for commit 7e4cf8a2fd2539d68240aa046e2cc892d21d2e89 to
ghosttykit-checksums.txt (matching the file's existing format) so CI can verify
the artifact, or revert the ghostty bump if you cannot produce the checksum yet.
🪄 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: db248747-8da8-4146-a5a6-5e03ff270036
📒 Files selected for processing (7)
Sources/App/ShortcutRoutingSupport.swiftSources/AppDelegate.swiftSources/GhosttyTerminalView.swiftcmuxTests/AppDelegateShortcutRoutingTests.swiftcmuxTests/TerminalAndGhosttyTests.swiftdocs/ghostty-fork.mdghostty
Stale CodeRabbit review on an older commit. The two actionable threads it referenced are resolved, CodeRabbit has re-reviewed the current commit as commented/pass, and the GhosttyKit checksum/download path now validates.
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 `@Sources/GhosttyTerminalView.swift`:
- Around line 7015-7017: The IBAction method selectAll(_:) has unnecessarily
broad visibility and should be private to satisfy the private_action lint rule;
update the method declaration for selectAll(_:) to be private while keeping it
as an IBAction so it remains connected to Interface Builder, e.g. change the
declaration for selectAll(_:) and leave the call to
selectCommandLineAtCursorOrAll() unchanged.
🪄 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: 134b724b-8079-4647-a9df-03475c4fd038
📒 Files selected for processing (6)
Sources/App/ShortcutRoutingSupport.swiftSources/AppDelegate.swiftSources/GhosttyTerminalView.swiftcmuxTests/WorkspaceUnitTests.swiftdocs/ghostty-fork.mdscripts/ghosttykit-checksums.txt
Stale CodeRabbit review: the private_action finding was addressed in commit 280840a with a compile-safe lint suppression because Swift requires NSResponder overrides to remain internal. The review thread is resolved and the current CodeRabbit check is passing.
…-text-engine # Conflicts: # ghostty # scripts/ghosttykit-checksums.txt
…-text-engine # Conflicts: # .github/workflows/build-ghosttykit.yml # docs/ghostty-fork.md # ghostty
lawrencecchen
added
the
stale-revisit
This branch was successfully deployed
3 participants
Closes #3802
Repro
select_allbinding, which callsScreen.selectAll()and selects the terminal viewport/screen contents.Changes
selectAll(_:)path instead of the Ghostty binding fallback.selectAll(_:)by using Ghostty's cursor line-selection path, which preserves OSC 133 input boundaries, and falls back to the oldselect_allonly when that path is unavailable.Verification
git diff --checkNote
Medium Risk
Touches macOS key-equivalent routing and terminal selection behavior, which can subtly affect shortcut handling and text selection across the app. Also changes GhosttyKit build/download tagging logic, which could break CI artifact resolution if misdetected.
Overview
Terminal Cmd+A now selects the current input line instead of the whole viewport.
NSWindowkey-equivalent routing detects Cmd+A when a Ghostty terminal is focused and calls the terminal responder’sselectAll(_:), avoiding the menu-miss/binding fallback.Terminal
selectAll(_:)is implemented with Ghostty’s semantic cursor-line selection.GhosttyTerminalViewadds aselectAlloverride that prefers the newghostty_surface_select_cursor_lineC API (OSC-133 aware), falling back to the existingselect_allbinding when unavailable, and updates text-input coordinates.GhosttyKit CI/scripts now auto-detect crash-report-subdir support and tag format. Workflows and helper scripts resolve whether to use flavored
xcframework-<sha>-<flavor>tags and-Dcrash-report-subdirat build/download time, and tests switch to the shareddownload-prebuilt-ghosttykit.sh; docs/checksums are updated for the new pinned Ghostty fork SHA and archive hash, and a regression test asserts Cmd+A behavior.Reviewed by Cursor Bugbot for commit b461417. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by cubic
Cmd+A in a focused terminal now selects the current command/input line via Ghostty’s cursor-line selection (OSC‑133 aware), not the entire viewport. Fixes #3802.
Bug Fixes
NSWindow.performKeyEquivalentto the terminal responder’sselectAll(_:)before menu/binding fallback; add a regression test.GhosttyNSView.selectAll(_:)to preferghostty_surface_select_cursor_line(semantic cursor line), falling back toselect_all; update text-input coordinates.Dependencies
ghosttyto a fork exposingghostty_surface_select_cursor_line; update fork docs and the checksum inscripts/ghosttykit-checksums.txt.GhosttyKitbuild/download scripts and CI auto-detect-Dcrash-report-subdirsupport and set release tags/flags accordingly; switch test workflows toscripts/download-prebuilt-ghosttykit.sh.Written for commit b461417. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Bug Fixes
Chores