From 201198c74a8f6facc844c1564d1ebc48fa779bd6 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 9 Jan 2026 08:27:27 -0800 Subject: [PATCH] input: do value comparison for Set hash maps We previously only compared the hashes for triggers and actions for hash map equality. I'm genuinely surprised this never bit us before because it can result in false positives when two different values have the same hash. Fix that up! --- src/input/Binding.zig | 64 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 52 insertions(+), 12 deletions(-) diff --git a/src/input/Binding.zig b/src/input/Binding.zig index 08475c7e1..14db736ae 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -8,6 +8,7 @@ const assert = @import("../quirks.zig").inlineAssert; const build_config = @import("../build_config.zig"); const uucode = @import("uucode"); const EntryFormatter = @import("../config/formatter.zig").EntryFormatter; +const deepEqual = @import("../datastruct/comparison.zig").deepEqual; const key = @import("key.zig"); const key_mods = @import("key_mods.zig"); const KeyEvent = key.KeyEvent; @@ -1587,6 +1588,24 @@ pub const Action = union(enum) { }, } } + + /// Compares two actions for equality. + pub fn equal(self: Action, other: Action) bool { + if (std.meta.activeTag(self) != std.meta.activeTag(other)) return false; + return switch (self) { + inline else => |field_self, tag| { + const field_other = @field(other, @tagName(tag)); + return deepEqual( + @TypeOf(field_self), + field_self, + field_other, + ); + }, + }; + } + + /// For the Set.Context + const bindingSetEqual = equal; }; /// Trigger is the associated key state that can trigger an action. @@ -1908,7 +1927,7 @@ pub const Trigger = struct { } /// Returns true if two triggers are equal. - pub fn eql(self: Trigger, other: Trigger) bool { + pub fn equal(self: Trigger, other: Trigger) bool { if (self.mods != other.mods) return false; const self_tag = std.meta.activeTag(self.key); const other_tag = std.meta.activeTag(other.key); @@ -1920,6 +1939,26 @@ pub const Trigger = struct { }; } + /// Returns true if two triggers are equal using folded codepoints. + pub fn foldedEqual(self: Trigger, other: Trigger) bool { + if (self.mods != other.mods) return false; + const self_tag = std.meta.activeTag(self.key); + const other_tag = std.meta.activeTag(other.key); + if (self_tag != other_tag) return false; + return switch (self.key) { + .physical => |v| v == other.key.physical, + .unicode => |v| deepEqual( + [3]u21, + foldedCodepoint(v), + foldedCodepoint(other.key.unicode), + ), + .catch_all => true, + }; + } + + /// For the Set.Context + const bindingSetEqual = foldedEqual; + /// Convert the trigger to a C API compatible trigger. pub fn cval(self: Trigger) C { return .{ @@ -2674,7 +2713,7 @@ pub const Set = struct { // If our value is not the same as the old trigger, we can // ignore it because our reverse mapping points somewhere else. - if (!entry.value_ptr.eql(old)) return; + if (!entry.value_ptr.equal(old)) return; // It is the same trigger, so let's now go through our bindings // and try to find another trigger that maps to the same action. @@ -2745,7 +2784,8 @@ pub const Set = struct { } pub fn eql(ctx: @This(), a: KeyType, b: KeyType) bool { - return ctx.hash(a) == ctx.hash(b); + _ = ctx; + return a.bindingSetEqual(b); } }; } @@ -3110,63 +3150,63 @@ test "parse: all triggers" { } } -test "Trigger: eql" { +test "Trigger: equal" { const testing = std.testing; // Equal physical keys { const t1: Trigger = .{ .key = .{ .physical = .arrow_up }, .mods = .{ .ctrl = true } }; const t2: Trigger = .{ .key = .{ .physical = .arrow_up }, .mods = .{ .ctrl = true } }; - try testing.expect(t1.eql(t2)); + try testing.expect(t1.equal(t2)); } // Different physical keys { const t1: Trigger = .{ .key = .{ .physical = .arrow_up }, .mods = .{ .ctrl = true } }; const t2: Trigger = .{ .key = .{ .physical = .arrow_down }, .mods = .{ .ctrl = true } }; - try testing.expect(!t1.eql(t2)); + try testing.expect(!t1.equal(t2)); } // Different mods { const t1: Trigger = .{ .key = .{ .physical = .arrow_up }, .mods = .{ .ctrl = true } }; const t2: Trigger = .{ .key = .{ .physical = .arrow_up }, .mods = .{ .shift = true } }; - try testing.expect(!t1.eql(t2)); + try testing.expect(!t1.equal(t2)); } // Equal unicode keys { const t1: Trigger = .{ .key = .{ .unicode = 'a' }, .mods = .{} }; const t2: Trigger = .{ .key = .{ .unicode = 'a' }, .mods = .{} }; - try testing.expect(t1.eql(t2)); + try testing.expect(t1.equal(t2)); } // Different unicode keys { const t1: Trigger = .{ .key = .{ .unicode = 'a' }, .mods = .{} }; const t2: Trigger = .{ .key = .{ .unicode = 'b' }, .mods = .{} }; - try testing.expect(!t1.eql(t2)); + try testing.expect(!t1.equal(t2)); } // Different key types { const t1: Trigger = .{ .key = .{ .unicode = 'a' }, .mods = .{} }; const t2: Trigger = .{ .key = .{ .physical = .key_a }, .mods = .{} }; - try testing.expect(!t1.eql(t2)); + try testing.expect(!t1.equal(t2)); } // catch_all { const t1: Trigger = .{ .key = .catch_all, .mods = .{} }; const t2: Trigger = .{ .key = .catch_all, .mods = .{} }; - try testing.expect(t1.eql(t2)); + try testing.expect(t1.equal(t2)); } // catch_all with different mods { const t1: Trigger = .{ .key = .catch_all, .mods = .{} }; const t2: Trigger = .{ .key = .catch_all, .mods = .{ .alt = true } }; - try testing.expect(!t1.eql(t2)); + try testing.expect(!t1.equal(t2)); } }