input: converting set entries to Entry from Action

src/cli/list_keybinds.zig

@@ -117,13 +117,17 @@ fn prettyPrint(alloc: Allocator, keybinds: Config.Keybinds) !u8 {
     var widest_key: usize = 0;
     var buf: [64]u8 = undefined;
     while (iter.next()) |bind| {
+        const action = switch (bind.value_ptr.*) {
+            .leader => continue, // TODO: support this
+            .action, .action_unconsumed => |action| action,
+        };
         const key = switch (bind.key_ptr.key) {
             .translated => |k| try std.fmt.bufPrint(&buf, "{s}", .{@tagName(k)}),
             .physical => |k| try std.fmt.bufPrint(&buf, "physical:{s}", .{@tagName(k)}),
             .unicode => |c| try std.fmt.bufPrint(&buf, "{u}", .{c}),
         };
         widest_key = @max(widest_key, win.gwidth(key));
-        try bindings.append(.{ .trigger = bind.key_ptr.*, .action = bind.value_ptr.* });
+        try bindings.append(.{ .trigger = bind.key_ptr.*, .action = action });
     }
     std.mem.sort(Binding, bindings.items, {}, Binding.lessThan);
 

src/config/Config.zig

@@ -3317,30 +3317,45 @@ pub const Keybinds = struct {
 
     /// Deep copy of the struct. Required by Config.
     pub fn clone(self: *const Keybinds, alloc: Allocator) !Keybinds {
+        // TODO: leaders
         return .{
             .set = .{
                 .bindings = try self.set.bindings.clone(alloc),
                 .reverse = try self.set.reverse.clone(alloc),
-                .unconsumed = try self.set.unconsumed.clone(alloc),
             },
         };
     }
 
     /// Compare if two of our value are requal. Required by Config.
     pub fn equal(self: Keybinds, other: Keybinds) bool {
+        // Two keybinds are considered equal if their primary bindings
+        // are the same. We don't compare reverse mappings and such.
         const self_map = self.set.bindings;
         const other_map = other.set.bindings;
+
+        // If the count of mappings isn't identical they can't be equal
         if (self_map.count() != other_map.count()) return false;
 
         var it = self_map.iterator();
         while (it.next()) |self_entry| {
+            // If the trigger isn't in the other map, they can't be equal
             const other_entry = other_map.getEntry(self_entry.key_ptr.*) orelse
                 return false;
-            if (!equalField(
+
-                inputpkg.Binding.Action,
+            // If the entry types are different, they can't be equal
-                self_entry.value_ptr.*,
+            if (std.meta.activeTag(self_entry.value_ptr.*) !=
-                other_entry.value_ptr.*,
+                std.meta.activeTag(other_entry.value_ptr.*)) return false;
-            )) return false;
+
+            switch (self_entry.value_ptr.*) {
+                .leader => @panic("TODO"),
+
+                // Actions are compared by field directly
+                inline .action, .action_unconsumed => |_, tag| if (!equalField(
+                    inputpkg.Binding.Action,
+                    @field(self_entry.value_ptr.*, @tagName(tag)),
+                    @field(other_entry.value_ptr.*, @tagName(tag)),
+                )) return false,
+            }
         }
 
         return true;

src/input/Binding.zig

@@ -772,7 +772,7 @@ pub const Trigger = struct {
 pub const Set = struct {
     const HashMap = std.HashMapUnmanaged(
         Trigger,
-        Action,
+        Entry,
         Context(Trigger),
         std.hash_map.default_max_load_percentage,
     );
@@ -797,23 +797,15 @@ pub const Set = struct {
     /// 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.
+    ///
+    /// Sequenced triggers are never present in the reverse map at this time.
+    /// 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.
     reverse: ReverseMap = .{},
 
-    /// The map of triggers that explicitly do not want to be consumed
-    /// when matched. A trigger is "consumed" when it is not further
-    /// processed and potentially sent to the terminal. An "unconsumed"
-    /// trigger will perform both its action and also continue normal
-    /// encoding processing (if any).
-    ///
-    /// This is stored as a separate map since unconsumed triggers are
-    /// rare and we don't want to bloat our map with a byte per entry
-    /// (for boolean state) when most entries will be consumed.
-    ///
-    /// Assert: trigger in this map is also in bindings.
-    unconsumed: UnconsumedMap = .{},
-
     /// The entry type for the forward mapping of trigger to action.
-    const Entry = union(enum) {
+    pub const Entry = union(enum) {
         /// This key is a leader key in a sequence. You must follow the given
         /// set to find the next key in the sequence.
         leader: *Set,
@@ -823,12 +815,32 @@ pub const Set = struct {
         /// should not consume the input.
         action: Action,
         action_unconsumed: Action,
+
+        /// Implements the formatter for the fmt package. This encodes the
+        /// action back into the format used by parse.
+        pub fn format(
+            self: Entry,
+            comptime layout: []const u8,
+            opts: std.fmt.FormatOptions,
+            writer: anytype,
+        ) !void {
+            _ = layout;
+            _ = opts;
+
+            switch (self) {
+                .leader => @panic("TODO"),
+
+                .action, .action_unconsumed => |action| {
+                    // action implements the format
+                    try writer.print("{s}", .{action});
+                },
+            }
+        }
     };
 
     pub fn deinit(self: *Set, alloc: Allocator) void {
         self.bindings.deinit(alloc);
         self.reverse.deinit(alloc);
-        self.unconsumed.deinit(alloc);
         self.* = undefined;
     }
 
@@ -906,7 +918,6 @@ pub const Set = struct {
         assert(action != .unbind);
 
         const gop = try self.bindings.getOrPut(alloc, t);
-        if (!consumed) try self.unconsumed.put(alloc, t, {});
 
         // If we have an existing binding for this trigger, we have to
         // update the reverse mapping to remove the old action.
@@ -919,19 +930,21 @@ pub const Set = struct {
                     break :it;
                 }
             }
-
-            // We also have to remove the unconsumed state if it exists.
-            if (consumed) _ = self.unconsumed.remove(t);
         }
 
-        gop.value_ptr.* = action;
+        // TODO: i hate this error handling
+        gop.value_ptr.* = if (consumed) .{
+            .action = action,
+        } else .{
+            .action_unconsumed = 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.
-    pub fn get(self: Set, t: Trigger) ?Action {
+    pub fn get(self: Set, t: Trigger) ?Entry {
         return self.bindings.get(t);
     }
 
@@ -941,34 +954,37 @@ pub const Set = struct {
         return self.reverse.get(a);
     }
 
-    /// Returns true if the given trigger should be consumed. Requires
-    /// that trigger is in the set to be valid so this should only follow
-    /// a non-null get.
-    pub fn getConsumed(self: Set, t: Trigger) bool {
-        return self.unconsumed.get(t) == null;
-    }
-
     /// Remove a binding for a given trigger.
     pub fn remove(self: *Set, t: Trigger) void {
-        const action = self.bindings.get(t) orelse return;
+        const entry = self.bindings.get(t) orelse return;
         _ = self.bindings.remove(t);
-        _ = self.unconsumed.remove(t);
 
-        // Look for a matching action in bindings and use that.
+        switch (entry) {
-        // Note: we'd LIKE to replace this with the most recent binding but
+            .leader => @panic("TODO"),
-        // our hash map obviously has no concept of ordering so we have to
+
-        // choose whatever. Maybe a switch to an array hash map here.
+            // For an action we need to fix up the reverse mapping.
-        const action_hash = action.hash();
+            // Note: we'd LIKE to replace this with the most recent binding but
-        var it = self.bindings.iterator();
+            // our hash map obviously has no concept of ordering so we have to
-        while (it.next()) |entry| {
+            // choose whatever. Maybe a switch to an array hash map here.
-            if (entry.value_ptr.hash() == action_hash) {
+            .action, .action_unconsumed => |action| {
-                self.reverse.putAssumeCapacity(action, entry.key_ptr.*);
+                const action_hash = action.hash();
-                break;
+                var it = self.bindings.iterator();
-            }
+                while (it.next()) |it_entry| {
-        } else {
+                    switch (it_entry.value_ptr.*) {
-            // No over trigger points to this action so we remove
+                        .leader => {},
-            // the reverse mapping completely.
+                        .action, .action_unconsumed => |action_search| {
-            _ = self.reverse.remove(action);
+                            if (action_search.hash() == action_hash) {
+                                self.reverse.putAssumeCapacity(action, it_entry.key_ptr.*);
+                                break;
+                            }
+                        },
+                    }
+                } else {
+                    // No over trigger points to this action so we remove
+                    // the reverse mapping completely.
+                    _ = self.reverse.remove(action);
+                }
+            },
         }
     }
 
@@ -1307,7 +1323,7 @@ test "set: parseAndPut typical binding" {
 
     // Creates forward mapping
     {
-        const action = s.get(.{ .key = .{ .translated = .a } }).?;
+        const action = s.get(.{ .key = .{ .translated = .a } }).?.action;
         try testing.expect(action == .new_window);
     }
 
@@ -1330,9 +1346,8 @@ test "set: parseAndPut unconsumed binding" {
     // Creates forward mapping
     {
         const trigger: Trigger = .{ .key = .{ .translated = .a } };
-        const action = s.get(trigger).?;
+        const action = s.get(trigger).?.action_unconsumed;
         try testing.expect(action == .new_window);
-        try testing.expect(!s.getConsumed(trigger));
     }
 
     // Creates reverse mapping
@@ -1417,11 +1432,11 @@ test "set: consumed state" {
     defer s.deinit(alloc);
 
     try s.put(alloc, .{ .key = .{ .translated = .a } }, .{ .new_window = {} });
-    try testing.expect(s.getConsumed(.{ .key = .{ .translated = .a } }));
+    try testing.expect(s.get(.{ .key = .{ .translated = .a } }).? == .action);
 
     try s.putUnconsumed(alloc, .{ .key = .{ .translated = .a } }, .{ .new_window = {} });
-    try testing.expect(!s.getConsumed(.{ .key = .{ .translated = .a } }));
+    try testing.expect(s.get(.{ .key = .{ .translated = .a } }).? == .action_unconsumed);
 
     try s.put(alloc, .{ .key = .{ .translated = .a } }, .{ .new_window = {} });
-    try testing.expect(s.getConsumed(.{ .key = .{ .translated = .a } }));
+    try testing.expect(s.get(.{ .key = .{ .translated = .a } }).? == .action);
 }