Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

macOS: super+period doesn't reach Surface keyCallback #4540

Closed
mitchellh opened this issue Jan 3, 2025 · 1 comment · Fixed by #4591
Closed

macOS: super+period doesn't reach Surface keyCallback #4540

mitchellh opened this issue Jan 3, 2025 · 1 comment · Fixed by #4591
Labels
input Keyboard or mouse input os/macos
Milestone

Comments

@mitchellh
Copy link
Contributor

Discussion: #2881

If you put a log into Surface.keyCallback, super+period doesn't create a key event. This is causing some keybinds to not work.

@mitchellh mitchellh added input Keyboard or mouse input os/macos labels Jan 3, 2025
@mitchellh
Copy link
Contributor Author

Believe it or not this is somehow the same thing as #4522. I'm going to keep this issue open but as I'm fixing this I'm also fixing that one.

liby added a commit to liby/ghostty that referenced this issue Jan 4, 2025
Previously on macOS, `super+period` key combination was being blocked
in `performKeyEquivalent` due to a control modifier check. This change
removes that restriction to allow `super+period` and similar key
combinations to be properly handled.

Fixes ghostty-org#4540
mitchellh added a commit that referenced this issue Jan 4, 2025
This changes quit signaling from a boolean return from core app `tick()`
to an apprt action. This simplifies the API and conceptually makes more
sense to me now.

This wasn't done just for that; this change was also needed so that
macOS can quit cleanly while fixing #4540 since we may no longer trigger
menu items. I wanted to split this out into a separate commit/PR because
it adds complexity making the diff harder to read.
mitchellh added a commit that referenced this issue Jan 4, 2025
This changes quit signaling from a boolean return from core app `tick()`
to an apprt action. This simplifies the API and conceptually makes more
sense to me now.

This wasn't done just for that; this change was also needed so that
macOS can quit cleanly while fixing #4540 since we may no longer trigger
menu items. I wanted to split this out into a separate commit/PR because
it adds complexity making the diff harder to read.
mitchellh added a commit that referenced this issue Jan 4, 2025
This changes quit signaling from a boolean return from core app `tick()`
to an apprt action. This simplifies the API and conceptually makes more
sense to me now.

This wasn't done just for that; this change was also needed so that
macOS can quit cleanly while fixing #4540 since we may no longer trigger
menu items. I wanted to split this out into a separate commit/PR because
it adds complexity making the diff harder to read.
mitchellh added a commit that referenced this issue Jan 4, 2025
Sorry for the vague title. This PR addresses multiple issues:

1. Fixes #4540 
2. #4522 is fixed for macOS only
3. Fixes #4590 
4. Fixes an untracked issue where `command+key` events will not send
release events for Kitty keyboard protocol, something I only noticed
while working on this.

There are multiple components to this PR.

## Part 1: `App/Surface.keyEventIsBinding`

This new API (also available in libghostty as
`ghostty_surface_key_is_binding`) returns a boolean true if the given
key event would match a binding trigger if it was the next key event
sent. It does not process the binding now.

This can be used by event handlers that intercept key events to
determine if it should send the event to Ghostty. This helps resolve
#4590 for us but is also part of all resolved issues.

## Part 2: macOS `performKeyEquivalent` changes

macOS calls `performKeyEquivalent` for any key combination that may
trigger a key equivalent. if this returns `true` then it is handled and
macOS ceases processing the event.

We were already using this to intercept things like `Ctrl+/` which
triggers a context menu in macOS Sequoia. But we now expand this to
intercept all events to check for bindings. This lets us fix #4590.

Additionally, it's been changed to special case `cmd+period`. I'm sure
more need to be added.

## Part 3: NSEvent local listener for command keyUp events

macOS simply doesn't send `keyUp` events for key events with command
pressed. The only way to work around this is to register an `NSEvent`
local listener. We now do this. This fixes the untracked issue noted
above.
@github-actions github-actions bot added this to the 1.0.2 milestone Jan 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
input Keyboard or mouse input os/macos
Projects
None yet
2 participants
@mitchellh and others