input: performable bindings aren't part of the reverse mapping

src/config/Config.zig

@@ -1015,6 +1015,12 @@ class: ?[:0]const u8 = null,
 ///     performable (acting identically to not having a keybind set at
 ///     all).
 ///
+///     Performable keybinds will not appear as menu shortcuts in the
+///     application menu. This is because the menu shortcuts force the
+///     action to be performed regardless of the state of the terminal.
+///     Performable keybinds will still work, they just won't appear as
+///     a shortcut label in the menu.
+///
 /// Keybind triggers are not unique per prefix combination. For example,
 /// `ctrl+a` and `global:ctrl+a` are not two separate keybinds. The keybind
 /// set later will overwrite the keybind set earlier. In this case, the

src/input/Binding.zig

@@ -284,11 +284,11 @@ pub const Action = union(enum) {
     scroll_page_fractional: f32,
     scroll_page_lines: i16,
 
-    /// Adjust the current selection in a given direction. Does nothing if no 
+    /// Adjust the current selection in a given direction. Does nothing if no
     /// selection exists.
     ///
     /// Arguments:
-    ///   - left, right, up, down, page_up, page_down, home, end, 
+    ///   - left, right, up, down, page_up, page_down, home, end,
     ///     beginning_of_line, end_of_line
     ///
     /// Example: Extend selection to the right
@@ -1230,6 +1230,13 @@ pub const Set = struct {
     /// This is a conscious decision since the primary use case of the reverse
     /// map is to support GUI toolkit keyboard accelerators and no mainstream
     /// GUI toolkit supports sequences.
+    ///
+    /// Performable triggers are also not present in the reverse map. This
+    /// is so that GUI toolkits don't register performable triggers as
+    /// menu shortcuts (the primary use case of the reverse map). GUI toolkits
+    /// such as GTK handle menu shortcuts too early in the event lifecycle
+    /// for performable to work so this is a conscious decision to ease the
+    /// integration with GUI toolkits.
     reverse: ReverseMap = .{},
 
     /// The entry type for the forward mapping of trigger to action.
@@ -1494,6 +1501,11 @@ pub const Set = struct {
         // unbind should never go into the set, it should be handled prior
         assert(action != .unbind);
 
+        // This is true if we're going to track this entry as
+        // a reverse mapping. There are certain scenarios we don't.
+        // See the reverse map docs for more information.
+        const track_reverse: bool = !flags.performable;
+
         const gop = try self.bindings.getOrPut(alloc, t);
 
         if (gop.found_existing) switch (gop.value_ptr.*) {
@@ -1505,7 +1517,7 @@ pub const Set = struct {
 
             // If we have an existing binding for this trigger, we have to
             // update the reverse mapping to remove the old action.
-            .leaf => {
+            .leaf => if (track_reverse) {
                 const t_hash = t.hash();
                 var it = self.reverse.iterator();
                 while (it.next()) |reverse_entry| it: {
@@ -1522,8 +1534,9 @@ pub const Set = struct {
             .flags = flags,
         } };
         errdefer _ = self.bindings.remove(t);
-        try self.reverse.put(alloc, action, t);
-        errdefer _ = self.reverse.remove(action);
+
+        if (track_reverse) try self.reverse.put(alloc, action, t);
+        errdefer if (track_reverse) self.reverse.remove(action);
     }
 
     /// Get a binding for a given trigger.
@@ -2373,6 +2386,39 @@ test "set: maintains reverse mapping" {
     }
 }
 
+test "set: performable is not part of reverse mappings" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var s: Set = .{};
+    defer s.deinit(alloc);
+
+    try s.put(alloc, .{ .key = .{ .translated = .a } }, .{ .new_window = {} });
+    {
+        const trigger = s.getTrigger(.{ .new_window = {} }).?;
+        try testing.expect(trigger.key.translated == .a);
+    }
+
+    // trigger should be non-performable
+    try s.putFlags(
+        alloc,
+        .{ .key = .{ .translated = .b } },
+        .{ .new_window = {} },
+        .{ .performable = true },
+    );
+    {
+        const trigger = s.getTrigger(.{ .new_window = {} }).?;
+        try testing.expect(trigger.key.translated == .a);
+    }
+
+    // removal of performable should do nothing
+    s.remove(alloc, .{ .key = .{ .translated = .b } });
+    {
+        const trigger = s.getTrigger(.{ .new_window = {} }).?;
+        try testing.expect(trigger.key.translated == .a);
+    }
+}
+
 test "set: overriding a mapping updates reverse" {
     const testing = std.testing;
     const alloc = testing.allocator;