Merge pull request #368 from mitchellh/macos-sync

macos: sync keybindings with Mac menu items

src/apprt/embedded.zig

@@ -882,22 +882,21 @@ pub const CAPI = struct {
     /// Invoke an action on the surface.
     export fn ghostty_surface_binding_action(
         ptr: *Surface,
-        key: input.Binding.Key,
-        unused: *anyopaque,
-    ) void {
-        // For future arguments
-        _ = unused;
-
-        const action: input.Binding.Action = switch (key) {
-            .copy_to_clipboard => .{ .copy_to_clipboard = {} },
-            .paste_from_clipboard => .{ .paste_from_clipboard = {} },
-            .new_tab => .{ .new_tab = {} },
-            .new_window => .{ .new_window = {} },
+        action_ptr: [*]const u8,
+        action_len: usize,
+    ) bool {
+        const action_str = action_ptr[0..action_len];
+        const action = input.Binding.Action.parse(action_str) catch |err| {
+            log.err("error parsing binding action action={s} err={}", .{ action_str, err });
+            return false;
         };
 
         ptr.core_surface.performBindingAction(action) catch |err| {
             log.err("error performing binding action action={} err={}", .{ action, err });
+            return false;
         };
+
+        return true;
     }
 
     /// Sets the window background blur on macOS to the desired value.

src/config.zig

@@ -1559,6 +1559,25 @@ pub const CAPI = struct {
             log.err("error finalizing config err={}", .{err});
         };
     }
+
+    export fn ghostty_config_trigger(
+        self: *Config,
+        str: [*]const u8,
+        len: usize,
+    ) inputpkg.Binding.Trigger {
+        return config_trigger_(self, str[0..len]) catch |err| err: {
+            log.err("error finding trigger err={}", .{err});
+            break :err .{};
+        };
+    }
+
+    fn config_trigger_(
+        self: *Config,
+        str: []const u8,
+    ) !inputpkg.Binding.Trigger {
+        const action = try inputpkg.Binding.Action.parse(str);
+        return self.keybind.set.getTrigger(action) orelse .{};
+    }
 };
 
 test {

src/input/Binding.zig

@@ -82,72 +82,7 @@ pub fn parse(input: []const u8) !Binding {
     };
 
     // Find a matching action
-    const action: Action = action: {
-        // Split our action by colon. A colon may not exist for some
-        // actions so it is optional. The part preceding the colon is the
-        // action name.
-        const actionRaw = input[eqlIdx + 1 ..];
-        const colonIdx = std.mem.indexOf(u8, actionRaw, ":");
-        const action = actionRaw[0..(colonIdx orelse actionRaw.len)];
-
-        // An action name is always required
-        if (action.len == 0) return Error.InvalidFormat;
-
-        const actionInfo = @typeInfo(Action).Union;
-        inline for (actionInfo.fields) |field| {
-            if (std.mem.eql(u8, action, field.name)) {
-                // If the field type is void we expect no value
-                switch (field.type) {
-                    void => {
-                        if (colonIdx != null) return Error.InvalidFormat;
-                        break :action @unionInit(Action, field.name, {});
-                    },
-
-                    []const u8 => {
-                        const idx = colonIdx orelse return Error.InvalidFormat;
-                        const param = actionRaw[idx + 1 ..];
-                        break :action @unionInit(Action, field.name, param);
-                    },
-
-                    // Cursor keys can't be set currently
-                    Action.CursorKey => return Error.InvalidAction,
-
-                    else => switch (@typeInfo(field.type)) {
-                        .Enum => {
-                            const idx = colonIdx orelse return Error.InvalidFormat;
-                            const param = actionRaw[idx + 1 ..];
-                            const value = std.meta.stringToEnum(
-                                field.type,
-                                param,
-                            ) orelse return Error.InvalidFormat;
-
-                            break :action @unionInit(Action, field.name, value);
-                        },
-
-                        .Int => {
-                            const idx = colonIdx orelse return Error.InvalidFormat;
-                            const param = actionRaw[idx + 1 ..];
-                            const value = std.fmt.parseInt(field.type, param, 10) catch
-                                return Error.InvalidFormat;
-                            break :action @unionInit(Action, field.name, value);
-                        },
-
-                        .Float => {
-                            const idx = colonIdx orelse return Error.InvalidFormat;
-                            const param = actionRaw[idx + 1 ..];
-                            const value = std.fmt.parseFloat(field.type, param) catch
-                                return Error.InvalidFormat;
-                            break :action @unionInit(Action, field.name, value);
-                        },
-
-                        else => unreachable,
-                    },
-                }
-            }
-        }
-
-        return Error.InvalidFormat;
-    };
+    const action = try Action.parse(input[eqlIdx + 1 ..]);
 
     return Binding{ .trigger = trigger, .action = action };
 }
