From 35e78939e5f9e32176a59e38990d098ace6de6a9 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <mitchell.hashimoto@gmail.com>
Date: Mon, 13 Nov 2023 19:08:20 -0800
Subject: [PATCH] macos: ignore alt key with other modifiers set

This enables shifted alt-prefixed keys, such as `shift+alt+.` on
US standard becoming `M->`. To do this, we needed to fix a few bugs:

  (1) translation mods should strip alt even if other mods are set
  (2) AppKit translation event needs to construct new characters with
      the translation mods.
  (3) Alt-prefix handling in KeyEncoder needs to allow ASCII utf8
      translations even for macOS.
---
 macos/Sources/Ghostty/SurfaceView.swift |  2 +-
 src/apprt/embedded.zig                  | 17 +++++++--------
 src/input/KeyEncoder.zig                | 28 +++++++++++++++++--------
 src/input/key.zig                       | 16 +++++++-------
 4 files changed, 35 insertions(+), 28 deletions(-)

diff --git a/macos/Sources/Ghostty/SurfaceView.swift b/macos/Sources/Ghostty/SurfaceView.swift
index d8c69d882..7702cffd7 100644
--- a/macos/Sources/Ghostty/SurfaceView.swift
+++ b/macos/Sources/Ghostty/SurfaceView.swift
@@ -758,7 +758,7 @@ extension Ghostty {
                 timestamp: event.timestamp,
                 windowNumber: event.windowNumber,
                 context: nil,
-                characters: event.characters ?? "",
+                characters: event.characters(byApplyingModifiers: translationMods) ?? "",
                 charactersIgnoringModifiers: event.charactersIgnoringModifiers ?? "",
                 isARepeat: event.isARepeat,
                 keyCode: event.keyCode
diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig
index f505d6b4c..9d6dc5d00 100644
--- a/src/apprt/embedded.zig
+++ b/src/apprt/embedded.zig
@@ -658,15 +658,14 @@ pub const Surface = struct {
         // then we strip the alt modifier from the mods for translation.
         const translate_mods = translate_mods: {
             var translate_mods = mods;
-            switch (self.app.config.@"macos-option-as-alt") {
-                .false => {},
-                .true => translate_mods.alt = false,
-                .left => if (mods.sides.alt == .left) {
-                    translate_mods.alt = false;
-                },
-                .right => if (mods.sides.alt == .right) {
-                    translate_mods.alt = false;
-                },
+            if (comptime builtin.target.isDarwin()) {
+                const strip = switch (self.app.config.@"macos-option-as-alt") {
+                    .false => false,
+                    .true => mods.alt,
+                    .left => mods.sides.alt == .left,
+                    .right => mods.sides.alt == .right,
+                };
+                if (strip) translate_mods.alt = false;
             }
 
             // On macOS we strip ctrl because UCKeyTranslate
diff --git a/src/input/KeyEncoder.zig b/src/input/KeyEncoder.zig
index 521a78b95..b8c91bd5d 100644
--- a/src/input/KeyEncoder.zig
+++ b/src/input/KeyEncoder.zig
@@ -325,15 +325,6 @@ fn legacyAltPrefix(
             .right => if (mods.sides.alt == .left) return null,
             .true => {},
         }
-
-        if (self.event.unshifted_codepoint > 0) {
-            if (std.math.cast(
-                u8,
-                self.event.unshifted_codepoint,
-            )) |byte| {
-                return byte;
-            }
-        }
     }
 
     // Otherwise, we require utf8 to already have the byte represented.
@@ -1241,6 +1232,25 @@ test "legacy: alt+x macos" {
     try testing.expectEqualStrings("\x1Bc", actual);
 }
 
+test "legacy: shift+alt+. macos" {
+    if (comptime !builtin.target.isDarwin()) return error.SkipZigTest;
+
+    var buf: [128]u8 = undefined;
+    var enc: KeyEncoder = .{
+        .event = .{
+            .key = .period,
+            .utf8 = ">",
+            .unshifted_codepoint = '.',
+            .mods = .{ .alt = true, .shift = true },
+        },
+        .alt_esc_prefix = true,
+        .macos_option_as_alt = .true,
+    };
+
+    const actual = try enc.legacy(&buf);
+    try testing.expectEqualStrings("\x1B>", actual);
+}
+
 test "legacy: alt+ф" {
     var buf: [128]u8 = undefined;
     var enc: KeyEncoder = .{
diff --git a/src/input/key.zig b/src/input/key.zig
index 2d9a16dcb..2a7795cbd 100644
--- a/src/input/key.zig
+++ b/src/input/key.zig
@@ -131,15 +131,6 @@ pub const Mods = packed struct(Mods.Backing) {
         // platforms don't need to do anything.
         if (comptime !builtin.target.isDarwin()) return self;
 
-        // We care if only alt is set.
-        const alt_only: bool = alt_only: {
-            const alt_mods: Mods = .{ .alt = true };
-            var compare = self;
-            compare.sides = .{};
-            break :alt_only alt_mods.equal(compare);
-        };
-        if (!alt_only) return self;
-
         // Alt has to be set only on the correct side
         switch (option_as_alt) {
             .false => return self,
@@ -201,6 +192,13 @@ pub const Mods = packed struct(Mods.Backing) {
             const result = mods.translation(.right);
             try testing.expectEqual(result, mods);
         }
+
+        // Set with other mods
+        {
+            const mods: Mods = .{ .alt = true, .shift = true };
+            const result = mods.translation(.true);
+            try testing.expectEqual(Mods{ .shift = true }, result);
+        }
     }
 };