mirror of
https://github.com/ghostty-org/ghostty.git
synced 2026-04-18 13:30:29 +00:00
config: improve key table parsing robustness (#10022)
Fixes #10020 This improves parsing key tables so that the following edge cases are now handled correctly, which were regressions from prior tip behavior: - `/=action` - `ctrl+/=action` - `table//=action` (valid to bind `/` in a table) - `table/a>//=action` (valid to bind a table with a sequence)
This commit is contained in:
@@ -1714,8 +1714,10 @@ class: ?[:0]const u8 = null,
|
||||
/// Key tables are defined using the syntax `<table>/<binding>`. The
|
||||
/// `<binding>` value is everything documented above for keybinds. The
|
||||
/// `<table>` value is the name of the key table. Table names can contain
|
||||
/// anything except `/` and `=`. For example `foo/ctrl+a=new_window`
|
||||
/// defines a binding within a table named `foo`.
|
||||
/// anything except `/`, `=`, `+`, and `>`. The characters `+` and `>` are
|
||||
/// reserved for keybind syntax (modifier combinations and key sequences).
|
||||
/// For example `foo/ctrl+a=new_window` defines a binding within a table
|
||||
/// named `foo`.
|
||||
///
|
||||
/// Tables are activated and deactivated using the binding actions
|
||||
/// `activate_key_table:<name>` and `deactivate_key_table`. Other table
|
||||
@@ -6682,12 +6684,21 @@ pub const Keybinds = struct {
|
||||
// We look for '/' only before the first '=' to avoid matching
|
||||
// action arguments like "foo=text:/hello".
|
||||
const eq_idx = std.mem.indexOfScalar(u8, value, '=') orelse value.len;
|
||||
if (std.mem.indexOfScalar(u8, value[0..eq_idx], '/')) |slash_idx| {
|
||||
if (std.mem.indexOfScalar(u8, value[0..eq_idx], '/')) |slash_idx| table: {
|
||||
const table_name = value[0..slash_idx];
|
||||
const binding = value[slash_idx + 1 ..];
|
||||
|
||||
// Table name cannot be empty
|
||||
if (table_name.len == 0) return error.InvalidFormat;
|
||||
// Length zero is valid, so you can set `/=action` for the slash key
|
||||
if (table_name.len == 0) break :table;
|
||||
|
||||
// Ignore '+', '>' because they can be part of sequences and
|
||||
// triggers. This lets things like `ctrl+/=action` work.
|
||||
if (std.mem.indexOfAny(
|
||||
u8,
|
||||
table_name,
|
||||
"+>",
|
||||
) != null) break :table;
|
||||
|
||||
const binding = value[slash_idx + 1 ..];
|
||||
|
||||
// Get or create the table
|
||||
const gop = try self.tables.getOrPut(alloc, table_name);
|
||||
@@ -7055,6 +7066,105 @@ pub const Keybinds = struct {
|
||||
try testing.expectEqual(0, keybinds.tables.count());
|
||||
}
|
||||
|
||||
test "parseCLI slash as key with modifier is not a table" {
|
||||
const testing = std.testing;
|
||||
var arena = ArenaAllocator.init(testing.allocator);
|
||||
defer arena.deinit();
|
||||
const alloc = arena.allocator();
|
||||
|
||||
var keybinds: Keybinds = .{};
|
||||
|
||||
// ctrl+/ should be parsed as a keybind with '/' as the key, not a table
|
||||
try keybinds.parseCLI(alloc, "ctrl+/=text:foo");
|
||||
|
||||
// Should be in root set, not a table
|
||||
try testing.expectEqual(1, keybinds.set.bindings.count());
|
||||
try testing.expectEqual(0, keybinds.tables.count());
|
||||
}
|
||||
|
||||
test "parseCLI shift+slash as key is not a table" {
|
||||
const testing = std.testing;
|
||||
var arena = ArenaAllocator.init(testing.allocator);
|
||||
defer arena.deinit();
|
||||
const alloc = arena.allocator();
|
||||
|
||||
var keybinds: Keybinds = .{};
|
||||
|
||||
// shift+/ should be parsed as a keybind, not a table
|
||||
try keybinds.parseCLI(alloc, "shift+/=ignore");
|
||||
|
||||
// Should be in root set, not a table
|
||||
try testing.expectEqual(1, keybinds.set.bindings.count());
|
||||
try testing.expectEqual(0, keybinds.tables.count());
|
||||
}
|
||||
|
||||
test "parseCLI bare slash as key is not a table" {
|
||||
const testing = std.testing;
|
||||
var arena = ArenaAllocator.init(testing.allocator);
|
||||
defer arena.deinit();
|
||||
const alloc = arena.allocator();
|
||||
|
||||
var keybinds: Keybinds = .{};
|
||||
|
||||
// Bare / as a key should work (empty table name is rejected)
|
||||
try keybinds.parseCLI(alloc, "/=text:foo");
|
||||
|
||||
// Should be in root set, not a table
|
||||
try testing.expectEqual(1, keybinds.set.bindings.count());
|
||||
try testing.expectEqual(0, keybinds.tables.count());
|
||||
}
|
||||
|
||||
test "parseCLI slash in key sequence is not a table" {
|
||||
const testing = std.testing;
|
||||
var arena = ArenaAllocator.init(testing.allocator);
|
||||
defer arena.deinit();
|
||||
const alloc = arena.allocator();
|
||||
|
||||
var keybinds: Keybinds = .{};
|
||||
|
||||
// Key sequence ending with / should work
|
||||
try keybinds.parseCLI(alloc, "ctrl+a>ctrl+/=new_window");
|
||||
|
||||
// Should be in root set, not a table
|
||||
try testing.expectEqual(1, keybinds.set.bindings.count());
|
||||
try testing.expectEqual(0, keybinds.tables.count());
|
||||
}
|
||||
|
||||
test "parseCLI table with slash in binding" {
|
||||
const testing = std.testing;
|
||||
var arena = ArenaAllocator.init(testing.allocator);
|
||||
defer arena.deinit();
|
||||
const alloc = arena.allocator();
|
||||
|
||||
var keybinds: Keybinds = .{};
|
||||
|
||||
// Table with a binding that uses / as the key
|
||||
try keybinds.parseCLI(alloc, "mytable//=text:foo");
|
||||
|
||||
// Should be in the table
|
||||
try testing.expectEqual(0, keybinds.set.bindings.count());
|
||||
try testing.expectEqual(1, keybinds.tables.count());
|
||||
try testing.expect(keybinds.tables.contains("mytable"));
|
||||
try testing.expectEqual(1, keybinds.tables.get("mytable").?.bindings.count());
|
||||
}
|
||||
|
||||
test "parseCLI table with sequence containing slash" {
|
||||
const testing = std.testing;
|
||||
var arena = ArenaAllocator.init(testing.allocator);
|
||||
defer arena.deinit();
|
||||
const alloc = arena.allocator();
|
||||
|
||||
var keybinds: Keybinds = .{};
|
||||
|
||||
// Table with a key sequence that ends with /
|
||||
try keybinds.parseCLI(alloc, "mytable/a>/=new_window");
|
||||
|
||||
// Should be in the table
|
||||
try testing.expectEqual(0, keybinds.set.bindings.count());
|
||||
try testing.expectEqual(1, keybinds.tables.count());
|
||||
try testing.expect(keybinds.tables.contains("mytable"));
|
||||
}
|
||||
|
||||
test "clone with tables" {
|
||||
const testing = std.testing;
|
||||
var arena = ArenaAllocator.init(testing.allocator);
|
||||
|
||||
Reference in New Issue
Block a user