diff --git a/src/config/Config.zig b/src/config/Config.zig index 1e1bca74af..0ed98bdea8 100644 --- a/src/config/Config.zig +++ b/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 diff --git a/src/input/Binding.zig b/src/input/Binding.zig index a1e759bf86..19c1031957 100644 --- a/src/input/Binding.zig +++ b/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;