Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion Sources/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14497,7 +14497,15 @@ private extension NSWindow {
}
#endif
if !consumedByMenu {
// Fall through to the original performKeyEquivalent path below.
// After a direct-to-menu miss, let Ghostty resolve the command key
// through its normal binding path so user key overrides still win.
let consumedByGhostty = firstResponderGhosttyView?.performKeyEquivalentAfterMenuMiss(with: event) == true
#if DEBUG
dlog(" → mainMenu miss; ghostty command path: \(consumedByGhostty)")
#endif
if consumedByGhostty {
return true
}
} else {
#if DEBUG
dlog(" → consumed by mainMenu (bypassed SwiftUI)")
Expand Down
22 changes: 20 additions & 2 deletions Sources/GhosttyTerminalView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5927,6 +5927,15 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
#endif
}

@discardableResult
func prepareSurfaceForPaste(reason: String) -> Bool {
guard ensureSurfaceReadyForInput() != nil else {
requestInputRecoveryAfterSurfaceMiss(reason: reason)
return false
}
return true
}

func performBindingAction(_ action: String) -> Bool {
guard let surface = surface else { return false }
return action.withCString { cString in
Expand Down Expand Up @@ -6149,11 +6158,13 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
// MARK: - Clipboard paste

@IBAction func paste(_ sender: Any?) {
guard prepareSurfaceForPaste(reason: "paste.missingSurface") else { return }
_ = performBindingAction("paste_from_clipboard")
}

/// Pastes clipboard text as plain text, stripping any rich formatting.
@IBAction func pasteAsPlainText(_ sender: Any?) {
guard prepareSurfaceForPaste(reason: "pasteAsPlainText.missingSurface") else { return }
_ = performBindingAction("paste_from_clipboard")
}

Expand Down Expand Up @@ -6433,6 +6444,14 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
}

override func performKeyEquivalent(with event: NSEvent) -> Bool {
performKeyEquivalent(with: event, shouldRetryMainMenu: true)
}

func performKeyEquivalentAfterMenuMiss(with event: NSEvent) -> Bool {
performKeyEquivalent(with: event, shouldRetryMainMenu: false)
}

private func performKeyEquivalent(with event: NSEvent, shouldRetryMainMenu: Bool) -> Bool {
#if DEBUG
let typingTimingStart = CmuxTypingTiming.start()
defer {
Expand Down Expand Up @@ -6504,14 +6523,13 @@ class GhosttyNSView: NSView, NSUserInterfaceValidations {
if let bindingFlags {
let isConsumed = (bindingFlags.rawValue & GHOSTTY_BINDING_FLAGS_CONSUMED.rawValue) != 0
let isAll = (bindingFlags.rawValue & GHOSTTY_BINDING_FLAGS_ALL.rawValue) != 0
let isPerformable = (bindingFlags.rawValue & GHOSTTY_BINDING_FLAGS_PERFORMABLE.rawValue) != 0

// If the binding is consumed and not meant for the menu, allow menu first.
// Performable bindings (e.g. paste_from_clipboard) also need the menu
// path so that Edit > Paste handles Cmd+V instead of keyDown double-
// firing the clipboard request through both interpretKeyEvents and
// ghostty_surface_key.
if isConsumed && !isAll && keySequence.isEmpty && keyTables.isEmpty {
if shouldRetryMainMenu && isConsumed && !isAll && keySequence.isEmpty && keyTables.isEmpty {
if let menu = NSApp.mainMenu, menu.performKeyEquivalent(with: event) {
return true
}
Expand Down
68 changes: 68 additions & 0 deletions cmuxTests/AppDelegateShortcutRoutingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,25 @@ private final class FakeWKInspectorContainerView: NSView {}
private final class FocusableTestView: NSView {
override var acceptsFirstResponder: Bool { true }
}
private final class GhosttyCommandEquivalentProbeView: GhosttyNSView {
var afterMenuMissCallCount = 0
var pasteCallCount = 0
var pasteAsPlainTextCallCount = 0
var performAfterMenuMissResult = true

override func performKeyEquivalentAfterMenuMiss(with event: NSEvent) -> Bool {
afterMenuMissCallCount += 1
return performAfterMenuMissResult
}

override func paste(_ sender: Any?) {
pasteCallCount += 1
}

override func pasteAsPlainText(_ sender: Any?) {
pasteAsPlainTextCallCount += 1
}
}
Comment on lines +14 to +32

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect GhosttyNSView initializers and any init-time side effects that could
# make direct instantiation from a test fixture unsafe.
ast-grep --pattern $'class GhosttyNSView: $_ {
  $$$
}'

# Find all initializers of GhosttyNSView to see what runs on init(frame:).
rg -nP --type=swift -C5 '\bclass\s+GhosttyNSView\b|override\s+init\s*\(frame\b|required\s+init\?\(coder\b'

# Check for any override of performKeyEquivalentAfterMenuMiss to confirm the
# method exists on GhosttyNSView (and see its default behavior).
rg -nP --type=swift -C3 '\bperformKeyEquivalentAfterMenuMiss\s*\('

Repository: manaflow-ai/cmux

Length of output: 33790


🏁 Script executed:

#!/bin/bash
# Read the full GhosttyNSView init implementations and setup method
sed -n '5390,5450p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 3160


🏁 Script executed:

#!/bin/bash
# Find installEventMonitor and updateTrackingAreas implementations
rg -n "private func installEventMonitor|func updateTrackingAreas" Sources/GhosttyTerminalView.swift -A 20

Repository: manaflow-ai/cmux

Length of output: 2270


🏁 Script executed:

#!/bin/bash
# Check if there's deinit cleanup for the event monitor
rg -n "deinit" Sources/GhosttyTerminalView.swift -A 15 | head -80

Repository: manaflow-ai/cmux

Length of output: 3699


🏁 Script executed:

#!/bin/bash
# Check GhosttyNSView deinit to see if event monitor is cleaned up
sed -n '5131,5180p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 1947


🏁 Script executed:

#!/bin/bash
# Search for event monitor cleanup
rg -n "eventMonitor" Sources/GhosttyTerminalView.swift | grep -E "(nil|removeMonitor|removeLocalMonitor)"

Repository: manaflow-ai/cmux

Length of output: 167


🏁 Script executed:

#!/bin/bash
# Check where eventMonitor is removed and the overall cleanup pattern
rg -n "eventMonitor\s*=" Sources/GhosttyTerminalView.swift -B2 -A2

Repository: manaflow-ai/cmux

Length of output: 355


🏁 Script executed:

#!/bin/bash
# Check what class/context the eventMonitor removal is in
sed -n '8350,8365p' Sources/GhosttyTerminalView.swift

Repository: manaflow-ai/cmux

Length of output: 702


Test only exercises the "Ghostty handled" branch.

The probe defaults performAfterMenuMissResult = true and is never toggled, so the window routing is only verified for the case where Ghostty's binding resolution claims the event. The complementary path — performKeyEquivalentAfterMenuMiss returning false, causing the window-level V fallback to invoke paste(_:) / pasteAsPlainText(_:) — is not asserted in this file. Per the PR summary, that positive path is covered elsewhere (paste-action swizzling, pasteboard snapshot/restore), so this test's narrow "don't preempt Ghostty" assertion is appropriate on its own.

GhosttyCommandEquivalentProbeView(frame:) directly instantiates GhosttyNSView rather than using the surfaceView(in:) lookup elsewhere in the file. This is safe: setup() registers a global scroll-wheel event monitor (cleaned up properly in deinit), sets up tracking areas, and registers drag types—none of which require a Ghostty surface to be present. The probe view has no surface assigned and doesn't access self.surface, so there's no risk of crashes or state leakage into subsequent tests.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@cmuxTests/AppDelegateShortcutRoutingTests.swift` around lines 14 - 32, Test
only exercises the "Ghostty handled" branch because
GhosttyCommandEquivalentProbeView defaults performAfterMenuMissResult = true and
is never toggled; add a complementary test that sets
probe.performAfterMenuMissResult = false, triggers the same key-equivalent flow,
and asserts that window-level handlers were invoked by checking pasteCallCount
and pasteAsPlainTextCallCount on GhosttyCommandEquivalentProbeView (the class
overrides performKeyEquivalentAfterMenuMiss(with:), paste(_:), and
pasteAsPlainText(_:), so locate and reuse those symbols to flip the boolean and
assert the fallback behavior).


@MainActor
final class AppDelegateShortcutRoutingTests: XCTestCase {
Expand Down Expand Up @@ -4254,6 +4273,55 @@ final class AppDelegateShortcutRoutingTests: XCTestCase {
#endif
}

func testWindowPerformKeyEquivalentDefersTerminalPasteMenuMissToGhosttyBindingResolution() {
let previousMainMenu = NSApp.mainMenu
let probeWindow = NSWindow(
contentRect: NSRect(x: 0, y: 0, width: 320, height: 240),
styleMask: [.titled, .closable],
backing: .buffered,
defer: false
)
let contentView = NSView(frame: probeWindow.contentRect(forFrameRect: probeWindow.frame))
let probeView = GhosttyCommandEquivalentProbeView(frame: NSRect(x: 0, y: 0, width: 200, height: 120))

defer {
NSApp.mainMenu = previousMainMenu
probeWindow.orderOut(nil)
}

let emptyMenu = NSMenu(title: "Test")
emptyMenu.addItem(withTitle: "Placeholder", action: nil, keyEquivalent: "")
NSApp.mainMenu = emptyMenu

probeWindow.contentView = contentView
contentView.addSubview(probeView)
probeWindow.makeKeyAndOrderFront(nil)
probeWindow.displayIfNeeded()
XCTAssertTrue(probeWindow.makeFirstResponder(probeView), "Expected probe Ghostty view to own first responder")

guard let event = makeKeyDownEvent(
key: "v",
modifiers: [.command],
keyCode: 9,
windowNumber: probeWindow.windowNumber
) else {
XCTFail("Failed to construct Cmd+V event")
return
}

XCTAssertTrue(
probeWindow.performKeyEquivalent(with: event),
"Cmd+V menu miss should still route through Ghostty binding resolution"
)
XCTAssertEqual(probeView.afterMenuMissCallCount, 1, "Ghostty binding resolution should run after the menu miss")
XCTAssertEqual(probeView.pasteCallCount, 0, "Window routing must not force paste before Ghostty inspects bindings")
XCTAssertEqual(
probeView.pasteAsPlainTextCallCount,
0,
"Window routing must not force plain-text paste before Ghostty inspects bindings"
)
}

func testWindowSendEventRepairsVisibleSameWindowResponderDriftForFocusedTerminalTyping() {
guard let appDelegate = AppDelegate.shared else {
XCTFail("Expected AppDelegate.shared")
Expand Down
Loading
Loading