Add terminal right-click paste mode with long-press context menu + delay setting#2320
Open
wing2005 wants to merge 17 commits intomanaflow-ai:mainfrom
Open
Add terminal right-click paste mode with long-press context menu + delay setting#2320wing2005 wants to merge 17 commits intomanaflow-ai:mainfrom
wing2005 wants to merge 17 commits intomanaflow-ai:mainfrom
Conversation
Expose terminal-specific settings to switch right click from context menu to paste, with an optional long-right-click context menu fallback and configurable delay so users can tune interaction speed. Made-with: Cursor
|
@wing2005 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
|
To use Codex here, create a Codex account and connect to github. |
|
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:
📝 WalkthroughWalkthroughRemoves “React Grab” localization keys, adds en/ja localization for terminal right‑click settings and long‑press duration, introduces persisted terminal right‑click preferences and Settings UI, implements long‑press scheduling/cancellation and paste-vs-context‑menu behavior in the terminal view, and adjusts terminal context‑menu keying. Changes
Sequence DiagramsequenceDiagram
participant User
participant SettingsView
participant UserDefaults
participant GhosttyNSView
participant DispatchQueue
participant Clipboard
User->>SettingsView: change right-click behavior/delay
SettingsView->>UserDefaults: save setting value
User->>GhosttyNSView: right-click on terminal
GhosttyNSView->>UserDefaults: read TerminalRightClickSettings
alt Behavior == pasteFromClipboard
GhosttyNSView->>DispatchQueue: schedule delayed work item (after delay)
Note right of DispatchQueue: rgba(100,149,237,0.5)
alt long-press fires
DispatchQueue->>GhosttyNSView: execute work item -> show context menu
GhosttyNSView->>GhosttyNSView: set didTriggerContextMenuOnLongRightClick
else short click (release before delay)
User->>GhosttyNSView: right-up before fire
GhosttyNSView->>DispatchQueue: cancel work item
GhosttyNSView->>Clipboard: paste(nil)
end
else Behavior == contextMenu
GhosttyNSView->>GhosttyNSView: build & show context menu immediately
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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)
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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 3 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/cmuxApp.swift">
<violation number="1" location="Sources/cmuxApp.swift:3915">
P2: Reset All Settings doesn’t restore the new terminal right-click preferences, so these values stay customized after a global reset.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Contributor
Greptile SummaryThis PR adds a configurable terminal right-click behavior: the default context-menu mode is unchanged, while a new "Paste from Clipboard" mode turns short right-clicks into paste and optionally opens the context menu on a long press with a configurable delay (Fast/Default/Slow). All three previously flagged issues (drag-triggers-unintended-paste, mid-gesture settings change, stale modifier flags in long-press timer) are resolved in this revision. The one minor leftover: Confidence Score: 5/5Safe to merge; all previous blocking concerns are resolved and the only remaining finding is a dead localization string. All P0/P1 findings from prior review rounds are addressed. The single remaining finding (orphaned terminalContextMenu.resetTerminal localization key) is a P2 style/cleanup issue that does not affect runtime behaviour. Resources/Localizable.xcstrings — orphaned terminalContextMenu.resetTerminal key should be cleaned up. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant GhosttyNSView
participant Timer as DispatchWorkItem (long-press)
participant Surface as GhosttyTerminalSurface
participant Clipboard
User->>GhosttyNSView: rightMouseDown
GhosttyNSView->>GhosttyNSView: capture behavior (paste mode)
GhosttyNSView->>Timer: arm long-press timer (0.20–0.45 s)
alt long press fires before release
Timer->>GhosttyNSView: buildContextMenu (fresh modifierFlags)
GhosttyNSView->>Surface: GHOSTTY_MOUSE_PRESS (right)
GhosttyNSView->>User: popUp context menu
GhosttyNSView->>GhosttyNSView: didTriggerContextMenuOnLongRightClick = true
User->>GhosttyNSView: rightMouseUp
GhosttyNSView->>GhosttyNSView: skip paste (context menu already shown)
else drag > 4 pt threshold
User->>GhosttyNSView: rightMouseDragged
GhosttyNSView->>Timer: cancel long-press timer
GhosttyNSView->>GhosttyNSView: didRightMouseDrag = true
User->>GhosttyNSView: rightMouseUp
GhosttyNSView->>GhosttyNSView: skip paste (drag detected)
else short click, no drag
User->>GhosttyNSView: rightMouseUp
GhosttyNSView->>Timer: cancel long-press timer
GhosttyNSView->>Clipboard: paste(nil)
end
Reviews (2): Last reviewed commit: "Merge origin/main into feat/terminal-rig..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?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.
Actionable comments posted: 1
🤖 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 3915-3920: The resetAllSettings() implementation in SettingsView
doesn't reset the new AppStorage fields terminalRightClickBehavior,
terminalRightClickLongPressContextMenuEnabled, and
terminalRightClickLongPressDuration; update resetAllSettings() to explicitly set
terminalRightClickBehavior =
TerminalRightClickSettings.defaultBehavior.rawValue,
terminalRightClickLongPressContextMenuEnabled =
TerminalRightClickSettings.defaultLongPressContextMenuEnabled, and
terminalRightClickLongPressDuration =
TerminalRightClickSettings.defaultLongPressDuration so the “Reset All Settings”
flow returns the terminal right-click settings to their defaults.
🪄 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: 1df57049-4445-4ca3-a193-fd2366156883
📒 Files selected for processing (3)
Resources/Localizable.xcstringsSources/GhosttyTerminalView.swiftSources/cmuxApp.swift
Ensure the global settings reset flow restores terminal right-click behavior, long-press menu toggle, and long-press delay to their defaults. Made-with: Cursor
Author
|
Addressed the reset-flow issue identified by cubic and CodeRabbit. Fix
Commit
|
Track right-button drag state during paste-mode gestures so releasing after movement no longer triggers clipboard paste, while preserving long-press context menu behavior. Made-with: Cursor
Author
|
Updated PR with a follow-up fix for right-click drag jitter. Fix
Commit
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/GhosttyTerminalView.swift`:
- Around line 6442-6447: The menu(for:) method currently returns nil when
TerminalRightClickSettings.behavior() == .pasteFromClipboard which suppresses
Control-click and other AppKit-triggered contextual menus; remove that branch so
menu(for:) always returns buildContextMenu(for: event) (i.e., delete the
conditional early-return) and rely on rightMouseDown(with:) and
armLongRightClickContextMenuIfNeeded to handle paste-mode right-click behavior
and long-press behavior respectively.
🪄 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: 226f3192-4f54-46b7-8c05-2816ddc5e227
📒 Files selected for processing (1)
Sources/GhosttyTerminalView.swift
Author
|
Addressed the latest CodeRabbit contextual-menu concern. Fix
Commit
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/GhosttyTerminalView.swift (1)
6446-6493:⚠️ Potential issue | 🟡 MinorLocalize the extracted context-menu titles.
Now that the menu is constructed here, titles like
Trigger Flash,Copy,Paste, and the split actions are still hard-coded English. Please switch them toString(localized:..., defaultValue:...)and add xcstrings entries so the new right-click flows stay translated in supported locales.As per coding guidelines, “All user-facing strings must be localized using
String(localized: "key.name", defaultValue: "English text")… Never use bare string literals in … UI elements”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 6446 - 6493, The context menu in buildContextMenu uses hard-coded English titles; replace each literal title ("Trigger Flash", "Copy", "Paste", "Split Horizontally", "Split Vertically") with localized variants using String(localized: "key.name", defaultValue: "English text") and update the corresponding NSMenuItem creation calls (the items returned for selectors triggerFlash(_:), copy(_:), paste(_:), splitHorizontally(_:), splitVertically(_:)) to use those localized strings; also add matching entries to the .xcstrings resource file for each key name so translations can be provided.
🤖 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/GhosttyTerminalView.swift`:
- Around line 6367-6375: The long-press context-menu timer is being cancelled on
any tiny right-button movement which prevents the menu from ever firing; in the
right-mouse-drag handling path that checks TerminalRightClickSettings.behavior()
and ghostty_surface_mouse_captured(surface), preserve the didRightMouseDrag flag
for paste suppression but only invalidate/cancel the long-press timer and set
didTriggerContextMenuOnLongRightClick when the drag distance exceeds a small
threshold (e.g., a few pixels) instead of on the first drag event; update the
logic in the methods around requestPointerFocusRecovery(),
armLongRightClickContextMenuIfNeeded(with:), and the right-drag handling so
minor jitter just sets didRightMouseDrag but does not cancel the timer until
movement > threshold.
---
Outside diff comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 6446-6493: The context menu in buildContextMenu uses hard-coded
English titles; replace each literal title ("Trigger Flash", "Copy", "Paste",
"Split Horizontally", "Split Vertically") with localized variants using
String(localized: "key.name", defaultValue: "English text") and update the
corresponding NSMenuItem creation calls (the items returned for selectors
triggerFlash(_:), copy(_:), paste(_:), splitHorizontally(_:),
splitVertically(_:)) to use those localized strings; also add matching entries
to the .xcstrings resource file for each key name so translations can be
provided.
🪄 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: bcf73f0e-aa45-482a-88c0-63fc1cc52627
📒 Files selected for processing (1)
Sources/GhosttyTerminalView.swift
wing2005
force-pushed
the
feat/terminal-right-click-paste-menu
branch
from
0c6ede8 to
84a5903
Compare
Author
|
Patched and amended with the latest review fixes:\n\n- Added long-right-click drag-cancel threshold so tiny pointer jitter no longer cancels the long-press timer ( threshold).\n- Localized terminal context menu titles (, , , , ) with EN/JA keys in .\n\nAmended commit: (force-pushed). |
Author
|
Correction to previous comment (shell formatting issue): Patched and amended with the latest review fixes:
Amended commit: 84a5903 (force-pushed). |
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Preserve AppKit contextual menu paths while using right-click paste mode, make long-right-click more reliable, and ensure right-click gesture handling uses consistent behavior/modifier state. Also localize terminal context menu item titles. Made-with: Cursor
wing2005
force-pushed
the
feat/terminal-right-click-paste-menu
branch
from
84a5903 to
a7d0a39
Compare
Resolve cmuxApp settings-section conflicts while keeping terminal right-click paste/long-press behavior changes and main-branch copy-on-select settings updates. Made-with: Cursor
Reconcile localization key namespace updates from main with terminal right-click paste/long-press changes by standardizing on terminalContextMenu keys and preserving new right-click settings entries. Made-with: Cursor
|
Waiting for this PR to be merged. We set |
Integrate latest main branch updates while preserving terminal right-click paste/long-press behavior and associated context-menu localization changes. Made-with: Cursor
Remove the leftover SettingsView helper that references terminalCopyOnSelectEnabled, which no longer exists on current main. Made-with: Cursor
Author
|
PR readiness pass complete on latest head (777bc31):\n- Branch is mergeable with main (no conflicts).\n- Unresolved review threads: 0.\n- All non-Vercel checks are passing (Socket, cubic, CodeRabbit).\n\nRemaining blocker is Vercel authorization for the Manaflow team. A team member needs to authorize via the Vercel check link, then re-run that check. |
Resolve GhosttyTerminalView conflicts: keep main Cmd-hover/word-path and layer-background config paths; preserve right-click context menu behavior. Made-with: Cursor
- Restore GhosttyNSView wordPathHoverActive property and cursor cleanup in viewDidMoveToWindow / mouseExited (main Cmd-path hover). - Extract terminal right-click App settings into terminalRightClickAppSettingsRows() to satisfy Swift type-checker; use configurationReview: .settingsOnly on those rows until settings.json paths are registered. Made-with: Cursor
- Add app.terminalRightClickBehavior, long-press menu toggle, and duration to CmuxSettingsFileStore supported paths, parseAppSection, and default template. - Document keys in cmux-settings.schema.json. - Point Settings UI rows at the new app.* configurationReview paths. Made-with: Cursor
Resolve conflicts: keep terminal right-click settings UI, merge Terminal scroll bar settings from main, restore prepareSurfaceForPaste helpers, add React Grab localization entries, align terminal paste JA/ko strings. Made-with: Cursor
Keep main's DEBUG surface helpers in GhosttyTerminalView (required by recordDebugForceRefresh call sites). Made-with: Cursor
Resolve cmuxApp.swift conflict: main moved runtime settings to WorkspaceRuntimeSettings.swift; keep that structure and add TerminalRightClickSettings there instead of duplicating at top of cmuxApp. Made-with: Cursor
Author
|
@codex review |
|
To use Codex here, create a Codex account and connect to github. |
@wing2005 I have started the AI code review. It will take a few minutes to complete. |
|
🧠 Learnings used✅ Actions performedReview triggered.
|
|
To use Codex here, create a Codex account and connect to github. |
There was a problem hiding this comment.
3 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="Resources/Localizable.xcstrings">
<violation number="1" location="Resources/Localizable.xcstrings:11289">
P2: Removed localization keys are still referenced by UI/menu/shortcut code, causing localization fallback regressions.</violation>
<violation number="2" location="Resources/Localizable.xcstrings:88589">
P2: Repurposed `resetTerminal` entry retains stale Korean text from `triggerFlash`, causing incorrect localization.</violation>
</file>
<file name="Sources/KeyboardShortcutSettingsFileStore.swift">
<violation number="1" location="Sources/KeyboardShortcutSettingsFileStore.swift:437">
P2: Invalid `app.terminalRightClickBehavior` value aborts parsing of the rest of the `app` section due to an early `return`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/GhosttyTerminalView.swift (1)
4305-4319:⚠️ Potential issue | 🟠 MajorKeep closed terminal surfaces from being recreated.
After
teardownSurface()marks the lifecycle closed and clearssurface, stale attach/background-start callbacks can still callcreateSurface(for:). That can spawn a new Ghostty runtime for a panel thatcanAcceptPortalBinding(...)would reject as non-live.Proposed lifecycle guard
func attachToView(_ view: GhosttyNSView) { `#if` DEBUG dlog( "surface.attach surface=\(id.uuidString.prefix(5)) view=\(Unmanaged.passUnretained(view).toOpaque()) " + "attached=\(attachedView != nil ? 1 : 0) hasSurface=\(surface != nil ? 1 : 0) inWindow=\(view.window != nil ? 1 : 0)" ) `#endif` + guard portalLifecycleState == .live else { +#if DEBUG + dlog( + "surface.attach.skip surface=\(id.uuidString.prefix(5)) " + + "reason=lifecycleState state=\(portalLifecycleState.rawValue)" + ) +#endif + return + } // If already attached to this view, nothing to do.private func createSurface(for view: GhosttyNSView) { + guard portalLifecycleState == .live else { +#if DEBUG + dlog( + "surface.create.skip surface=\(id.uuidString.prefix(5)) " + + "reason=lifecycleState state=\(portalLifecycleState.rawValue)" + ) +#endif + return + } + `#if` DEBUG let resourcesDir = getenv("GHOSTTY_RESOURCES_DIR").flatMap { String(cString: $0) } ?? "(unset)"- guard surface == nil, attachedView != nil else { return } + guard portalLifecycleState == .live, surface == nil, attachedView != nil else { return } guard !backgroundSurfaceStartQueued else { return } backgroundSurfaceStartQueued = true DispatchQueue.main.async { [weak self] in guard let self else { return } self.backgroundSurfaceStartQueued = false + guard self.portalLifecycleState == .live else { return } guard self.surface == nil, let view = self.attachedView else { return }Also applies to: 4334-4341, 4918-4938
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 4305 - 4319, The attach/background-start paths call createSurface(for:) even after teardownSurface() has set the lifecycle to closed and cleared surface; add a lifecycle guard so these callbacks bail out when the panel is closed: before calling createSurface(for:) (in the branches around surface == nil at the shown sites and the other similar blocks), check the lifecycle/closed flag (or call canAcceptPortalBinding(...) / a new isClosed/isLive property) and return if closed, ensuring createSurface(for:) is never invoked for a closed panel and preventing a new Ghostty runtime from being spawned for non-live panels.
🧹 Nitpick comments (2)
Sources/App/WorkspaceRuntimeSettings.swift (1)
123-126: Centralize the long-press clamp bounds.The 0.15/1.00 bounds are duplicated in
Sources/KeyboardShortcutSettingsFileStore.swift(Line 445). Exposing them here avoids drift if the range is ever tuned.♻️ Proposed change
static let defaultLongPressDuration: TimeInterval = 0.30 + static let minLongPressDuration: TimeInterval = 0.15 + static let maxLongPressDuration: TimeInterval = 1.00 @@ static func longPressDuration(defaults: UserDefaults = .standard) -> TimeInterval { let raw = defaults.object(forKey: longPressDurationKey) as? Double ?? defaultLongPressDuration - return min(max(raw, 0.15), 1.00) + return min(max(raw, minLongPressDuration), maxLongPressDuration) }And in
KeyboardShortcutSettingsFileStore.swift:- let clamped = min(max(raw, 0.15), 1.00) + let clamped = min( + max(raw, TerminalRightClickSettings.minLongPressDuration), + TerminalRightClickSettings.maxLongPressDuration + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/App/WorkspaceRuntimeSettings.swift` around lines 123 - 126, Introduce centralized min/max constants in WorkspaceRuntimeSettings and use them instead of hard-coded literals: add e.g. static let minLongPressDuration: TimeInterval = 0.15 and static let maxLongPressDuration: TimeInterval = 1.0 alongside existing defaultLongPressDuration and longPressDurationKey, then update the longPressDuration(defaults:) implementation to clamp using those constants (min/max) rather than 0.15/1.00; finally update the duplicate clamp in KeyboardShortcutSettingsFileStore (referencing the KeyboardShortcutSettingsFileStore type) to use WorkspaceRuntimeSettings.minLongPressDuration and .maxLongPressDuration so both places share the same source of truth and run tests to confirm behavior.Sources/GhosttyTerminalView.swift (1)
8148-8173: Add DEBUGdlog()coverage for the new right-click gesture paths.The new paste-mode and long-press branches are mouse event paths, but they do not emit DEBUG events, making right-click regressions hard to diagnose.
As per coding guidelines, All debug events (keys, mouse, focus, splits, tabs) must be logged to the unified debug log using the
dlog()function and wrapped in#if DEBUG/#endifpreprocessor directives.Also applies to: 8309-8329
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/GhosttyTerminalView.swift` around lines 8148 - 8173, Add DEBUG dlog() calls wrapped in `#if` DEBUG / `#endif` to cover the new right-click gesture paths in rightMouseDown(with:): log entering the pasteFromClipboard branch (before requestPointerFocusRecovery/armLongRightClickContextMenuIfNeeded), log when cancelLongRightClickContextMenu() is invoked, and log the mouse position/button send (before calling ghostty_surface_mouse_pos and ghostty_surface_mouse_button); similarly add matching dlog() coverage in the corresponding long-press/context-menu handling code paths (e.g., armLongRightClickContextMenuIfNeeded and the code around ghostty_surface_mouse_button) so all right-click branches emit unified debug events.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Resources/Localizable.xcstrings`:
- Around line 88589-88604: The Korean localization for
terminalContextMenu.resetTerminal is stale (value '플래시 실행' from old key) —
update the "ko" stringUnit for terminalContextMenu.resetTerminal to the correct
Korean translation for "Reset Terminal" (e.g., "터미널 재설정") or change its
stringUnit state to "needs review" so the English fallback is used; modify the
"ko" block for terminalContextMenu.resetTerminal accordingly without altering
the English or Japanese entries.
In `@Sources/cmuxApp.swift`:
- Around line 4481-4568: The picker for terminal long-right-click delay only
offers fixed tags (0.20, 0.30, 0.45) so arbitrary clamped values (e.g. 0.25)
don't show a coherent selection; update the SettingsPickerRow (selection:
terminalLongRightClickDurationSelection) to append a dynamic "Custom" option
when terminalRightClickLongPressDuration is not equal to any of the preset tags
— render a localized label like "Custom (0.25s)" (using the current
terminalRightClickLongPressDuration) and tag it with the actual
terminalRightClickLongPressDuration so the picker reflects and preserves
arbitrary clamped values. Ensure the existing binding
terminalLongRightClickDurationSelection and clamping logic remain unchanged.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 8175-8188: The rightMouseDragged handler currently bails out when
`surface` is nil, leaving the delayed long-right-click work item and gesture
flags alive; move or duplicate the cleanup so that
`cancelLongRightClickContextMenu()` is invoked and gesture state
(`didRightMouseDrag`, `rightMouseDownLocation` checks) is handled regardless of
`surface` presence before any early return, then only call
`ghostty_surface_mouse_pos(...)` if `surface` exists; apply the same change to
the corresponding `rightMouseUp` handler (the block around lines 8190-8205) so
pending menu work items and flags are always cleared even if the `surface` has
been torn down or detached.
---
Outside diff comments:
In `@Sources/GhosttyTerminalView.swift`:
- Around line 4305-4319: The attach/background-start paths call
createSurface(for:) even after teardownSurface() has set the lifecycle to closed
and cleared surface; add a lifecycle guard so these callbacks bail out when the
panel is closed: before calling createSurface(for:) (in the branches around
surface == nil at the shown sites and the other similar blocks), check the
lifecycle/closed flag (or call canAcceptPortalBinding(...) / a new
isClosed/isLive property) and return if closed, ensuring createSurface(for:) is
never invoked for a closed panel and preventing a new Ghostty runtime from being
spawned for non-live panels.
---
Nitpick comments:
In `@Sources/App/WorkspaceRuntimeSettings.swift`:
- Around line 123-126: Introduce centralized min/max constants in
WorkspaceRuntimeSettings and use them instead of hard-coded literals: add e.g.
static let minLongPressDuration: TimeInterval = 0.15 and static let
maxLongPressDuration: TimeInterval = 1.0 alongside existing
defaultLongPressDuration and longPressDurationKey, then update the
longPressDuration(defaults:) implementation to clamp using those constants
(min/max) rather than 0.15/1.00; finally update the duplicate clamp in
KeyboardShortcutSettingsFileStore (referencing the
KeyboardShortcutSettingsFileStore type) to use
WorkspaceRuntimeSettings.minLongPressDuration and .maxLongPressDuration so both
places share the same source of truth and run tests to confirm behavior.
In `@Sources/GhosttyTerminalView.swift`:
- Around line 8148-8173: Add DEBUG dlog() calls wrapped in `#if` DEBUG / `#endif` to
cover the new right-click gesture paths in rightMouseDown(with:): log entering
the pasteFromClipboard branch (before
requestPointerFocusRecovery/armLongRightClickContextMenuIfNeeded), log when
cancelLongRightClickContextMenu() is invoked, and log the mouse position/button
send (before calling ghostty_surface_mouse_pos and
ghostty_surface_mouse_button); similarly add matching dlog() coverage in the
corresponding long-press/context-menu handling code paths (e.g.,
armLongRightClickContextMenuIfNeeded and the code around
ghostty_surface_mouse_button) so all right-click branches emit unified debug
events.
🪄 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: be8fa21f-62fb-4295-a046-caa4035ea2cf
📒 Files selected for processing (5)
Resources/Localizable.xcstringsSources/App/WorkspaceRuntimeSettings.swiftSources/GhosttyTerminalView.swiftSources/KeyboardShortcutSettingsFileStore.swiftSources/cmuxApp.swift
…ure cleanup - Restore browser.reactGrab strings; remove duplicate resetTerminal entry - Add custom long-press duration picker row for non-preset values - Stop aborting full app section parse on invalid terminalRightClickBehavior - Clear long-press state on right-drag/up even when surface is nil Made-with: Cursor
Resolve GhosttyTerminalView: keep main's allowsRuntimeSurfaceCreation gating (attachView + createSurface + requestBackgroundSurfaceStartIfNeeded) and add the helper next to recordRuntimeSurfaceCreation. Made-with: Cursor
Merge had advanced the pointer to an older snapshot without BonsplitConfiguration.SplitActionButton; restore main's revision so CmuxConfig builds. Made-with: Cursor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
./scripts/reload.sh --tag right-click-paste --launchTerminal Right Clickpicker appears in Settings.Long Right Click Opens Context Menutoggle appears when paste mode is selected.Long Right Click Delaypicker appears when long-right-click context menu is enabled.Demo Video
Review Trigger (Copy/Paste as PR comment)
Checklist
Made with Cursor
Summary by cubic
Adds a terminal-only right‑click mode: context menu (default) or paste, with an optional long‑press menu and a configurable delay. Settings are exposed via
app.*keys insettings.jsonand documented in the schema.New Features
settings.jsondoesn’t match presets.app.terminalRightClickBehavior,app.terminalRightClickLongPressContextMenuEnabled,app.terminalRightClickLongPressDuration(defaults included).Bug Fixes
terminalContextMenulocalization keys; restoredbrowser.reactGrabstrings; removed duplicate “Reset Terminal” menu entry.app.terminalRightClickBehavior: invalid values no longer abort settings parse.vendor/bonsplitsubmodule pointer toorigin/mainto restore build.Written for commit dc47a10. Summary will update on new commits.
Summary by CodeRabbit
New Features
Localization