@@ -266,6 +201,118 @@ pub const Action = union(enum) {
         bottom,
         right,
     };
+
+    /// Parse an action in the format of "key=value" where key is the
+    /// action name and value is the action parameter. The parameter
+    /// is optional depending on the action.
+    pub fn parse(input: []const u8) !Action {
+        // Split our action by colon. A colon may not exist for some
+        // actions so it is optional. The part preceding the colon is the
+        // action name.
+        const colonIdx = std.mem.indexOf(u8, input, ":");
+        const action = input[0..(colonIdx orelse input.len)];
+
+        // An action name is always required
+        if (action.len == 0) return Error.InvalidFormat;
+
+        const actionInfo = @typeInfo(Action).Union;
+        inline for (actionInfo.fields) |field| {
+            if (std.mem.eql(u8, action, field.name)) {
+                // If the field type is void we expect no value
+                switch (field.type) {
+                    void => {
+                        if (colonIdx != null) return Error.InvalidFormat;
+                        return @unionInit(Action, field.name, {});
+                    },
+
+                    []const u8 => {
+                        const idx = colonIdx orelse return Error.InvalidFormat;
+                        const param = input[idx + 1 ..];
+                        return @unionInit(Action, field.name, param);
+                    },
+
+                    // Cursor keys can't be set currently
+                    Action.CursorKey => return Error.InvalidAction,
+
+                    else => switch (@typeInfo(field.type)) {
+                        .Enum => {
+                            const idx = colonIdx orelse return Error.InvalidFormat;
+                            const param = input[idx + 1 ..];
+                            const value = std.meta.stringToEnum(
+                                field.type,
+                                param,
+                            ) orelse return Error.InvalidFormat;
+
+                            return @unionInit(Action, field.name, value);
+                        },
+
+                        .Int => {
+                            const idx = colonIdx orelse return Error.InvalidFormat;
+                            const param = input[idx + 1 ..];
+                            const value = std.fmt.parseInt(field.type, param, 10) catch
+                                return Error.InvalidFormat;
+                            return @unionInit(Action, field.name, value);
+                        },
+
+                        .Float => {
+                            const idx = colonIdx orelse return Error.InvalidFormat;
+                            const param = input[idx + 1 ..];
+                            const value = std.fmt.parseFloat(field.type, param) catch
+                                return Error.InvalidFormat;
+                            return @unionInit(Action, field.name, value);
+                        },
+
+                        else => unreachable,
+                    },
+                }
+            }
+        }
+
+        return Error.InvalidAction;
+    }
+
+    /// Returns a hash code that can be used to uniquely identify this
+    /// action.
+    pub fn hash(self: Action) u64 {
+        var hasher = std.hash.Wyhash.init(0);
+
+        // Always has the active tag.
+        const Tag = @typeInfo(Action).Union.tag_type.?;
+        std.hash.autoHash(&hasher, @as(Tag, self));
+
+        // Hash the value of the field.
+        switch (self) {
+            inline else => |field| {
+                const FieldType = @TypeOf(field);
+                switch (FieldType) {
+                    // Do nothing for void
+                    void => {},
+
+                    // Floats are hashed by their bits. This is totally not
+                    // portable and there are edge cases such as NaNs and
+                    // signed zeros but these are not cases we expect for
+                    // our bindings.
+                    f32 => std.hash.autoHash(
+                        &hasher,
+                        @as(u32, @bitCast(field)),
+                    ),
+                    f64 => std.hash.autoHash(
+                        &hasher,
+                        @as(u64, @bitCast(field)),
+                    ),
+
+                    // Everything else automatically handle.
+                    else => std.hash.autoHashStrat(
+                        &hasher,
+                        field,
+                        .DeepRecursive,
+                    ),
+                }
+            },
+        }
+
+        return hasher.final();
+    }
 };
 
 // A key for the C API to execute an action. This must be kept in sync
@@ -278,7 +325,10 @@ pub const Key = enum(c_int) {
 };
 
 /// Trigger is the associated key state that can trigger an action.
