From 359c390218025d57a262c7eefb4412c4c249f079 Mon Sep 17 00:00:00 2001
From: Mitchell Hashimoto <m@mitchellh.com>
Date: Mon, 6 Jan 2025 09:45:26 -0800
Subject: [PATCH] config: `unbind` keybind triggers unbinds both translated and
 physical

Fixes #4703

This changes `unbind` so it always removes all keybinds with the given
trigger pattern regardless of if it is translated or physical.

The previous behavior was technically correct, but this implements the pattern
of least surprise. I can't think of a scenario where you really want to
be exact about what key you're unbinding. And if that scenario does
exist, you can always fix it by rebinding after unbind.
---
 src/config/Config.zig |  4 +++-
 src/input/Binding.zig | 36 +++++++++++++++++++++++++++++++++++-
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/src/config/Config.zig b/src/config/Config.zig
index 6cd6ad75e..e8acb9fea 100644
--- a/src/config/Config.zig
+++ b/src/config/Config.zig
@@ -914,7 +914,9 @@ class: ?[:0]const u8 = null,
 ///
 ///   * `unbind` - Remove the binding. This makes it so the previous action
 ///     is removed, and the key will be sent through to the child command
-///     if it is printable.
+///     if it is printable. Unbind will remove any matching trigger,
+///     including `physical:`-prefixed triggers without specifying the
+///     prefix.
 ///
 ///   * `csi:text` - Send a CSI sequence. i.e. `csi:A` sends "cursor up".
 ///
diff --git a/src/input/Binding.zig b/src/input/Binding.zig
index e0ad6c571..64e07e85e 100644
--- a/src/input/Binding.zig
+++ b/src/input/Binding.zig
@@ -1528,6 +1528,22 @@ pub const Set = struct {
 
     /// Remove a binding for a given trigger.
     pub fn remove(self: *Set, alloc: Allocator, t: Trigger) void {
+        // Remove whatever this trigger is
+        self.removeExact(alloc, t);
+
+        // If we have a physical we remove translated and vice versa.
+        const alternate: Trigger.Key = switch (t.key) {
+            .unicode => return,
+            .translated => |k| .{ .physical = k },
+            .physical => |k| .{ .translated = k },
+        };
+
+        var alt_t: Trigger = t;
+        alt_t.key = alternate;
+        self.removeExact(alloc, alt_t);
+    }
+
+    fn removeExact(self: *Set, alloc: Allocator, t: Trigger) void {
         const entry = self.bindings.get(t) orelse return;
         _ = self.bindings.remove(t);
 
@@ -1559,7 +1575,7 @@ pub const Set = struct {
                         },
                     }
                 } else {
-                    // No over trigger points to this action so we remove
+                    // No other trigger points to this action so we remove
                     // the reverse mapping completely.
                     _ = self.reverse.remove(leaf.action);
                 }
@@ -2101,6 +2117,24 @@ test "set: parseAndPut removed binding" {
     try testing.expect(s.getTrigger(.{ .new_window = {} }) == null);
 }
 
+test "set: parseAndPut removed physical binding" {
+    const testing = std.testing;
+    const alloc = testing.allocator;
+
+    var s: Set = .{};
+    defer s.deinit(alloc);
+
+    try s.parseAndPut(alloc, "physical:a=new_window");
+    try s.parseAndPut(alloc, "a=unbind");
+
+    // Creates forward mapping
+    {
+        const trigger: Trigger = .{ .key = .{ .physical = .a } };
+        try testing.expect(s.get(trigger) == null);
+    }
+    try testing.expect(s.getTrigger(.{ .new_window = {} }) == null);
+}
+
 test "set: parseAndPut sequence" {
     const testing = std.testing;
     const alloc = testing.allocator;