Skip to content

Commit

Permalink
macos: don't remove ctrl modifier for text input
Browse files Browse the repository at this point in the history
Fixes #5448

We previously removed the ctrl modifier for text commit (IME-style)
to workaround a libghostty quirk (as noted in the comment in the diff).
But this broke other keyboard layouts.

This commit attempts to clean this up slightly -- but not completely --
by removing that hack, and only modifying the ctrl behavior for the
UCKeyTranslate call.

Long term, I plan to remove UCKeyTranslate completely, as noted in the
todo comment already written just below this diff.

This fixes the aforementioned issue and hopefully doesn't regress any
other behavior. I tested the following:

  1. Dvorak Ctrl characters
  2. Ergo-L Ctrl characters
  3. US standard Ctrl characters
  4. Japanese IME input Ctrl input to modify IME state
  • Loading branch information
mitchellh committed Feb 13, 2025
1 parent 15e4195 commit 3104b21
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 30 deletions.
24 changes: 2 additions & 22 deletions macos/Sources/Ghostty/SurfaceView_AppKit.swift
Original file line number Diff line number Diff line change
Expand Up @@ -849,28 +849,8 @@ extension Ghostty {
var handled: Bool = false
if let list = keyTextAccumulator, list.count > 0 {
handled = true

// This is a hack. libghostty on macOS treats ctrl input as not having
// text because some keyboard layouts generate bogus characters for
// ctrl+key. libghostty can't tell this is from an IM keyboard giving
// us direct values. So, we just remove control.
var modifierFlags = event.modifierFlags
modifierFlags.remove(.control)
if let keyTextEvent = NSEvent.keyEvent(
with: .keyDown,
location: event.locationInWindow,
modifierFlags: modifierFlags,
timestamp: event.timestamp,
windowNumber: event.windowNumber,
context: nil,
characters: event.characters ?? "",
charactersIgnoringModifiers: event.charactersIgnoringModifiers ?? "",
isARepeat: event.isARepeat,
keyCode: event.keyCode
) {
for text in list {
_ = keyAction(action, event: keyTextEvent, text: text)
}
for text in list {
_ = keyAction(action, event: event, text: text)
}
}

Expand Down
18 changes: 10 additions & 8 deletions src/apprt/embedded.zig
Original file line number Diff line number Diff line change
Expand Up @@ -182,14 +182,9 @@ pub const App = struct {
if (strip) translate_mods.alt = false;
}

// On macOS we strip ctrl because UCKeyTranslate
// converts to the masked values (i.e. ctrl+c becomes 3)
// and we don't want that behavior.
//
// We also strip super because its not used for translation
// on macos and it results in a bad translation.
// We strip super on macOS because its not used for translation
// it results in a bad translation.
if (comptime builtin.target.isDarwin()) {
translate_mods.ctrl = false;
translate_mods.super = false;
}

Expand Down Expand Up @@ -236,7 +231,14 @@ pub const App = struct {
.surface => |surface| &surface.keymap_state,
},
@intCast(keycode),
translate_mods,
if (comptime builtin.target.isDarwin()) mods: {
// On macOS we strip ctrl because UCKeyTranslate
// converts to the masked values (i.e. ctrl+c becomes 3)
// and we don't want that behavior.
var v = translate_mods;
v.ctrl = false;
break :mods v;
} else translate_mods,
);

// TODO(mitchellh): I think we can get rid of the above keymap
Expand Down

0 comments on commit 3104b21

Please sign in to comment.