-pub const Trigger = struct {
+/// This is an extern struct because this is also used in the C API.
+///
+/// This must be kept in sync with include/ghostty.h ghostty_input_trigger_s
+pub const Trigger = extern struct {
     /// The key that has to be pressed for a binding to take action.
     key: key.Key = .invalid,
 
@@ -308,15 +358,28 @@ pub const Set = struct {
     const HashMap = std.HashMapUnmanaged(
         Trigger,
         Action,
-        Context,
+        Context(Trigger),
+        std.hash_map.default_max_load_percentage,
+    );
+
+    const ReverseMap = std.HashMapUnmanaged(
+        Action,
+        Trigger,
+        Context(Action),
         std.hash_map.default_max_load_percentage,
     );
 
     /// The set of bindings.
     bindings: HashMap = .{},
 
+    /// The reverse mapping of action to binding. Note that multiple
+    /// bindings can map to the same action and this map will only have
+    /// the most recently added binding for an action.
+    reverse: ReverseMap = .{},
+
     pub fn deinit(self: *Set, alloc: Allocator) void {
         self.bindings.deinit(alloc);
+        self.reverse.deinit(alloc);
         self.* = undefined;
     }
 
@@ -331,6 +394,9 @@ pub const Set = struct {
         // unbind should never go into the set, it should be handled prior
         assert(action != .unbind);
         try self.bindings.put(alloc, t, action);
+        errdefer _ = self.bindings.remove(t);
+        try self.reverse.put(alloc, action, t);
+        errdefer _ = self.reverse.remove(action);
     }
 
     /// Get a binding for a given trigger.
@@ -338,23 +404,45 @@ pub const Set = struct {
         return self.bindings.get(t);
     }
 
+    /// Get a trigger for the given action. An action can have multiple
+    /// triggers so this will return the first one found.
+    pub fn getTrigger(self: Set, a: Action) ?Trigger {
+        return self.reverse.get(a);
+    }
+
     /// Remove a binding for a given trigger.
     pub fn remove(self: *Set, t: Trigger) void {
+        const action = self.bindings.get(t) orelse return;
         _ = self.bindings.remove(t);
+
+        // Look for a matching action in bindings and use that.
+        // Note: we'd LIKE to replace this with the most recent binding but
+        // our hash map obviously has no concept of ordering so we have to
+        // choose whatever. Maybe a switch to an array hash map here.
+        const action_hash = action.hash();
+        var it = self.bindings.iterator();
+        while (it.next()) |entry| {
+            if (entry.value_ptr.hash() == action_hash) {
+                self.reverse.putAssumeCapacity(action, entry.key_ptr.*);
+                break;
+            }
+        }
     }
 
     /// The hash map context for the set. This defines how the hash map
     /// gets the hash key and checks for equality.
-    const Context = struct {
-        pub fn hash(ctx: Context, k: Trigger) u64 {
-            _ = ctx;
-            return k.hash();
-        }
+    fn Context(comptime KeyType: type) type {
+        return struct {
+            pub fn hash(ctx: @This(), k: KeyType) u64 {
+                _ = ctx;
+                return k.hash();
+            }
 
-        pub fn eql(ctx: Context, a: Trigger, b: Trigger) bool {
-            return ctx.hash(a) == ctx.hash(b);
-        }
-    };
+            pub fn eql(ctx: @This(), a: KeyType, b: KeyType) bool {
+                return ctx.hash(a) == ctx.hash(b);
+            }
+        };
+    }
 };
 
 test "parse: triggers" {
@@ -427,7 +515,7 @@ test "parse: action invalid" {
     const testing = std.testing;
 
     // invalid action
-    try testing.expectError(Error.InvalidFormat, parse("a=nopenopenope"));
+    try testing.expectError(Error.InvalidAction, parse("a=nopenopenope"));
 }
 
 test "parse: action no parameters" {
@@ -494,3 +582,31 @@ test "parse: action with float" {
         try testing.expectEqual(@as(f32, 0.5), binding.action.scroll_page_fractional);
     }
 }
+
+test "set: maintains reverse mapping" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var s: Set = .{};
+    defer s.deinit(alloc);
+
+    try s.put(alloc, .{ .key = .a }, .{ .new_window = {} });
+    {
+        const trigger = s.getTrigger(.{ .new_window = {} }).?;
+        try testing.expect(trigger.key == .a);
+    }
+
+    // should be most recent
+    try s.put(alloc, .{ .key = .b }, .{ .new_window = {} });
+    {
+        const trigger = s.getTrigger(.{ .new_window = {} }).?;
+        try testing.expect(trigger.key == .b);
+    }
+
+    // removal should replace
+    s.remove(.{ .key = .b });
+    {
+        const trigger = s.getTrigger(.{ .new_window = {} }).?;
+        try testing.expect(trigger.key == .a);
+    }
+}