keybind: format leader bindings into multiple entries

**Context**

Currently, if there are multiple keybindings with a shared prefix,
they are grouped into a nested series of Binding.Sets.
For example, as reported in #2734, the following bindings:

    keybind = ctrl+z>1=goto_tab:1
    keybind = ctrl+z>2=goto_tab:2
    keybind = ctrl+z>3=goto_tab:3

Result in roughly the following structure (in pseudo-code):

    Keybinds{
        Trigger("ctrl+z"): Value.leader{
            Trigger("1"): Value.leaf{action: "goto_tab:1"}),
            Trigger("2"): Value.leaf{action: "goto_tab:2"}),
            Trigger("3"): Value.leaf{action: "goto_tab:3"}),
        }
    }

When this is formatted into a string (and therefore in +list-keybinds),
it is turned into the following as Value.format just concatenates
all the sibling bindings ('1', '2', '3') into consecutive bindings,
and this is then fed into a single configuration entry:

    keybind = ctrl+z>1=goto_tab:1>3=goto_tab:3>2=goto_tab:2

**Fix**

To fix this, Value needs to produce a separate configuration entry
for each sibling binding in the Value.leader case.
So we can't produce the entry (formatter.formatEntry) in Keybinds
and need to pass information down the Value tree to the leaf nodes,
each of which will produce a separate entry with that function.

This is accomplished with the help of a new Value.formatEntries method
that recursively builds up the prefix for the keybinding,
finally flushing it to the formatter when it reaches a leaf node.

This is done without extra allocations by using a FixedBufferStream
with the same buffer as before, sharing it between calls to nested
siblings of the same prefix.

**Caveats**

We do not track the order in which the bindings were added
so the order is not retained in the formatConfig output.

Resolves #2734
This commit is contained in:
Abhinav Gupta
2024-12-10 21:05:33 -08:00
parent 59df17a699
commit e2e12efbbf
2 changed files with 88 additions and 8 deletions

View File

@@ -4213,14 +4213,9 @@ pub const Keybinds = struct {
}
}
try formatter.formatEntry(
[]const u8,
std.fmt.bufPrint(
&buf,
"{}{}",
.{ k, v },
) catch return error.OutOfMemory,
);
var buffer_stream = std.io.fixedBufferStream(&buf);
try std.fmt.format(buffer_stream.writer(), "{}", .{k});
try v.formatEntries(&buffer_stream, formatter);
}
}
@@ -4254,6 +4249,56 @@ pub const Keybinds = struct {
try list.formatEntry(formatterpkg.entryFormatter("a", buf.writer()));
try std.testing.expectEqualSlices(u8, "a = shift+a=csi:hello\n", buf.items);
}
// Regression test for https://github.com/ghostty-org/ghostty/issues/2734
test "formatConfig multiple items" {
const testing = std.testing;
var buf = std.ArrayList(u8).init(testing.allocator);
defer buf.deinit();
var arena = ArenaAllocator.init(testing.allocator);
defer arena.deinit();
const alloc = arena.allocator();
var list: Keybinds = .{};
try list.parseCLI(alloc, "ctrl+z>1=goto_tab:1");
try list.parseCLI(alloc, "ctrl+z>2=goto_tab:2");
try list.formatEntry(formatterpkg.entryFormatter("keybind", buf.writer()));
const want =
\\keybind = ctrl+z>1=goto_tab:1
\\keybind = ctrl+z>2=goto_tab:2
\\
;
try std.testing.expectEqualStrings(want, buf.items);
}
test "formatConfig multiple items nested" {
const testing = std.testing;
var buf = std.ArrayList(u8).init(testing.allocator);
defer buf.deinit();
var arena = ArenaAllocator.init(testing.allocator);
defer arena.deinit();
const alloc = arena.allocator();
var list: Keybinds = .{};
try list.parseCLI(alloc, "ctrl+a>ctrl+b>n=new_window");
try list.parseCLI(alloc, "ctrl+a>ctrl+b>w=close_window");
try list.parseCLI(alloc, "ctrl+a>ctrl+c>t=new_tab");
try list.parseCLI(alloc, "ctrl+b>ctrl+d>a=previous_tab");
try list.formatEntry(formatterpkg.entryFormatter("a", buf.writer()));
// NB: This does not currently retain the order of the keybinds.
const want =
\\a = ctrl+a>ctrl+b>w=close_window
\\a = ctrl+a>ctrl+b>n=new_window
\\a = ctrl+a>ctrl+c>t=new_tab
\\a = ctrl+b>ctrl+d>a=previous_tab
\\
;
try std.testing.expectEqualStrings(want, buf.items);
}
};
/// See "font-codepoint-map" for documentation.