From 3877ead07133070792b02b79a10aa9e51d560879 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 08:46:57 -0800 Subject: [PATCH 01/17] input: parse chains (don't do anything with them yet) --- src/input/Binding.zig | 59 ++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/src/input/Binding.zig b/src/input/Binding.zig index 22a5e8386..0bacea87d 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -52,6 +52,7 @@ pub const Parser = struct { trigger_it: SequenceIterator, action: Action, flags: Flags = .{}, + chain: bool, pub const Elem = union(enum) { /// A leader trigger in a sequence. @@ -59,6 +60,12 @@ pub const Parser = struct { /// The final trigger and action in a sequence. binding: Binding, + + /// A chained action `chain=` that should be appended + /// to the previous binding. Note that any action is parsed, including + /// invalid actions for chains such as `unbind`. We expect downstream + /// consumers to validate that the action is valid for chaining. + chain: Action, }; pub fn init(raw_input: []const u8) Error!Parser { @@ -95,12 +102,23 @@ pub const Parser = struct { return Error.InvalidFormat; }; + // Detect chains. Chains must not have flags. + const chain = std.mem.eql(u8, input[0..eql_idx], "chain"); + if (chain and start_idx > 0) return Error.InvalidFormat; + // Sequence iterator goes up to the equal, action is after. We can // parse the action now. return .{ - .trigger_it = .{ .input = input[0..eql_idx] }, + .trigger_it = .{ + // This is kind of hacky but we put a dummy trigger + // for chained inputs. The `next` will never yield this + // because we have chain set. When we find a nicer way to + // do this we can remove it, the e2e is tested. + .input = if (chain) "a" else input[0..eql_idx], + }, .action = try .parse(input[eql_idx + 1 ..]), .flags = flags, + .chain = chain, }; } @@ -156,6 +174,9 @@ pub const Parser = struct { return .{ .leader = trigger }; } + // If we're a chain then return it as-is. + if (self.chain) return .{ .chain = self.action }; + // Out of triggers, yield the final action. return .{ .binding = .{ .trigger = trigger, @@ -191,19 +212,26 @@ const SequenceIterator = struct { /// Returns true if there are no more triggers to parse. pub fn done(self: *const SequenceIterator) bool { - return self.i > self.input.len; + return self.i >= self.input.len; } }; /// Parse a single, non-sequenced binding. To support sequences you must /// use parse. This is a convenience function for single bindings aimed /// primarily at tests. -fn parseSingle(raw_input: []const u8) (Error || error{UnexpectedSequence})!Binding { +/// +/// This doesn't support `chain` either, since chaining requires some +/// stateful concept of a prior binding. +fn parseSingle(raw_input: []const u8) (Error || error{ + UnexpectedChain, + UnexpectedSequence, +})!Binding { var p = try Parser.init(raw_input); const elem = (try p.next()) orelse return Error.InvalidFormat; return switch (elem) { .leader => error.UnexpectedSequence, .binding => elem.binding, + .chain => error.UnexpectedChain, }; } @@ -2155,6 +2183,8 @@ pub const Set = struct { b.flags, ), }, + + .chain => @panic("TODO"), } } @@ -2887,6 +2917,29 @@ test "parse: action with a tuple" { try testing.expectError(Error.InvalidFormat, parseSingle("a=resize_split:up,four")); } +test "parse: chain" { + const testing = std.testing; + + // Valid + { + var p = try Parser.init("chain=new_tab"); + try testing.expectEqual(Parser.Elem{ + .chain = .new_tab, + }, try p.next()); + try testing.expect(try p.next() == null); + } + + // Chain can't have flags + try testing.expectError(error.InvalidFormat, Parser.init("global:chain=ignore")); + + // Chain can't be part of a sequence + { + var p = try Parser.init("a>chain=ignore"); + _ = try p.next(); + try testing.expectError(error.InvalidFormat, p.next()); + } +} + test "sequence iterator" { const testing = std.testing; From 42c21eb16b0afc6d44820968bf4d585bbf9ccb21 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 09:13:05 -0800 Subject: [PATCH 02/17] input: leaf_chained tagged union value --- src/App.zig | 53 +++++++++++++------- src/Surface.zig | 44 ++++++++++++----- src/apprt/embedded.zig | 2 +- src/cli/list_keybinds.zig | 4 +- src/config/Config.zig | 15 ++++++ src/input/Binding.zig | 101 ++++++++++++++++++++++++++++++++++++-- 6 files changed, 184 insertions(+), 35 deletions(-) diff --git a/src/App.zig b/src/App.zig index 99d03399c..00be56f49 100644 --- a/src/App.zig +++ b/src/App.zig @@ -357,15 +357,17 @@ pub fn keyEvent( // Get the keybind entry for this event. We don't support key sequences // so we can look directly in the top-level set. const entry = rt_app.config.keybind.set.getEvent(event) orelse return false; - const leaf: input.Binding.Set.Leaf = switch (entry.value_ptr.*) { + const leaf: input.Binding.Set.GenericLeaf = switch (entry.value_ptr.*) { // Sequences aren't supported. Our configuration parser verifies // this for global keybinds but we may still get an entry for // a non-global keybind. .leader => return false, // Leaf entries are good - .leaf => |leaf| leaf, + inline .leaf, .leaf_chained => |leaf| leaf.generic(), }; + const actions: []const input.Binding.Action = leaf.actionsSlice(); + assert(actions.len > 0); // If we aren't focused, then we only process global keybinds. if (!self.focused and !leaf.flags.global) return false; @@ -373,13 +375,7 @@ pub fn keyEvent( // Global keybinds are done using performAll so that they // can target all surfaces too. if (leaf.flags.global) { - self.performAllAction(rt_app, leaf.action) catch |err| { - log.warn("error performing global keybind action action={s} err={}", .{ - @tagName(leaf.action), - err, - }); - }; - + self.performAllChainedAction(rt_app, actions); return true; } @@ -389,14 +385,20 @@ pub fn keyEvent( // If we are focused, then we process keybinds only if they are // app-scoped. Otherwise, we do nothing. Surface-scoped should - // be processed by Surface.keyEvent. - const app_action = leaf.action.scoped(.app) orelse return false; - self.performAction(rt_app, app_action) catch |err| { - log.warn("error performing app keybind action action={s} err={}", .{ - @tagName(app_action), - err, - }); - }; + // be processed by Surface.keyEvent. For chained actions, all + // actions must be app-scoped. + for (actions) |action| if (action.scoped(.app) == null) return false; + for (actions) |action| { + self.performAction( + rt_app, + action.scoped(.app).?, + ) catch |err| { + log.warn("error performing app keybind action action={s} err={}", .{ + @tagName(action), + err, + }); + }; + } return true; } @@ -454,6 +456,23 @@ pub fn performAction( } } +/// Performs a chained action. We will continue executing each action +/// even if there is a failure in a prior action. +pub fn performAllChainedAction( + self: *App, + rt_app: *apprt.App, + actions: []const input.Binding.Action, +) void { + for (actions) |action| { + self.performAllAction(rt_app, action) catch |err| { + log.warn("error performing chained action action={s} err={}", .{ + @tagName(action), + err, + }); + }; + } +} + /// Perform an app-wide binding action. If the action is surface-specific /// then it will be performed on all surfaces. To perform only app-scoped /// actions, use performAction. diff --git a/src/Surface.zig b/src/Surface.zig index 2784f93db..0ce758636 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -2866,7 +2866,7 @@ fn maybeHandleBinding( }; // Determine if this entry has an action or if its a leader key. - const leaf: input.Binding.Set.Leaf = switch (entry.value_ptr.*) { + const leaf: input.Binding.Set.GenericLeaf = switch (entry.value_ptr.*) { .leader => |set| { // Setup the next set we'll look at. self.keyboard.sequence_set = set; @@ -2893,9 +2893,8 @@ fn maybeHandleBinding( return .consumed; }, - .leaf => |leaf| leaf, + inline .leaf, .leaf_chained => |leaf| leaf.generic(), }; - const action = leaf.action; // consumed determines if the input is consumed or if we continue // encoding the key (if we have a key to encode). @@ -2917,36 +2916,58 @@ fn maybeHandleBinding( // An action also always resets the sequence set. self.keyboard.sequence_set = null; + // Setup our actions + const actions = leaf.actionsSlice(); + // Attempt to perform the action - log.debug("key event binding flags={} action={f}", .{ + log.debug("key event binding flags={} action={any}", .{ leaf.flags, - action, + actions, }); const performed = performed: { // If this is a global or all action, then we perform it on // the app and it applies to every surface. if (leaf.flags.global or leaf.flags.all) { - try self.app.performAllAction(self.rt_app, action); + self.app.performAllChainedAction( + self.rt_app, + actions, + ); // "All" actions are always performed since they are global. break :performed true; } - break :performed try self.performBindingAction(action); + // Perform each action. We are performed if ANY of the chained + // actions perform. + var performed: bool = false; + for (actions) |action| { + if (self.performBindingAction(action)) |_| { + performed = true; + } else |err| { + log.info( + "key binding action failed action={t} err={}", + .{ action, err }, + ); + } + } + + break :performed performed; }; if (performed) { // If we performed an action and it was a closing action, // our "self" pointer is not safe to use anymore so we need to // just exit immediately. - if (closingAction(action)) { + for (actions) |action| if (closingAction(action)) { log.debug("key binding is a closing binding, halting key event processing", .{}); return .closed; - } + }; // If our action was "ignore" then we return the special input // effect of "ignored". - if (action == .ignore) return .ignored; + for (actions) |action| if (action == .ignore) { + return .ignored; + }; } // If we have the performable flag and the action was not performed, @@ -2970,7 +2991,8 @@ fn maybeHandleBinding( // Store our last trigger so we don't encode the release event self.keyboard.last_trigger = event.bindingHash(); - if (insp_ev) |ev| ev.binding = action; + // TODO: Inspector must support chained events + if (insp_ev) |ev| ev.binding = actions[0]; return .consumed; } diff --git a/src/apprt/embedded.zig b/src/apprt/embedded.zig index da7a585a5..1cb9231bc 100644 --- a/src/apprt/embedded.zig +++ b/src/apprt/embedded.zig @@ -155,7 +155,7 @@ pub const App = struct { while (it.next()) |entry| { switch (entry.value_ptr.*) { .leader => {}, - .leaf => |leaf| if (leaf.flags.global) return true, + inline .leaf, .leaf_chained => |leaf| if (leaf.flags.global) return true, } } diff --git a/src/cli/list_keybinds.zig b/src/cli/list_keybinds.zig index e463f55b9..fb7ad19ec 100644 --- a/src/cli/list_keybinds.zig +++ b/src/cli/list_keybinds.zig @@ -326,7 +326,6 @@ fn iterateBindings( switch (bind.value_ptr.*) { .leader => |leader| { - // Recursively iterate on the set of bindings for this leader key var n_iter = leader.bindings.iterator(); const sub_bindings, const max_width = try iterateBindings(alloc, &n_iter, win); @@ -353,6 +352,9 @@ fn iterateBindings( .action = leaf.action, }); }, + .leaf_chained => { + // TODO: Show these. + }, } } diff --git a/src/config/Config.zig b/src/config/Config.zig index 8f1cece45..f75944aeb 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -6749,6 +6749,21 @@ pub const Keybinds = struct { other_leaf, )) return false; }, + + .leaf_chained => { + const self_chain = self_entry.value_ptr.*.leaf_chained; + const other_chain = other_entry.value_ptr.*.leaf_chained; + + if (self_chain.flags != other_chain.flags) return false; + if (self_chain.actions.items.len != other_chain.actions.items.len) return false; + for (self_chain.actions.items, other_chain.actions.items) |a1, a2| { + if (!equalField( + inputpkg.Binding.Action, + a1, + a2, + )) return false; + } + }, } } diff --git a/src/input/Binding.zig b/src/input/Binding.zig index 0bacea87d..39b6ffcba 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -212,7 +212,7 @@ const SequenceIterator = struct { /// Returns true if there are no more triggers to parse. pub fn done(self: *const SequenceIterator) bool { - return self.i >= self.input.len; + return self.i > self.input.len; } }; @@ -1953,6 +1953,9 @@ pub const Set = struct { /// to take along with the flags that may define binding behavior. leaf: Leaf, + /// A set of actions to take in response to a trigger. + leaf_chained: LeafChained, + /// Implements the formatter for the fmt package. This encodes the /// action back into the format used by parse. pub fn format( @@ -2018,6 +2021,8 @@ pub const Set = struct { buffer.print("={f}", .{leaf.action}) catch return error.OutOfMemory; try formatter.formatEntry([]const u8, buffer.buffer[0..buffer.end]); }, + + .leaf_chained => @panic("TODO"), } } }; @@ -2044,6 +2049,47 @@ pub const Set = struct { std.hash.autoHash(&hasher, self.flags); return hasher.final(); } + + pub fn generic(self: *const Leaf) GenericLeaf { + return .{ + .flags = self.flags, + .actions = .{ .single = .{self.action} }, + }; + } + }; + + /// Leaf node of a set that triggers multiple actions in sequence. + pub const LeafChained = struct { + actions: std.ArrayList(Action), + flags: Flags, + + pub fn deinit(self: *LeafChained, alloc: Allocator) void { + self.actions.deinit(alloc); + } + + pub fn generic(self: *const LeafChained) GenericLeaf { + return .{ + .flags = self.flags, + .actions = .{ .many = self.actions.items }, + }; + } + }; + + /// A generic leaf node that can be used to unify the handling of + /// leaf and leaf_chained. + pub const GenericLeaf = struct { + flags: Flags, + actions: union(enum) { + single: [1]Action, + many: []const Action, + }, + + pub fn actionsSlice(self: *const GenericLeaf) []const Action { + return switch (self.actions) { + .single => |*arr| arr, + .many => |slice| slice, + }; + } }; /// A full key-value entry for the set. @@ -2057,6 +2103,9 @@ pub const Set = struct { s.deinit(alloc); alloc.destroy(s); }, + + .leaf_chained => |*l| l.deinit(alloc), + .leaf => {}, }; @@ -2133,7 +2182,7 @@ pub const Set = struct { error.OutOfMemory => return error.OutOfMemory, }, - .leaf => { + .leaf, .leaf_chained => { // Remove the existing action. Fallthrough as if // we don't have a leader. set.remove(alloc, t); @@ -2163,6 +2212,7 @@ pub const Set = struct { leaf.action, leaf.flags, ) catch {}, + .leaf_chained => @panic("TODO"), }; }, @@ -2184,7 +2234,9 @@ pub const Set = struct { ), }, - .chain => @panic("TODO"), + .chain => { + // TODO: Do this, ignore for now. + }, } } @@ -2236,6 +2288,12 @@ pub const Set = struct { } } }, + + // Chained leaves aren't in the reverse mapping so we just + // clear it out. + .leaf_chained => |*l| { + l.deinit(alloc); + }, }; gop.value_ptr.* = .{ .leaf = .{ @@ -2312,7 +2370,7 @@ pub const Set = struct { } fn removeExact(self: *Set, alloc: Allocator, t: Trigger) void { - const entry = self.bindings.get(t) orelse return; + var entry = self.bindings.get(t) orelse return; _ = self.bindings.remove(t); switch (entry) { @@ -2334,7 +2392,7 @@ pub const Set = struct { var it = self.bindings.iterator(); while (it.next()) |it_entry| { switch (it_entry.value_ptr.*) { - .leader => {}, + .leader, .leaf_chained => {}, .leaf => |leaf_search| { if (leaf_search.action.hash() == action_hash) { self.reverse.putAssumeCapacity(leaf.action, it_entry.key_ptr.*); @@ -2348,6 +2406,12 @@ pub const Set = struct { _ = self.reverse.remove(leaf.action); } }, + + // Chained leaves are never in our reverse mapping so no + // cleanup is required. + .leaf_chained => |*l| { + l.deinit(alloc); + }, } } @@ -2366,6 +2430,8 @@ pub const Set = struct { // contain allocated strings). .leaf => |*s| s.* = try s.clone(alloc), + .leaf_chained => @panic("TODO"), + // Must be deep cloned. .leader => |*s| { const ptr = try alloc.create(Set); @@ -3356,6 +3422,31 @@ test "set: consumed state" { try testing.expect(s.get(.{ .key = .{ .unicode = 'a' } }).?.value_ptr.*.leaf.flags.consumed); } +// test "set: parseAndPut chain" { +// const testing = std.testing; +// const alloc = testing.allocator; +// +// var s: Set = .{}; +// defer s.deinit(alloc); +// +// try s.parseAndPut(alloc, "a=new_window"); +// try s.parseAndPut(alloc, "chain=new_tab"); +// +// // Creates forward mapping +// { +// const action = s.get(.{ .key = .{ .unicode = 'a' } }).?.value_ptr.*.leaf; +// try testing.expect(action.action == .new_window); +// try testing.expectEqual(Flags{}, action.flags); +// } +// +// // Does not create reverse mapping, because reverse mappings are only for +// // non-chain actions. +// { +// const trigger = s.getTrigger(.new_window); +// try testing.expect(trigger == null); +// } +// } + test "set: getEvent physical" { const testing = std.testing; const alloc = testing.allocator; From 457fededeb6f30b272b81bfe4583987091a0a846 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 10:51:43 -0800 Subject: [PATCH 03/17] input: keep track of chain parent --- src/input/Binding.zig | 323 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 305 insertions(+), 18 deletions(-) diff --git a/src/input/Binding.zig b/src/input/Binding.zig index 39b6ffcba..6c91415e4 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -1943,6 +1943,10 @@ pub const Set = struct { /// integration with GUI toolkits. reverse: ReverseMap = .{}, + /// The chain parent is the information necessary to attach a chained + /// action to the proper location in our mapping. + chain_parent: ?HashMap.Entry = null, + /// The entry type for the forward mapping of trigger to action. pub const Value = union(enum) { /// This key is a leader key in a sequence. You must follow the given @@ -2135,26 +2139,55 @@ pub const Set = struct { // We use recursion so that we can utilize the stack as our state // for cleanup. - self.parseAndPutRecurse(alloc, &it) catch |err| switch (err) { - // If this gets sent up to the root then we've unbound - // all the way up and this put was a success. - error.SequenceUnbind => {}, + const updated_set_ = self.parseAndPutRecurse( + alloc, + &it, + ) catch |err| err: { + switch (err) { + // If this gets sent up to the root then we've unbound + // all the way up and this put was a success. + error.SequenceUnbind => break :err null, - // Unrecoverable - error.OutOfMemory => return error.OutOfMemory, + // If our parser input was too short then the format + // is invalid because we handle all valid cases. + error.UnexpectedEndOfInput => return error.InvalidFormat, + + // Unrecoverable + error.OutOfMemory => return error.OutOfMemory, + } + + // Errors must never fall through. + unreachable; }; + + // If we have an updated set (a binding was added) then we store + // it for our chain parent. If we didn't update a set then we clear + // our chain parent since chaining is no longer valid until a + // valid binding is saved. + if (updated_set_) |updated_set| { + // A successful addition must have recorded a chain parent. + assert(updated_set.chain_parent != null); + if (updated_set != self) self.chain_parent = updated_set.chain_parent; + assert(self.chain_parent != null); + } else { + self.chain_parent = null; + } } const ParseAndPutRecurseError = Allocator.Error || error{ SequenceUnbind, + UnexpectedEndOfInput, }; + /// Returns the set that was ultimately updated if a binding was + /// added. Unbind does not return a set since nothing was added. fn parseAndPutRecurse( set: *Set, alloc: Allocator, it: *Parser, - ) ParseAndPutRecurseError!void { - const elem = (it.next() catch unreachable) orelse return; + ) ParseAndPutRecurseError!?*Set { + const elem = (it.next() catch unreachable) orelse + return error.UnexpectedEndOfInput; switch (elem) { .leader => |t| { // If we have a leader, we need to upsert a set for it. @@ -2177,9 +2210,11 @@ pub const Set = struct { error.SequenceUnbind => if (s.bindings.count() == 0) { set.remove(alloc, t); return error.SequenceUnbind; - }, + } else null, - error.OutOfMemory => return error.OutOfMemory, + error.UnexpectedEndOfInput, + error.OutOfMemory, + => err, }, .leaf, .leaf_chained => { @@ -2199,7 +2234,7 @@ pub const Set = struct { try set.bindings.put(alloc, t, .{ .leader = next }); // Recurse - parseAndPutRecurse(next, alloc, it) catch |err| switch (err) { + return parseAndPutRecurse(next, alloc, it) catch |err| switch (err) { // If our action was to unbind, we restore the old // action if we have it. error.SequenceUnbind => { @@ -2214,9 +2249,13 @@ pub const Set = struct { ) catch {}, .leaf_chained => @panic("TODO"), }; + + return null; }, - error.OutOfMemory => return error.OutOfMemory, + error.UnexpectedEndOfInput, + error.OutOfMemory, + => return err, }; }, @@ -2226,16 +2265,20 @@ pub const Set = struct { return error.SequenceUnbind; }, - else => try set.putFlags( - alloc, - b.trigger, - b.action, - b.flags, - ), + else => { + try set.putFlags( + alloc, + b.trigger, + b.action, + b.flags, + ); + return set; + }, }, .chain => { // TODO: Do this, ignore for now. + return set; }, } } @@ -2267,7 +2310,21 @@ pub const Set = struct { // See the reverse map docs for more information. const track_reverse: bool = !flags.performable; + // No matter what our chained parent becomes invalid because + // getOrPut invalidates pointers. + self.chain_parent = null; + const gop = try self.bindings.getOrPut(alloc, t); + self.chain_parent = .{ + .key_ptr = gop.key_ptr, + .value_ptr = gop.value_ptr, + }; + errdefer { + // If we have any errors we can't trust our values here. And + // we can't restore the old values because they're also invalidated + // by getOrPut so we just disable chaining. + self.chain_parent = null; + } if (gop.found_existing) switch (gop.value_ptr.*) { // If we have a leader we need to clean up the memory @@ -2304,6 +2361,13 @@ pub const Set = struct { if (track_reverse) try self.reverse.put(alloc, action, t); errdefer if (track_reverse) self.reverse.remove(action); + + // Invariant: after successful put, chain_parent must be valid and point + // to the entry we just added/updated. + assert(self.chain_parent != null); + assert(self.chain_parent.?.key_ptr == gop.key_ptr); + assert(self.chain_parent.?.value_ptr == gop.value_ptr); + assert(self.chain_parent.?.value_ptr.* == .leaf); } /// Get a binding for a given trigger. @@ -2370,6 +2434,11 @@ pub const Set = struct { } fn removeExact(self: *Set, alloc: Allocator, t: Trigger) void { + // Removal always resets our chain parent. We could make this + // finer grained but the way it is documented is that chaining + // must happen directly after sets so this works. + self.chain_parent = null; + var entry = self.bindings.get(t) orelse return; _ = self.bindings.remove(t); @@ -3097,6 +3166,15 @@ test "set: parseAndPut typical binding" { const trigger = s.getTrigger(.{ .new_window = {} }).?; try testing.expect(trigger.key.unicode == 'a'); } + + // Sets up the chain parent properly + try testing.expect(s.chain_parent != null); + { + var buf: std.Io.Writer.Allocating = .init(alloc); + defer buf.deinit(); + try s.chain_parent.?.key_ptr.format(&buf.writer); + try testing.expectEqualStrings("a", buf.written()); + } } test "set: parseAndPut unconsumed binding" { @@ -3121,6 +3199,15 @@ test "set: parseAndPut unconsumed binding" { const trigger = s.getTrigger(.{ .new_window = {} }).?; try testing.expect(trigger.key.unicode == 'a'); } + + // Sets up the chain parent properly + try testing.expect(s.chain_parent != null); + { + var buf: std.Io.Writer.Allocating = .init(alloc); + defer buf.deinit(); + try s.chain_parent.?.key_ptr.format(&buf.writer); + try testing.expectEqualStrings("a", buf.written()); + } } test "set: parseAndPut removed binding" { @@ -3139,6 +3226,206 @@ test "set: parseAndPut removed binding" { try testing.expect(s.get(trigger) == null); } try testing.expect(s.getTrigger(.{ .new_window = {} }) == null); + + // Sets up the chain parent properly + try testing.expect(s.chain_parent == null); +} + +test "set: put sets chain_parent" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.put(alloc, .{ .key = .{ .unicode = 'a' } }, .{ .new_window = {} }); + + // chain_parent should be set + try testing.expect(s.chain_parent != null); + { + var buf: std.Io.Writer.Allocating = .init(alloc); + defer buf.deinit(); + try s.chain_parent.?.key_ptr.format(&buf.writer); + try testing.expectEqualStrings("a", buf.written()); + } + + // chain_parent value should be a leaf + try testing.expect(s.chain_parent.?.value_ptr.* == .leaf); +} + +test "set: putFlags sets chain_parent" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.putFlags( + alloc, + .{ .key = .{ .unicode = 'a' } }, + .{ .new_window = {} }, + .{ .consumed = false }, + ); + + // chain_parent should be set + try testing.expect(s.chain_parent != null); + { + var buf: std.Io.Writer.Allocating = .init(alloc); + defer buf.deinit(); + try s.chain_parent.?.key_ptr.format(&buf.writer); + try testing.expectEqualStrings("a", buf.written()); + } + + // chain_parent value should be a leaf with correct flags + try testing.expect(s.chain_parent.?.value_ptr.* == .leaf); + try testing.expect(!s.chain_parent.?.value_ptr.*.leaf.flags.consumed); +} + +test "set: sequence sets chain_parent to final leaf" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.parseAndPut(alloc, "a>b=new_window"); + + // chain_parent should be set and point to 'b' (the final leaf) + try testing.expect(s.chain_parent != null); + { + var buf: std.Io.Writer.Allocating = .init(alloc); + defer buf.deinit(); + try s.chain_parent.?.key_ptr.format(&buf.writer); + try testing.expectEqualStrings("b", buf.written()); + } + + // chain_parent value should be a leaf + try testing.expect(s.chain_parent.?.value_ptr.* == .leaf); + try testing.expect(s.chain_parent.?.value_ptr.*.leaf.action == .new_window); +} + +test "set: multiple leaves under leader updates chain_parent" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.parseAndPut(alloc, "a>b=new_window"); + + // After first binding, chain_parent should be 'b' + try testing.expect(s.chain_parent != null); + { + var buf: std.Io.Writer.Allocating = .init(alloc); + defer buf.deinit(); + try s.chain_parent.?.key_ptr.format(&buf.writer); + try testing.expectEqualStrings("b", buf.written()); + } + + try s.parseAndPut(alloc, "a>c=new_tab"); + + // After second binding, chain_parent should be updated to 'c' + try testing.expect(s.chain_parent != null); + { + var buf: std.Io.Writer.Allocating = .init(alloc); + defer buf.deinit(); + try s.chain_parent.?.key_ptr.format(&buf.writer); + try testing.expectEqualStrings("c", buf.written()); + } + try testing.expect(s.chain_parent.?.value_ptr.*.leaf.action == .new_tab); +} + +test "set: sequence unbind clears chain_parent" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.parseAndPut(alloc, "a>b=new_window"); + try testing.expect(s.chain_parent != null); + + try s.parseAndPut(alloc, "a>b=unbind"); + + // After unbind, chain_parent should be cleared + try testing.expect(s.chain_parent == null); +} + +test "set: sequence unbind with remaining leaves clears chain_parent" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.parseAndPut(alloc, "a>b=new_window"); + try s.parseAndPut(alloc, "a>c=new_tab"); + try s.parseAndPut(alloc, "a>b=unbind"); + + // After unbind, chain_parent should be cleared even though 'c' remains + try testing.expect(s.chain_parent == null); + + // But 'c' should still exist + const a_entry = s.get(.{ .key = .{ .unicode = 'a' } }).?; + try testing.expect(a_entry.value_ptr.* == .leader); + const inner_set = a_entry.value_ptr.*.leader; + try testing.expect(inner_set.get(.{ .key = .{ .unicode = 'c' } }) != null); +} + +test "set: direct remove clears chain_parent" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.put(alloc, .{ .key = .{ .unicode = 'a' } }, .{ .new_window = {} }); + try testing.expect(s.chain_parent != null); + + s.remove(alloc, .{ .key = .{ .unicode = 'a' } }); + + // After removal, chain_parent should be cleared + try testing.expect(s.chain_parent == null); +} + +test "set: invalid format preserves chain_parent" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.parseAndPut(alloc, "a=new_window"); + const before_key = s.chain_parent.?.key_ptr; + const before_value = s.chain_parent.?.value_ptr; + + // Try an invalid parse - should fail + try testing.expectError(error.InvalidAction, s.parseAndPut(alloc, "a=invalid_action_xyz")); + + // chain_parent should be unchanged + try testing.expect(s.chain_parent != null); + try testing.expect(s.chain_parent.?.key_ptr == before_key); + try testing.expect(s.chain_parent.?.value_ptr == before_value); +} + +test "set: clone produces null chain_parent" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.parseAndPut(alloc, "a=new_window"); + try testing.expect(s.chain_parent != null); + + var cloned = try s.clone(alloc); + defer cloned.deinit(alloc); + + // Clone should have null chain_parent + try testing.expect(cloned.chain_parent == null); + + // But should have the binding + try testing.expect(cloned.get(.{ .key = .{ .unicode = 'a' } }) != null); } test "set: parseAndPut sequence" { From 4fdc52b920c03035ad3ce9aef042b602d2bd0a0c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 12:32:52 -0800 Subject: [PATCH 04/17] input: appendChain --- src/input/Binding.zig | 123 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/src/input/Binding.zig b/src/input/Binding.zig index 6c91415e4..a5bb44b4d 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -2370,6 +2370,52 @@ pub const Set = struct { assert(self.chain_parent.?.value_ptr.* == .leaf); } + /// Append a chained action to the prior set action. + /// + /// It is an error if there is no valid prior chain parent. + pub fn appendChain( + self: *Set, + alloc: Allocator, + action: Action, + ) (Allocator.Error || error{NoChainParent})!void { + const parent = self.chain_parent orelse return error.NoChainParent; + switch (parent.value_ptr.*) { + // Leader can never be a chain parent. Verified through various + // assertions and unit tests. + .leader => unreachable, + + // If it is already a chained action, we just append the + // action. Easy! + .leaf_chained => |*leaf| try leaf.actions.append( + alloc, + action, + ), + + // If it is a leaf, we need to convert it to a leaf_chained. + // We also need to be careful to remove any prior reverse + // mappings for this action since chained actions are not + // part of the reverse mapping. + .leaf => |leaf| { + // Setup our failable actions list first. + var actions: std.ArrayList(Action) = .empty; + try actions.ensureTotalCapacity(alloc, 2); + errdefer actions.deinit(alloc); + actions.appendAssumeCapacity(leaf.action); + actions.appendAssumeCapacity(action); + + // Clean up our reverse mapping. We only do this if + // we're the chain parent because only the root set + // maintains reverse mappings. + // TODO + + parent.value_ptr.* = .{ .leaf_chained = .{ + .actions = actions, + .flags = leaf.flags, + } }; + }, + } + } + /// Get a binding for a given trigger. pub fn get(self: Set, t: Trigger) ?Entry { return self.bindings.getEntry(t); @@ -4069,3 +4115,80 @@ test "action: format" { try a.format(&buf.writer); try testing.expectEqualStrings("text:\\xf0\\x9f\\x91\\xbb", buf.written()); } + +test "set: appendChain with no parent returns error" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try testing.expectError(error.NoChainParent, s.appendChain(alloc, .{ .new_tab = {} })); +} + +test "set: appendChain after put converts to leaf_chained" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.put(alloc, .{ .key = .{ .unicode = 'a' } }, .{ .new_window = {} }); + + // First appendChain converts leaf to leaf_chained and appends the new action + try s.appendChain(alloc, .{ .new_tab = {} }); + + const entry = s.get(.{ .key = .{ .unicode = 'a' } }).?; + try testing.expect(entry.value_ptr.* == .leaf_chained); + + const chained = entry.value_ptr.*.leaf_chained; + try testing.expectEqual(@as(usize, 2), chained.actions.items.len); + try testing.expect(chained.actions.items[0] == .new_window); + try testing.expect(chained.actions.items[1] == .new_tab); +} + +test "set: appendChain after putFlags preserves flags" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.putFlags( + alloc, + .{ .key = .{ .unicode = 'a' } }, + .{ .new_window = {} }, + .{ .consumed = false }, + ); + try s.appendChain(alloc, .{ .new_tab = {} }); + + const entry = s.get(.{ .key = .{ .unicode = 'a' } }).?; + try testing.expect(entry.value_ptr.* == .leaf_chained); + + const chained = entry.value_ptr.*.leaf_chained; + try testing.expect(!chained.flags.consumed); + try testing.expectEqual(@as(usize, 2), chained.actions.items.len); + try testing.expect(chained.actions.items[0] == .new_window); + try testing.expect(chained.actions.items[1] == .new_tab); +} + +test "set: appendChain multiple times" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.put(alloc, .{ .key = .{ .unicode = 'a' } }, .{ .new_window = {} }); + try s.appendChain(alloc, .{ .new_tab = {} }); + try s.appendChain(alloc, .{ .close_surface = {} }); + + const entry = s.get(.{ .key = .{ .unicode = 'a' } }).?; + try testing.expect(entry.value_ptr.* == .leaf_chained); + + const chained = entry.value_ptr.*.leaf_chained; + try testing.expectEqual(@as(usize, 3), chained.actions.items.len); + try testing.expect(chained.actions.items[0] == .new_window); + try testing.expect(chained.actions.items[1] == .new_tab); + try testing.expect(chained.actions.items[2] == .close_surface); +} From a3373f3c6a3653df70c0bb447b14cf72a181e906 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 12:43:16 -0800 Subject: [PATCH 05/17] input: appendChain reverse mapping --- src/input/Binding.zig | 166 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 136 insertions(+), 30 deletions(-) diff --git a/src/input/Binding.zig b/src/input/Binding.zig index a5bb44b4d..b4f519379 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -1944,8 +1944,21 @@ pub const Set = struct { reverse: ReverseMap = .{}, /// The chain parent is the information necessary to attach a chained - /// action to the proper location in our mapping. - chain_parent: ?HashMap.Entry = null, + /// action to the proper location in our mapping. It tracks both the + /// entry in the hashmap and the set it belongs to, which is needed + /// to properly update reverse mappings when converting a leaf to + /// a chained action. + chain_parent: ?ChainParent = null, + + /// Information about a chain parent entry, including which set it + /// belongs to. This is needed because reverse mappings are only + /// maintained in the root set, but the chain parent entry may be + /// in a nested set (for leader key sequences). + const ChainParent = struct { + key_ptr: *Trigger, + value_ptr: *Value, + set: *Set, + }; /// The entry type for the forward mapping of trigger to action. pub const Value = union(enum) { @@ -2318,6 +2331,7 @@ pub const Set = struct { self.chain_parent = .{ .key_ptr = gop.key_ptr, .value_ptr = gop.value_ptr, + .set = self, }; errdefer { // If we have any errors we can't trust our values here. And @@ -2403,15 +2417,22 @@ pub const Set = struct { actions.appendAssumeCapacity(leaf.action); actions.appendAssumeCapacity(action); - // Clean up our reverse mapping. We only do this if - // we're the chain parent because only the root set - // maintains reverse mappings. - // TODO - + // Convert to leaf_chained first, before fixing up reverse + // mapping. This is important because fixupReverseForAction + // searches for other bindings with the same action, and we + // don't want to find this entry (which is now chained). parent.value_ptr.* = .{ .leaf_chained = .{ .actions = actions, .flags = leaf.flags, } }; + + // Clean up our reverse mapping. Chained actions are not + // part of the reverse mapping, so we need to fix up the + // reverse map (possibly restoring another trigger for the + // same action). + if (!leaf.flags.performable) { + parent.set.fixupReverseForAction(leaf.action); + } }, } } @@ -2498,29 +2519,7 @@ pub const Set = struct { }, // For an action we need to fix up the reverse mapping. - // Note: we'd LIKE to replace this with the most recent binding but - // our hash map obviously has no concept of ordering so we have to - // choose whatever. Maybe a switch to an array hash map here. - .leaf => |leaf| { - const action_hash = leaf.action.hash(); - - var it = self.bindings.iterator(); - while (it.next()) |it_entry| { - switch (it_entry.value_ptr.*) { - .leader, .leaf_chained => {}, - .leaf => |leaf_search| { - if (leaf_search.action.hash() == action_hash) { - self.reverse.putAssumeCapacity(leaf.action, it_entry.key_ptr.*); - break; - } - }, - } - } else { - // No other trigger points to this action so we remove - // the reverse mapping completely. - _ = self.reverse.remove(leaf.action); - } - }, + .leaf => |leaf| self.fixupReverseForAction(leaf.action), // Chained leaves are never in our reverse mapping so no // cleanup is required. @@ -2530,6 +2529,38 @@ pub const Set = struct { } } + /// Fix up the reverse mapping after removing an action. + /// + /// When an action is removed from a binding (either by removal or by + /// converting to a chained action), we need to update the reverse mapping. + /// If another binding has the same action, we update the reverse mapping + /// to point to that binding. Otherwise, we remove the action from the + /// reverse mapping entirely. + /// + /// Note: we'd LIKE to replace this with the most recent binding but + /// our hash map obviously has no concept of ordering so we have to + /// choose whatever. Maybe a switch to an array hash map here. + fn fixupReverseForAction(self: *Set, action: Action) void { + const action_hash = action.hash(); + + var it = self.bindings.iterator(); + while (it.next()) |it_entry| { + switch (it_entry.value_ptr.*) { + .leader, .leaf_chained => {}, + .leaf => |leaf_search| { + if (leaf_search.action.hash() == action_hash) { + self.reverse.putAssumeCapacity(action, it_entry.key_ptr.*); + return; + } + }, + } + } + + // No other trigger points to this action so we remove + // the reverse mapping completely. + _ = self.reverse.remove(action); + } + /// Deep clone the set. pub fn clone(self: *const Set, alloc: Allocator) !Set { var result: Set = .{ @@ -4192,3 +4223,78 @@ test "set: appendChain multiple times" { try testing.expect(chained.actions.items[1] == .new_tab); try testing.expect(chained.actions.items[2] == .close_surface); } + +test "set: appendChain removes reverse mapping" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.put(alloc, .{ .key = .{ .unicode = 'a' } }, .{ .new_window = {} }); + + // Verify reverse mapping exists before chaining + try testing.expect(s.getTrigger(.{ .new_window = {} }) != null); + + // Chaining should remove the reverse mapping + try s.appendChain(alloc, .{ .new_tab = {} }); + + // Reverse mapping should be gone since chained actions are not in reverse map + try testing.expect(s.getTrigger(.{ .new_window = {} }) == null); +} + +test "set: appendChain with performable does not affect reverse mapping" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + // Add a non-performable binding first + try s.put(alloc, .{ .key = .{ .unicode = 'b' } }, .{ .new_window = {} }); + try testing.expect(s.getTrigger(.{ .new_window = {} }) != null); + + // Add a performable binding (not in reverse map) and chain it + try s.putFlags( + alloc, + .{ .key = .{ .unicode = 'a' } }, + .{ .close_surface = {} }, + .{ .performable = true }, + ); + + // close_surface was performable, so not in reverse map + try testing.expect(s.getTrigger(.{ .close_surface = {} }) == null); + + // Chaining the performable binding should not crash or affect anything + try s.appendChain(alloc, .{ .new_tab = {} }); + + // The non-performable new_window binding should still be in reverse map + try testing.expect(s.getTrigger(.{ .new_window = {} }) != null); +} + +test "set: appendChain restores next valid reverse mapping" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + // Add two bindings for the same action + try s.put(alloc, .{ .key = .{ .unicode = 'a' } }, .{ .new_window = {} }); + try s.put(alloc, .{ .key = .{ .unicode = 'b' } }, .{ .new_window = {} }); + + // Reverse mapping should point to 'b' (most recent) + { + const trigger = s.getTrigger(.{ .new_window = {} }).?; + try testing.expect(trigger.key.unicode == 'b'); + } + + // Chain an action to 'b', which should restore 'a' in the reverse map + try s.appendChain(alloc, .{ .new_tab = {} }); + + // Now reverse mapping should point to 'a' + { + const trigger = s.getTrigger(.{ .new_window = {} }).?; + try testing.expect(trigger.key.unicode == 'a'); + } +} From 67be309e3f43ab91c759368d3d7d4fd396182b94 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 12:50:39 -0800 Subject: [PATCH 06/17] input: Trigger.eql --- src/input/Binding.zig | 73 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/src/input/Binding.zig b/src/input/Binding.zig index b4f519379..0d52f23cd 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -1870,6 +1870,19 @@ pub const Trigger = struct { return array; } + /// Returns true if two triggers are equal. + pub fn eql(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| v == other.key.unicode, + .catch_all => true, + }; + } + /// Convert the trigger to a C API compatible trigger. pub fn cval(self: Trigger) C { return .{ @@ -2974,6 +2987,66 @@ test "parse: all triggers" { } } +test "Trigger: eql" { + 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)); + } + + // 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)); + } + + // 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)); + } + + // Equal unicode keys + { + const t1: Trigger = .{ .key = .{ .unicode = 'a' }, .mods = .{} }; + const t2: Trigger = .{ .key = .{ .unicode = 'a' }, .mods = .{} }; + try testing.expect(t1.eql(t2)); + } + + // Different unicode keys + { + const t1: Trigger = .{ .key = .{ .unicode = 'a' }, .mods = .{} }; + const t2: Trigger = .{ .key = .{ .unicode = 'b' }, .mods = .{} }; + try testing.expect(!t1.eql(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)); + } + + // catch_all + { + const t1: Trigger = .{ .key = .catch_all, .mods = .{} }; + const t2: Trigger = .{ .key = .catch_all, .mods = .{} }; + try testing.expect(t1.eql(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)); + } +} + test "parse: modifier aliases" { const testing = std.testing; From 9bf1b9ac711252782aa73d166173570c8fedd44d Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 12:54:05 -0800 Subject: [PATCH 07/17] input: cleaner reverse mapping cleanup --- src/input/Binding.zig | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/src/input/Binding.zig b/src/input/Binding.zig index 0d52f23cd..2594cdbc5 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -2443,9 +2443,10 @@ pub const Set = struct { // part of the reverse mapping, so we need to fix up the // reverse map (possibly restoring another trigger for the // same action). - if (!leaf.flags.performable) { - parent.set.fixupReverseForAction(leaf.action); - } + parent.set.fixupReverseForAction( + leaf.action, + parent.key_ptr.*, + ); }, } } @@ -2532,7 +2533,10 @@ pub const Set = struct { }, // For an action we need to fix up the reverse mapping. - .leaf => |leaf| self.fixupReverseForAction(leaf.action), + .leaf => |leaf| self.fixupReverseForAction( + leaf.action, + t, + ), // Chained leaves are never in our reverse mapping so no // cleanup is required. @@ -2550,19 +2554,35 @@ pub const Set = struct { /// to point to that binding. Otherwise, we remove the action from the /// reverse mapping entirely. /// + /// The `old` parameter is the trigger that was previously bound to this + /// action. It is used to check if the reverse mapping still points to + /// this trigger; if not, no fixup is needed since the reverse map already + /// points to a different trigger for this action. + /// /// Note: we'd LIKE to replace this with the most recent binding but /// our hash map obviously has no concept of ordering so we have to /// choose whatever. Maybe a switch to an array hash map here. - fn fixupReverseForAction(self: *Set, action: Action) void { - const action_hash = action.hash(); + fn fixupReverseForAction( + self: *Set, + action: Action, + old: Trigger, + ) void { + const entry = self.reverse.getEntry(action) orelse return; + // 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; + + // 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. + const action_hash = action.hash(); var it = self.bindings.iterator(); while (it.next()) |it_entry| { switch (it_entry.value_ptr.*) { .leader, .leaf_chained => {}, .leaf => |leaf_search| { if (leaf_search.action.hash() == action_hash) { - self.reverse.putAssumeCapacity(action, it_entry.key_ptr.*); + entry.value_ptr.* = it_entry.key_ptr.*; return; } }, From b8fe66a70162712d845b7ff9bd5c989d629fde2c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 12:58:04 -0800 Subject: [PATCH 08/17] input: parseAndPut handles chains --- src/input/Binding.zig | 189 +++++++++++++++++++++++++++++++++++------- 1 file changed, 161 insertions(+), 28 deletions(-) diff --git a/src/input/Binding.zig b/src/input/Binding.zig index 2594cdbc5..66ebb49a2 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -2166,6 +2166,7 @@ pub const Set = struct { // We use recursion so that we can utilize the stack as our state // for cleanup. const updated_set_ = self.parseAndPutRecurse( + self, alloc, &it, ) catch |err| err: { @@ -2178,6 +2179,12 @@ pub const Set = struct { // is invalid because we handle all valid cases. error.UnexpectedEndOfInput => return error.InvalidFormat, + // If we had a chain without a parent then the format is wrong. + error.NoChainParent => return error.InvalidFormat, + + // If we had an invalid action for a chain (e.g. unbind). + error.InvalidChainAction => return error.InvalidFormat, + // Unrecoverable error.OutOfMemory => return error.OutOfMemory, } @@ -2202,12 +2209,15 @@ pub const Set = struct { const ParseAndPutRecurseError = Allocator.Error || error{ SequenceUnbind, + NoChainParent, UnexpectedEndOfInput, + InvalidChainAction, }; /// Returns the set that was ultimately updated if a binding was /// added. Unbind does not return a set since nothing was added. fn parseAndPutRecurse( + root: *Set, set: *Set, alloc: Allocator, it: *Parser, @@ -2225,7 +2235,7 @@ pub const Set = struct { if (old) |entry| switch (entry) { // We have an existing leader for this key already // so recurse into this set. - .leader => |s| return parseAndPutRecurse( + .leader => |s| return root.parseAndPutRecurse( s, alloc, it, @@ -2238,7 +2248,9 @@ pub const Set = struct { return error.SequenceUnbind; } else null, + error.NoChainParent, error.UnexpectedEndOfInput, + error.InvalidChainAction, error.OutOfMemory, => err, }, @@ -2260,7 +2272,7 @@ pub const Set = struct { try set.bindings.put(alloc, t, .{ .leader = next }); // Recurse - return parseAndPutRecurse(next, alloc, it) catch |err| switch (err) { + return root.parseAndPutRecurse(next, alloc, it) catch |err| switch (err) { // If our action was to unbind, we restore the old // action if we have it. error.SequenceUnbind => { @@ -2279,7 +2291,9 @@ pub const Set = struct { return null; }, + error.NoChainParent, error.UnexpectedEndOfInput, + error.InvalidChainAction, error.OutOfMemory, => return err, }; @@ -2302,8 +2316,12 @@ pub const Set = struct { }, }, - .chain => { - // TODO: Do this, ignore for now. + .chain => |action| { + // Chains can only happen on the root. + assert(set == root); + // Unbind is not valid for chains. + if (action == .unbind) return error.InvalidChainAction; + try set.appendChain(alloc, action); return set; }, } @@ -2405,6 +2423,9 @@ pub const Set = struct { alloc: Allocator, action: Action, ) (Allocator.Error || error{NoChainParent})!void { + // Unbind is not a valid chain action; callers must check this. + assert(action != .unbind); + const parent = self.chain_parent orelse return error.NoChainParent; switch (parent.value_ptr.*) { // Leader can never be a chain parent. Verified through various @@ -3879,30 +3900,142 @@ test "set: consumed state" { try testing.expect(s.get(.{ .key = .{ .unicode = 'a' } }).?.value_ptr.*.leaf.flags.consumed); } -// test "set: parseAndPut chain" { -// const testing = std.testing; -// const alloc = testing.allocator; -// -// var s: Set = .{}; -// defer s.deinit(alloc); -// -// try s.parseAndPut(alloc, "a=new_window"); -// try s.parseAndPut(alloc, "chain=new_tab"); -// -// // Creates forward mapping -// { -// const action = s.get(.{ .key = .{ .unicode = 'a' } }).?.value_ptr.*.leaf; -// try testing.expect(action.action == .new_window); -// try testing.expectEqual(Flags{}, action.flags); -// } -// -// // Does not create reverse mapping, because reverse mappings are only for -// // non-chain actions. -// { -// const trigger = s.getTrigger(.new_window); -// try testing.expect(trigger == null); -// } -// } +test "set: parseAndPut chain" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.parseAndPut(alloc, "a=new_window"); + try s.parseAndPut(alloc, "chain=new_tab"); + + // Creates forward mapping as leaf_chained + { + const entry = s.get(.{ .key = .{ .unicode = 'a' } }).?.value_ptr.*; + try testing.expect(entry == .leaf_chained); + const chained = entry.leaf_chained; + try testing.expectEqual(@as(usize, 2), chained.actions.items.len); + try testing.expect(chained.actions.items[0] == .new_window); + try testing.expect(chained.actions.items[1] == .new_tab); + } + + // Does not create reverse mapping, because reverse mappings are only for + // non-chained actions. + { + try testing.expect(s.getTrigger(.{ .new_window = {} }) == null); + } +} + +test "set: parseAndPut chain without parent is error" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + // Chain without a prior binding should fail + try testing.expectError(error.InvalidFormat, s.parseAndPut(alloc, "chain=new_tab")); +} + +test "set: parseAndPut chain multiple times" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.parseAndPut(alloc, "a=new_window"); + try s.parseAndPut(alloc, "chain=new_tab"); + try s.parseAndPut(alloc, "chain=close_surface"); + + // Should have 3 actions chained + { + const entry = s.get(.{ .key = .{ .unicode = 'a' } }).?.value_ptr.*; + try testing.expect(entry == .leaf_chained); + const chained = entry.leaf_chained; + try testing.expectEqual(@as(usize, 3), chained.actions.items.len); + try testing.expect(chained.actions.items[0] == .new_window); + try testing.expect(chained.actions.items[1] == .new_tab); + try testing.expect(chained.actions.items[2] == .close_surface); + } +} + +test "set: parseAndPut chain preserves flags" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.parseAndPut(alloc, "unconsumed:a=new_window"); + try s.parseAndPut(alloc, "chain=new_tab"); + + // Should preserve unconsumed flag + { + const entry = s.get(.{ .key = .{ .unicode = 'a' } }).?.value_ptr.*; + try testing.expect(entry == .leaf_chained); + const chained = entry.leaf_chained; + try testing.expect(!chained.flags.consumed); + try testing.expectEqual(@as(usize, 2), chained.actions.items.len); + } +} + +test "set: parseAndPut chain after unbind is error" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.parseAndPut(alloc, "a=new_window"); + try s.parseAndPut(alloc, "a=unbind"); + + // Chain after unbind should fail because chain_parent is cleared + try testing.expectError(error.InvalidFormat, s.parseAndPut(alloc, "chain=new_tab")); +} + +test "set: parseAndPut chain on sequence" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.parseAndPut(alloc, "a>b=new_window"); + try s.parseAndPut(alloc, "chain=new_tab"); + + // Navigate to the inner set + const a_entry = s.get(.{ .key = .{ .unicode = 'a' } }).?.value_ptr.*; + try testing.expect(a_entry == .leader); + const inner_set = a_entry.leader; + + // Check the chained binding + const b_entry = inner_set.get(.{ .key = .{ .unicode = 'b' } }).?.value_ptr.*; + try testing.expect(b_entry == .leaf_chained); + const chained = b_entry.leaf_chained; + try testing.expectEqual(@as(usize, 2), chained.actions.items.len); + try testing.expect(chained.actions.items[0] == .new_window); + try testing.expect(chained.actions.items[1] == .new_tab); +} + +test "set: parseAndPut chain with unbind is error" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + try s.parseAndPut(alloc, "a=new_window"); + + // chain=unbind is not valid + try testing.expectError(error.InvalidFormat, s.parseAndPut(alloc, "chain=unbind")); + + // Original binding should still exist + const entry = s.get(.{ .key = .{ .unicode = 'a' } }).?.value_ptr.*; + try testing.expect(entry == .leaf); + try testing.expect(entry.leaf.action == .new_window); +} test "set: getEvent physical" { const testing = std.testing; From 442146cf9f8a60f4b4d127199b162df662588971 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 13:13:34 -0800 Subject: [PATCH 09/17] input: implement leaf_chained clone --- src/input/Binding.zig | 75 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/src/input/Binding.zig b/src/input/Binding.zig index 66ebb49a2..85c3c2942 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -2093,6 +2093,21 @@ pub const Set = struct { actions: std.ArrayList(Action), flags: Flags, + pub fn clone( + self: LeafChained, + alloc: Allocator, + ) Allocator.Error!LeafChained { + var cloned_actions = try self.actions.clone(alloc); + errdefer cloned_actions.deinit(alloc); + for (cloned_actions.items) |*action| { + action.* = try action.clone(alloc); + } + return .{ + .actions = cloned_actions, + .flags = self.flags, + }; + } + pub fn deinit(self: *LeafChained, alloc: Allocator) void { self.actions.deinit(alloc); } @@ -2630,7 +2645,7 @@ pub const Set = struct { // contain allocated strings). .leaf => |*s| s.* = try s.clone(alloc), - .leaf_chained => @panic("TODO"), + .leaf_chained => |*s| s.* = try s.clone(alloc), // Must be deep cloned. .leader => |*s| { @@ -3619,6 +3634,64 @@ test "set: clone produces null chain_parent" { try testing.expect(cloned.get(.{ .key = .{ .unicode = 'a' } }) != null); } +test "set: clone with leaf_chained" { + const testing = std.testing; + const alloc = testing.allocator; + + var s: Set = .{}; + defer s.deinit(alloc); + + // Create a chained binding using parseAndPut with chain= + try s.parseAndPut(alloc, "a=new_window"); + try s.parseAndPut(alloc, "chain=new_tab"); + + // Verify we have a leaf_chained + const entry = s.get(.{ .key = .{ .unicode = 'a' } }).?; + try testing.expect(entry.value_ptr.* == .leaf_chained); + try testing.expectEqual(@as(usize, 2), entry.value_ptr.leaf_chained.actions.items.len); + + // Clone the set + var cloned = try s.clone(alloc); + defer cloned.deinit(alloc); + + // Verify the cloned set has the leaf_chained with same actions + const cloned_entry = cloned.get(.{ .key = .{ .unicode = 'a' } }).?; + try testing.expect(cloned_entry.value_ptr.* == .leaf_chained); + try testing.expectEqual(@as(usize, 2), cloned_entry.value_ptr.leaf_chained.actions.items.len); + try testing.expect(cloned_entry.value_ptr.leaf_chained.actions.items[0] == .new_window); + try testing.expect(cloned_entry.value_ptr.leaf_chained.actions.items[1] == .new_tab); +} + +test "set: clone with leaf_chained containing allocated data" { + const testing = std.testing; + var arena = std.heap.ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const alloc = arena.allocator(); + + var s: Set = .{}; + + // Create a chained binding with text actions (which have allocated strings) + try s.parseAndPut(alloc, "a=text:hello"); + try s.parseAndPut(alloc, "chain=text:world"); + + // Verify we have a leaf_chained + const entry = s.get(.{ .key = .{ .unicode = 'a' } }).?; + try testing.expect(entry.value_ptr.* == .leaf_chained); + + // Clone the set + const cloned = try s.clone(alloc); + + // Verify the cloned set has independent copies of the text + const cloned_entry = cloned.get(.{ .key = .{ .unicode = 'a' } }).?; + try testing.expect(cloned_entry.value_ptr.* == .leaf_chained); + try testing.expectEqualStrings("hello", cloned_entry.value_ptr.leaf_chained.actions.items[0].text); + try testing.expectEqualStrings("world", cloned_entry.value_ptr.leaf_chained.actions.items[1].text); + + // Verify the pointers are different (truly cloned, not shared) + try testing.expect(entry.value_ptr.leaf_chained.actions.items[0].text.ptr != + cloned_entry.value_ptr.leaf_chained.actions.items[0].text.ptr); +} + test "set: parseAndPut sequence" { const testing = std.testing; const alloc = testing.allocator; From 578b4c284b80fed9e07e121ada4e2515d17c8f58 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 13:19:09 -0800 Subject: [PATCH 10/17] apprt/gtk: handle global actions with chains --- src/apprt/gtk/class/global_shortcuts.zig | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/apprt/gtk/class/global_shortcuts.zig b/src/apprt/gtk/class/global_shortcuts.zig index 57652916a..718b371fd 100644 --- a/src/apprt/gtk/class/global_shortcuts.zig +++ b/src/apprt/gtk/class/global_shortcuts.zig @@ -169,13 +169,16 @@ pub const GlobalShortcuts = extern struct { var trigger_buf: [1024]u8 = undefined; var it = config.keybind.set.bindings.iterator(); while (it.next()) |entry| { - const leaf = switch (entry.value_ptr.*) { - // Global shortcuts can't have leaders + const leaf: Binding.Set.GenericLeaf = switch (entry.value_ptr.*) { .leader => continue, - .leaf => |leaf| leaf, + inline .leaf, .leaf_chained => |leaf| leaf.generic(), }; if (!leaf.flags.global) continue; + // We only allow global keybinds that map to exactly a single + // action for now. TODO: remove this restriction + if (leaf.actions.len != 1) continue; + const trigger = if (key.xdgShortcutFromTrigger( &trigger_buf, entry.key_ptr.*, @@ -197,7 +200,7 @@ pub const GlobalShortcuts = extern struct { try priv.map.put( alloc, try alloc.dupeZ(u8, trigger), - leaf.action, + leaf.actions[0], ); } From e4c7d4e059eb46fc98e63f2c7c9291e6354a6463 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 13:22:38 -0800 Subject: [PATCH 11/17] input: handle unbind cleanup for leaf chains --- src/input/Binding.zig | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/input/Binding.zig b/src/input/Binding.zig index 85c3c2942..b02d67019 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -2300,7 +2300,25 @@ pub const Set = struct { leaf.action, leaf.flags, ) catch {}, - .leaf_chained => @panic("TODO"), + + .leaf_chained => |leaf| chain: { + // Rebuild our chain + set.putFlags( + alloc, + t, + leaf.actions.items[0], + leaf.flags, + ) catch break :chain; + for (leaf.actions.items[1..]) |action| { + set.appendChain( + alloc, + action, + ) catch { + set.remove(alloc, t); + break :chain; + }; + } + }, }; return null; From 7dd903588b5ddcab6c67256b6590b869b71af9a8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 13:27:40 -0800 Subject: [PATCH 12/17] input: formatter for chained entries --- src/input/Binding.zig | 117 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 116 insertions(+), 1 deletion(-) diff --git a/src/input/Binding.zig b/src/input/Binding.zig index b02d67019..83c6ef38f 100644 --- a/src/input/Binding.zig +++ b/src/input/Binding.zig @@ -2052,7 +2052,19 @@ pub const Set = struct { try formatter.formatEntry([]const u8, buffer.buffer[0..buffer.end]); }, - .leaf_chained => @panic("TODO"), + .leaf_chained => |leaf| { + const pos = buffer.end; + for (leaf.actions.items, 0..) |action, i| { + if (i == 0) { + buffer.print("={f}", .{action}) catch return error.OutOfMemory; + } else { + buffer.end = 0; + buffer.print("chain={f}", .{action}) catch return error.OutOfMemory; + } + try formatter.formatEntry([]const u8, buffer.buffer[0..buffer.end]); + buffer.end = pos; + } + }, } } }; @@ -4615,3 +4627,106 @@ test "set: appendChain restores next valid reverse mapping" { try testing.expect(trigger.key.unicode == 'a'); } } + +test "set: formatEntries leaf_chained" { + const testing = std.testing; + const alloc = testing.allocator; + const formatterpkg = @import("../config/formatter.zig"); + + var s: Set = .{}; + defer s.deinit(alloc); + + // Create a chained binding + try s.parseAndPut(alloc, "a=new_window"); + try s.parseAndPut(alloc, "chain=new_tab"); + + // Verify it's a leaf_chained + const entry = s.get(.{ .key = .{ .unicode = 'a' } }).?; + try testing.expect(entry.value_ptr.* == .leaf_chained); + + // Format the entries + var output: std.Io.Writer.Allocating = .init(alloc); + defer output.deinit(); + + var buf: [1024]u8 = undefined; + var writer: std.Io.Writer = .fixed(&buf); + + // Write the trigger first (as formatEntry in Config.zig does) + try entry.key_ptr.format(&writer); + try entry.value_ptr.formatEntries(&writer, formatterpkg.entryFormatter("keybind", &output.writer)); + + const expected = + \\keybind = a=new_window + \\keybind = chain=new_tab + \\ + ; + try testing.expectEqualStrings(expected, output.written()); +} + +test "set: formatEntries leaf_chained multiple chains" { + const testing = std.testing; + const alloc = testing.allocator; + const formatterpkg = @import("../config/formatter.zig"); + + var s: Set = .{}; + defer s.deinit(alloc); + + // Create a chained binding with 3 actions + try s.parseAndPut(alloc, "ctrl+a=new_window"); + try s.parseAndPut(alloc, "chain=new_tab"); + try s.parseAndPut(alloc, "chain=close_surface"); + + // Verify it's a leaf_chained with 3 actions + const entry = s.get(.{ .key = .{ .unicode = 'a' }, .mods = .{ .ctrl = true } }).?; + try testing.expect(entry.value_ptr.* == .leaf_chained); + try testing.expectEqual(@as(usize, 3), entry.value_ptr.leaf_chained.actions.items.len); + + // Format the entries + var output: std.Io.Writer.Allocating = .init(alloc); + defer output.deinit(); + + var buf: [1024]u8 = undefined; + var writer: std.Io.Writer = .fixed(&buf); + + try entry.key_ptr.format(&writer); + try entry.value_ptr.formatEntries(&writer, formatterpkg.entryFormatter("keybind", &output.writer)); + + const expected = + \\keybind = ctrl+a=new_window + \\keybind = chain=new_tab + \\keybind = chain=close_surface + \\ + ; + try testing.expectEqualStrings(expected, output.written()); +} + +test "set: formatEntries leaf_chained with text action" { + const testing = std.testing; + const alloc = testing.allocator; + const formatterpkg = @import("../config/formatter.zig"); + + var s: Set = .{}; + defer s.deinit(alloc); + + // Create a chained binding with text actions + try s.parseAndPut(alloc, "a=text:hello"); + try s.parseAndPut(alloc, "chain=text:world"); + + // Format the entries + var output: std.Io.Writer.Allocating = .init(alloc); + defer output.deinit(); + + var buf: [1024]u8 = undefined; + var writer: std.Io.Writer = .fixed(&buf); + + const entry = s.get(.{ .key = .{ .unicode = 'a' } }).?; + try entry.key_ptr.format(&writer); + try entry.value_ptr.formatEntries(&writer, formatterpkg.entryFormatter("keybind", &output.writer)); + + const expected = + \\keybind = a=text:hello + \\keybind = chain=text:world + \\ + ; + try testing.expectEqualStrings(expected, output.written()); +} From 99325a3d451bdf5b3bb64b2569633f6f16f2e3c8 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 13:32:12 -0800 Subject: [PATCH 13/17] config: docs for chains --- src/config/Config.zig | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/config/Config.zig b/src/config/Config.zig index f75944aeb..a2ce88320 100644 --- a/src/config/Config.zig +++ b/src/config/Config.zig @@ -1667,6 +1667,44 @@ class: ?[:0]const u8 = null, /// - Notably, global shortcuts have not been implemented on wlroots-based /// compositors like Sway (see [upstream issue](https://github.com/emersion/xdg-desktop-portal-wlr/issues/240)). /// +/// ## Chained Actions +/// +/// A keybind can have multiple actions by using the `chain` keyword for +/// subsequent actions. When a keybind is activated, all chained actions are +/// executed in order. The syntax is: +/// +/// ```ini +/// keybind = ctrl+a=new_window +/// keybind = chain=goto_split:left +/// ``` +/// +/// This binds `ctrl+a` to first open a new window, then move focus to the +/// left split. Each `chain` entry appends an action to the most recently +/// defined keybind. You can chain as many actions as you want: +/// +/// ```ini +/// keybind = ctrl+a=new_window +/// keybind = chain=goto_split:left +/// keybind = chain=toggle_fullscreen +/// ``` +/// +/// Chained actions cannot have prefixes like `global:` or `unconsumed:`. +/// The flags from the original keybind apply to the entire chain. +/// +/// Chained actions work with key sequences as well. For example: +/// +/// ```ini +/// keybind = ctrl+a>n=new_window +/// keybind = chain=goto_split:left +/// ```` +/// +/// Chains with key sequences apply to the most recent binding in the +/// sequence. +/// +/// Chained keybinds are available since Ghostty 1.3.0. +/// +/// ## Key Tables +/// /// You may also create a named set of keybindings known as a "key table." /// A key table must be explicitly activated for the bindings to become /// available. This can be used to implement features such as a From 931c6c71f2f522f1ac19563035879a75cdbc12cb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 13:38:46 -0800 Subject: [PATCH 14/17] fix up gtk --- src/apprt/gtk/class/global_shortcuts.zig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/apprt/gtk/class/global_shortcuts.zig b/src/apprt/gtk/class/global_shortcuts.zig index 718b371fd..cf0f31a6e 100644 --- a/src/apprt/gtk/class/global_shortcuts.zig +++ b/src/apprt/gtk/class/global_shortcuts.zig @@ -177,7 +177,8 @@ pub const GlobalShortcuts = extern struct { // We only allow global keybinds that map to exactly a single // action for now. TODO: remove this restriction - if (leaf.actions.len != 1) continue; + const actions = leaf.actionsSlice(); + if (actions.len != 1) continue; const trigger = if (key.xdgShortcutFromTrigger( &trigger_buf, @@ -200,7 +201,7 @@ pub const GlobalShortcuts = extern struct { try priv.map.put( alloc, try alloc.dupeZ(u8, trigger), - leaf.actions[0], + actions[0], ); } From 76c0bdf559f466c56803e9b2f72a0790cbad07ad Mon Sep 17 00:00:00 2001 From: "Jeffrey C. Ollie" Date: Mon, 22 Dec 2025 17:48:04 -0600 Subject: [PATCH 15/17] input: fix performable bindings --- src/Surface.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index 0ce758636..0fb0c034b 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -2941,8 +2941,8 @@ fn maybeHandleBinding( // actions perform. var performed: bool = false; for (actions) |action| { - if (self.performBindingAction(action)) |_| { - performed = true; + if (self.performBindingAction(action)) |performed_| { + performed = performed or performed_; } else |err| { log.info( "key binding action failed action={t} err={}", From dcbb3fe56fbc67ed5257fd4233bedfd69e5e1a3c Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 20:34:23 -0800 Subject: [PATCH 16/17] inspector: show chained bindings --- src/Surface.zig | 18 ++++++++++++++---- src/inspector/key.zig | 24 +++++++++++++++++++++--- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/src/Surface.zig b/src/Surface.zig index 0fb0c034b..ea17c6104 100644 --- a/src/Surface.zig +++ b/src/Surface.zig @@ -2941,8 +2941,8 @@ fn maybeHandleBinding( // actions perform. var performed: bool = false; for (actions) |action| { - if (self.performBindingAction(action)) |performed_| { - performed = performed or performed_; + if (self.performBindingAction(action)) |v| { + performed = performed or v; } else |err| { log.info( "key binding action failed action={t} err={}", @@ -2991,8 +2991,18 @@ fn maybeHandleBinding( // Store our last trigger so we don't encode the release event self.keyboard.last_trigger = event.bindingHash(); - // TODO: Inspector must support chained events - if (insp_ev) |ev| ev.binding = actions[0]; + if (insp_ev) |ev| { + ev.binding = self.alloc.dupe( + input.Binding.Action, + actions, + ) catch |err| binding: { + log.warn( + "error allocating binding action for inspector err={}", + .{err}, + ); + break :binding &.{}; + }; + } return .consumed; } diff --git a/src/inspector/key.zig b/src/inspector/key.zig index dbccb47a8..e42e4f23c 100644 --- a/src/inspector/key.zig +++ b/src/inspector/key.zig @@ -13,7 +13,8 @@ pub const Event = struct { event: input.KeyEvent, /// The binding that was triggered as a result of this event. - binding: ?input.Binding.Action = null, + /// Multiple bindings are possible if they are chained. + binding: []const input.Binding.Action = &.{}, /// The data sent to the pty as a result of this keyboard event. /// This is allocated using the inspector allocator. @@ -32,6 +33,7 @@ pub const Event = struct { } pub fn deinit(self: *const Event, alloc: Allocator) void { + alloc.free(self.binding); if (self.event.utf8.len > 0) alloc.free(self.event.utf8); if (self.pty.len > 0) alloc.free(self.pty); } @@ -79,12 +81,28 @@ pub const Event = struct { ); defer cimgui.c.igEndTable(); - if (self.binding) |binding| { + if (self.binding.len > 0) { cimgui.c.igTableNextRow(cimgui.c.ImGuiTableRowFlags_None, 0); _ = cimgui.c.igTableSetColumnIndex(0); cimgui.c.igText("Triggered Binding"); _ = cimgui.c.igTableSetColumnIndex(1); - cimgui.c.igText("%s", @tagName(binding).ptr); + + const height: f32 = height: { + const item_count: f32 = @floatFromInt(@min(self.binding.len, 5)); + const padding = cimgui.c.igGetStyle().*.FramePadding.y * 2; + break :height cimgui.c.igGetTextLineHeightWithSpacing() * item_count + padding; + }; + if (cimgui.c.igBeginListBox("##bindings", .{ .x = 0, .y = height })) { + defer cimgui.c.igEndListBox(); + for (self.binding) |action| { + _ = cimgui.c.igSelectable_Bool( + @tagName(action).ptr, + false, + cimgui.c.ImGuiSelectableFlags_None, + .{ .x = 0, .y = 0 }, + ); + } + } } pty: { From c11febd0dd8d31e20c70227e0e1af6e212181b1b Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Mon, 22 Dec 2025 20:42:50 -0800 Subject: [PATCH 17/17] cli/list-keybinds: support chained keybindings --- src/cli/list_keybinds.zig | 54 +++++++++++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/src/cli/list_keybinds.zig b/src/cli/list_keybinds.zig index fb7ad19ec..2fb900e48 100644 --- a/src/cli/list_keybinds.zig +++ b/src/cli/list_keybinds.zig @@ -96,7 +96,7 @@ const TriggerNode = struct { const ChordBinding = struct { triggers: std.SinglyLinkedList, - action: Binding.Action, + actions: []const Binding.Action, // Order keybinds based on various properties // 1. Longest chord sequence @@ -281,16 +281,32 @@ fn prettyPrint(alloc: Allocator, keybinds: Config.Keybinds) !u8 { } } - const action = try std.fmt.allocPrint(alloc, "{f}", .{bind.action}); - // If our action has an argument, we print the argument in a different color - if (std.mem.indexOfScalar(u8, action, ':')) |idx| { - _ = win.print(&.{ - .{ .text = action[0..idx] }, - .{ .text = action[idx .. idx + 1], .style = .{ .dim = true } }, - .{ .text = action[idx + 1 ..], .style = .{ .fg = .{ .index = 5 } } }, - }, .{ .col_offset = widest_chord + 3 }); - } else { - _ = win.printSegment(.{ .text = action }, .{ .col_offset = widest_chord + 3 }); + var action_col: u16 = widest_chord + 3; + for (bind.actions, 0..) |act, i| { + if (i > 0) { + const chain_result = win.printSegment( + .{ .text = ", ", .style = .{ .dim = true } }, + .{ .col_offset = action_col }, + ); + action_col = chain_result.col; + } + + const action = try std.fmt.allocPrint(alloc, "{f}", .{act}); + // If our action has an argument, we print the argument in a different color + if (std.mem.indexOfScalar(u8, action, ':')) |idx| { + const print_result = win.print(&.{ + .{ .text = action[0..idx] }, + .{ .text = action[idx .. idx + 1], .style = .{ .dim = true } }, + .{ .text = action[idx + 1 ..], .style = .{ .fg = .{ .index = 5 } } }, + }, .{ .col_offset = action_col }); + action_col = print_result.col; + } else { + const print_result = win.printSegment( + .{ .text = action }, + .{ .col_offset = action_col }, + ); + action_col = print_result.col; + } } try vx.prettyPrint(writer); } @@ -346,14 +362,24 @@ fn iterateBindings( const node = try alloc.create(TriggerNode); node.* = .{ .data = bind.key_ptr.* }; + const actions = try alloc.alloc(Binding.Action, 1); + actions[0] = leaf.action; + widest_chord = @max(widest_chord, width); try bindings.append(alloc, .{ .triggers = .{ .first = &node.node }, - .action = leaf.action, + .actions = actions, }); }, - .leaf_chained => { - // TODO: Show these. + .leaf_chained => |leaf| { + const node = try alloc.create(TriggerNode); + node.* = .{ .data = bind.key_ptr.* }; + + widest_chord = @max(widest_chord, width); + try bindings.append(alloc, .{ + .triggers = .{ .first = &node.node }, + .actions = leaf.actions.items, + }); }, } }