macos: remove special-case cmd+period handling

Fixes #5522

This commit re-dispatches command inputs that are unhandled by our macOS
app so they can be encoded to the pty and handled by the core libghostty
key callback system.

We've had a special case `cmd+period` handling in Ghostty for a very
long time (since well into the private beta). `cmd+period` by default
binds to "cancel" in macOS, so it doesn't encode to the pty. We don't
handle "cancel" in any meaningful way in Ghostty, so we special-cased it
to encode properly to the pty.

However, as shown in #5522, if the user rebinds `cmd+period` at the
system level to some other operation, then this is ignored and we encode
it still. This isn't desirable, we just want to work around not caring
about "cancel."

The callback path that AppKit takes for key events is a bit convoluted.
For command keys, it first calls `performKeyEquivalent`. If this returns
false (we want to continue standard processing), then it calls EITHER
`keyDown` or `doCommand(by:)`. It calls the latter if there is a
standard system command that matches the key event. For `cmd+period` by
default, this is "cancel." Unfortunately, from `doCommand` we can't say
"oops, we don't want to handle this, please continue processing." Its
too late.

So, this commit stores the last command key event from
`performKeyEquivalent` and if we reach `doCommand` for it without having
called `keyDown`, we re-dispatch the event and send it to keyDown.

I'm honestly pretty sus about this whole logic but it is scoped to only
command-keys and I couldn't trigger any adverse behavior in my testing.
It also definitely fixed #5522 as far as I could reproduce it before.
This commit is contained in:
Mitchell Hashimoto
2025-03-25 14:53:14 -07:00
parent 416b617de9
commit 67f47a6e22

View File

@ -922,6 +922,33 @@ extension Ghostty {
_ = keyAction(GHOSTTY_ACTION_RELEASE, event: event)
}
/// Records the timestamp of the last event to performKeyEquivalent that had a command key active.
///
/// For command+key inputs, the AppKit input stack calls performKeyEquivalent to give us a chance
/// to handle them first. If we return "false" then it goes through the standard AppKit responder chain.
/// For an NSTextInputClient, that may redirect some commands _before_ our keyDown gets called.
/// Concretely: Command+Period will do: performKeyEquivalent, doCommand ("cancel:"). In doCommand,
/// we need to know that we actually want to handle that in keyDown, so we send it back through the
/// event dispatch system and use this timestamp as an identity to know to actually send it to keyDown.
///
/// Why not send it to keyDown always? Because if the user rebinds a command to something we
/// actually handle then we do want the standard response chain to handle the key input. Unfortunately,
/// we can't know what a command is bound to at a system level until we let it flow through the system.
/// That's the crux of the problem.
///
/// So, we have to send it back through if we didn't handle it.
///
/// The next part of the problem is comparing NSEvent identity seems pretty nasty. I couldn't
/// find a good way to do it. I originally stored a weak ref and did identity comparison but that
/// doesn't work and for reasons I couldn't figure out the value gets mangled (fields don't match
/// before/after the assignment). I suspect it has something to do with the fact an NSEvent is wrapping
/// a lower level event pointer and its just not surviving the Swift runtime somehow. I don't know.
///
/// The best thing I could find was to store the event timestamp which has decent granularity
/// and compare that. To further complicate things, some events are synthetic and have a zero
/// timestamp so we have to protect against that. Fun!
var lastCommandEvent: TimeInterval?
/// Special case handling for some control keys
override func performKeyEquivalent(with event: NSEvent) -> Bool {
switch (event.type) {
@ -975,15 +1002,41 @@ extension Ghostty {
equivalent = "\r"
case ".":
if (!event.modifierFlags.contains(.command)) {
default:
// It looks like some part of AppKit sometimes generates synthetic NSEvents
// with a zero timestamp. We never process these at this point. Concretely,
// this happens for me when pressing Cmd+period with default bindings. This
// binds to "cancel" which goes through AppKit to produce a synthetic "escape".
//
// Question: should we be ignoring all synthetic events? Should we be finding
// synthetic escape and ignoring it? I feel like Cmd+period could map to a
// escape binding by accident, but it hasn't happened yet...
if event.timestamp == 0 {
return false
}
equivalent = "."
// All of this logic here re: lastCommandEvent is to workaround some
// nasty behavior. See the docs for lastCommandEvent for more info.
default:
// Ignore other events
// Ignore all other non-command events. This lets the event continue
// through the AppKit event systems.
if (!event.modifierFlags.contains(.command)) {
// Reset since we got a non-command event.
lastCommandEvent = nil
return false
}
// If we have a prior command binding and the timestamp matches exactly
// then we pass it through to keyDown for encoding.
if let lastCommandEvent {
self.lastCommandEvent = nil
if lastCommandEvent == event.timestamp {
equivalent = event.characters ?? ""
break
}
}
lastCommandEvent = event.timestamp
return false
}
@ -1480,9 +1533,19 @@ extension Ghostty.SurfaceView: NSTextInputClient {
}
}
/// This function needs to exist for two reasons:
/// 1. Prevents an audible NSBeep for unimplemented actions.
/// 2. Allows us to properly encode super+key input events that we don't handle
override func doCommand(by selector: Selector) {
// This currently just prevents NSBeep from interpretKeyEvents but in the future
// we may want to make some of this work.
// If we are being processed by performKeyEquivalent with a command binding,
// we send it back through the event system so it can be encoded.
if let lastCommandEvent,
let current = NSApp.currentEvent,
lastCommandEvent == current.timestamp
{
NSApp.sendEvent(current)
return
}
print("SEL: \(selector)")
}