From b5f3e8bcc8b063a5a28205078cf5830e9298577b Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Thu, 24 Sep 2026 08:43:39 -0700 Subject: [PATCH] keybinding semantic validation --- docs/reference/config/keys-srf.md | 6 + src/tui.zig | 234 +++++++++++++++++++++++++++++- src/tui/keybinds.zig | 74 ++++++++++ 3 files changed, 308 insertions(+), 6 deletions(-) diff --git a/docs/reference/config/keys-srf.md b/docs/reference/config/keys-srf.md index d537448..07f73c8 100644 --- a/docs/reference/config/keys-srf.md +++ b/docs/reference/config/keys-srf.md @@ -37,6 +37,12 @@ action::ACTION_NAME,key::KEY_STRING[,scope::SCOPE] A tab-local binding may not reuse a globally-bound key; zfin refuses to start if you create that conflict. +A binding whose `scope` names no tab, or whose `action` isn't one of +that tab's actions, never fires. It also doesn't fall back to the +tab's default binding, because any override for a tab replaces all of +its defaults. zfin prints a warning for each one when the TUI starts, +and `zfin doctor` reports them with line numbers. + ## Default global bindings ```srf diff --git a/src/tui.zig b/src/tui.zig index 460841c..ebffdd2 100644 --- a/src/tui.zig +++ b/src/tui.zig @@ -106,6 +106,109 @@ pub const Tab = blk: { break :blk @Enum(u8, .exhaustive, &names, &values); }; +// ── keys.srf tab-scoped bindings ────────────────────────────── + +/// Why a `keys.srf` tab-scoped binding can never fire. +pub const ScopedBindingCheck = enum { + ok, + /// `scope::` names no tab, so the binding lands in a bucket + /// nothing ever consults. + unknown_scope, + /// The tab exists, but `action::` is not one of its `Action` enum + /// tags. The dispatcher finds no match and the key does nothing - + /// and because any override for a tab replaces its defaults + /// wholesale, it does not fall back to a default binding either. + unknown_action, +}; + +/// The one definition of "is this tab-scoped binding real", derived +/// from `tab_modules` at comptime. `keybinds.zig` deliberately doesn't +/// know about tabs, so it can't check this while loading; the TUI +/// startup path (`warnScopedBindingProblems`) and `zfin doctor` (via +/// `keybinds.srf_schema.semanticCheck`) both ask here instead. +/// +/// `scope::global` is not a tab scope; callers handle it before this. +pub fn checkScopedBinding(scope: []const u8, action: []const u8) ScopedBindingCheck { + inline for (std.meta.fields(@TypeOf(tab_modules))) |field| { + if (std.mem.eql(u8, field.name, scope)) { + const ActionT = @field(tab_modules, field.name).Action; + return if (std.meta.stringToEnum(ActionT, action) != null) .ok else .unknown_action; + } + } + return .unknown_scope; +} + +/// "global, portfolio, analysis, ..." - every valid `scope::` value. +const valid_scope_list: []const u8 = blk: { + var s: []const u8 = "global"; + for (std.meta.fields(@TypeOf(tab_modules))) |f| s = s ++ ", " ++ f.name; + break :blk s; +}; + +/// Comma-separated `Action` tags for tab `scope`, or null for a scope +/// that is not a tab. Empty for a tab with no local actions. +fn tabActionList(scope: []const u8) ?[]const u8 { + inline for (std.meta.fields(@TypeOf(tab_modules))) |field| { + if (std.mem.eql(u8, field.name, scope)) { + const list = comptime blk: { + var s: []const u8 = ""; + for (std.meta.fields(@field(tab_modules, field.name).Action), 0..) |af, i| { + s = s ++ (if (i == 0) "" else ", ") ++ af.name; + } + break :blk s; + }; + return list; + } + } + return null; +} + +/// The wording for a binding that `checkScopedBinding` rejected. +/// Shared by the TUI startup warning and `zfin doctor` so the two +/// cannot describe the same mistake differently. +pub fn writeScopedBindingProblem( + w: *std.Io.Writer, + scope: []const u8, + action: []const u8, + problem: ScopedBindingCheck, +) std.Io.Writer.Error!void { + switch (problem) { + .ok => {}, + .unknown_scope => try w.print( + "scope '{s}' is not a tab, so the binding for action '{s}' never fires; valid scopes: {s}", + .{ scope, action, valid_scope_list }, + ), + .unknown_action => { + const actions = tabActionList(scope) orelse ""; + if (actions.len == 0) { + try w.print("action '{s}' does nothing: the {s} tab has no local actions", .{ action, scope }); + } else { + try w.print( + "action '{s}' is not a {s} tab action, so that key does nothing there; valid: {s}", + .{ action, scope, actions }, + ); + } + }, + } +} + +/// Write one `zfin: keys.srf: ...` line per tab-scoped binding in +/// `keymap` that can never fire. Returns how many were written. +fn warnScopedBindingProblems(w: *std.Io.Writer, keymap: keybinds.KeyMap) std.Io.Writer.Error!usize { + var n: usize = 0; + for (keymap.tab_overrides) |to| { + for (to.bindings) |b| { + const problem = checkScopedBinding(to.scope, b.action_name); + if (problem == .ok) continue; + try w.writeAll("zfin: keys.srf: "); + try writeScopedBindingProblem(w, to.scope, b.action_name, problem); + try w.writeAll("\n"); + n += 1; + } + } + return n; +} + /// Comptime lookup table of tab-bar display labels, indexed by /// `@intFromEnum(tab)`. Each entry is `" {N}:{label} "` composed /// from the 1-indexed registry position + the tab module's @@ -946,9 +1049,11 @@ pub const App = struct { } } // Action name didn't resolve; treat as - // unbound (silent - keys.srf parsing - // already validated the action exists, - // future work). + // unbound. The loader can't validate + // tab-scoped action names, so this is + // reported at startup by + // `warnScopedBindingProblems` and by + // `zfin doctor`, not here per keypress. return false; } } @@ -2574,14 +2679,18 @@ pub fn run( defer keymap.deinit(); // Surface per-record parse warnings (unknown action, malformed - // key, etc.) to stderr. Non-fatal - the keymap is otherwise - // usable; user just sees that some lines didn't take effect. - if (keymap.warnings.len > 0) { + // key, etc.) to stderr, plus tab-scoped bindings that name no tab + // or no action of that tab - the loader can't check those because + // it doesn't know about tab modules. Non-fatal - the keymap is + // otherwise usable; user just sees that some lines didn't take + // effect. + { var stderr_buf: [4096]u8 = undefined; var stderr_writer = std.Io.File.stderr().writer(io, &stderr_buf); for (keymap.warnings) |w| { try stderr_writer.interface.print("zfin: {s}\n", .{w}); } + _ = try warnScopedBindingProblems(&stderr_writer.interface, keymap); try stderr_writer.interface.flush(); } @@ -3381,3 +3490,116 @@ test "toggleOverlay: same-sym match is case-insensitive" { app.toggleOverlay("vti"); try testing.expectEqualStrings("", app.overlay_symbol); } + +// ── checkScopedBinding / warnScopedBindingProblems ── + +test "checkScopedBinding: real tab and action is ok" { + try testing.expectEqual(ScopedBindingCheck.ok, checkScopedBinding("options", "expand_collapse")); + try testing.expectEqual(ScopedBindingCheck.ok, checkScopedBinding("portfolio", "sort_reverse")); +} + +test "checkScopedBinding: misspelled scope and misspelled action" { + try testing.expectEqual(ScopedBindingCheck.unknown_scope, checkScopedBinding("optons", "expand_collapse")); + try testing.expectEqual(ScopedBindingCheck.unknown_action, checkScopedBinding("options", "expand_colapse")); + // An action that exists, but on a different tab. + try testing.expectEqual(ScopedBindingCheck.unknown_action, checkScopedBinding("options", "sort_reverse")); +} + +test "checkScopedBinding: every registered tab is a valid scope" { + // Derived from `tab_modules`, so a new tab can't be forgotten. + inline for (std.meta.fields(@TypeOf(tab_modules))) |f| { + try testing.expect(checkScopedBinding(f.name, "__no_such_action__") != .unknown_scope); + } +} + +test "writeScopedBindingProblem: wording lists the valid choices" { + var buf: [512]u8 = undefined; + { + var w = std.Io.Writer.fixed(&buf); + try writeScopedBindingProblem(&w, "optons", "expand_collapse", .unknown_scope); + const msg = w.buffered(); + try testing.expect(std.mem.indexOf(u8, msg, "scope 'optons' is not a tab") != null); + try testing.expect(std.mem.indexOf(u8, msg, "valid scopes: global, portfolio,") != null); + try testing.expect(std.mem.indexOf(u8, msg, "options") != null); + } + { + var w = std.Io.Writer.fixed(&buf); + try writeScopedBindingProblem(&w, "options", "expand_colapse", .unknown_action); + const msg = w.buffered(); + try testing.expect(std.mem.startsWith(u8, msg, "action 'expand_colapse' is not a options tab action, so that key does nothing there; valid: ")); + // Every one of the tab's actions is offered - derived from the + // enum here too, not a hand-copied list. + inline for (std.meta.fields(@field(tab_modules, "options").Action)) |af| { + try testing.expect(std.mem.indexOf(u8, msg, af.name) != null); + } + } + // A tab with no local actions at all gets a message that says so, + // rather than an empty "valid:" list. Found from the registry rather + // than named, so the test can't go stale when a tab gains actions. + var checked_empty = false; + inline for (std.meta.fields(@TypeOf(tab_modules))) |f| { + if (comptime std.meta.fields(@field(tab_modules, f.name).Action).len == 0) { + var w = std.Io.Writer.fixed(&buf); + try writeScopedBindingProblem(&w, f.name, "anything", .unknown_action); + try testing.expectEqualStrings( + "action 'anything' does nothing: the " ++ f.name ++ " tab has no local actions", + w.buffered(), + ); + checked_empty = true; + } + } + try testing.expect(checked_empty); +} + +test "warnScopedBindingProblems: reports only the bindings that can't fire" { + const km: keybinds.KeyMap = .{ + .bindings = &.{}, + .tab_overrides = &.{ + .{ .scope = "options", .bindings = &.{ + .{ .action_name = "expand_collapse", .key = .{ .codepoint = 'x' } }, + .{ .action_name = "expand_colapse", .key = .{ .codepoint = 'y' } }, + } }, + .{ .scope = "optons", .bindings = &.{ + .{ .action_name = "expand_collapse", .key = .{ .codepoint = 'z' } }, + } }, + }, + }; + var buf: [2048]u8 = undefined; + var w = std.Io.Writer.fixed(&buf); + try testing.expectEqual(@as(usize, 2), try warnScopedBindingProblems(&w, km)); + const out = w.buffered(); + try testing.expect(std.mem.indexOf(u8, out, "zfin: keys.srf: action 'expand_colapse'") != null); + try testing.expect(std.mem.indexOf(u8, out, "zfin: keys.srf: scope 'optons'") != null); + // The valid binding is not mentioned. + try testing.expect(std.mem.indexOf(u8, out, "'expand_collapse' is not") == null); +} + +test "warnScopedBindingProblems: the built-in defaults are clean" { + var buf: [256]u8 = undefined; + var w = std.Io.Writer.fixed(&buf); + try testing.expectEqual(@as(usize, 0), try warnScopedBindingProblems(&w, keybinds.defaults())); +} + +test "writeScopedBindingProblem: every message fits in a lint finding" { + // `srf_lint.Finding.describe` truncates at `describe_max`, and these + // messages list the valid choices. A tab that grows enough actions + // would silently clip its list in `zfin doctor` - this fails first. + const srf_lint = @import("srf_lint.zig"); + // Generous stand-ins for a user's typo. + const long_scope = "a_long_misspelled_scope_name_xx"; + const long_action = "a_long_misspelled_action_name_xxxxxxxx"; + var buf: [4096]u8 = undefined; + { + var w = std.Io.Writer.fixed(&buf); + try writeScopedBindingProblem(&w, long_scope, long_action, .unknown_scope); + try testing.expect(w.buffered().len <= srf_lint.Finding.describe_max); + } + inline for (std.meta.fields(@TypeOf(tab_modules))) |f| { + var w = std.Io.Writer.fixed(&buf); + try writeScopedBindingProblem(&w, f.name, long_action, .unknown_action); + if (w.buffered().len > srf_lint.Finding.describe_max) { + std.debug.print("{s} tab message is {d} bytes, describe_max is {d}\n", .{ f.name, w.buffered().len, srf_lint.Finding.describe_max }); + return error.MessageTooLong; + } + } +} diff --git a/src/tui/keybinds.zig b/src/tui/keybinds.zig index e79a127..3542eaf 100644 --- a/src/tui/keybinds.zig +++ b/src/tui/keybinds.zig @@ -2,6 +2,11 @@ const std = @import("std"); const vaxis = @import("vaxis"); const srf = @import("srf"); const srf_opts = @import("../srf_opts.zig"); +const srf_lint = @import("../srf_lint.zig"); +// Cycle-safe: `tui.zig` imports this file too, and Zig resolves the +// loop lazily. Needed only by `srf_schema.semanticCheck`, which asks +// `tui.checkScopedBinding` because only `tui.zig` knows the tab set. +const tui = @import("../tui.zig"); pub const Action = enum { quit, @@ -432,6 +437,34 @@ pub const srf_schema = struct { pub const Record = RawRecord; pub const file_label: []const u8 = "keys.srf"; pub const doc_path: []const u8 = "reference/config/keys-srf.md"; + + /// Bindings that load but can never fire. The TUI prints the same + /// problems at startup; this puts them in `zfin doctor` with a line + /// number. The tab-scoped rule and its wording live in `tui.zig` + /// (`checkScopedBinding`, `writeScopedBindingProblem`), so there is + /// one definition for both. + pub fn semanticCheck(rec: Record, ctx: srf_lint.Context, sink: *srf_lint.Sink) !void { + _ = ctx; + const is_global = rec.scope == null or std.mem.eql(u8, rec.scope.?, "global"); + if (is_global) { + // Same test the loader uses to skip the record. + if (parseAction(rec.action) == null) { + try sink.addOwned(.semantic, "action", try std.fmt.allocPrint( + sink.allocator, + "unknown global action '{s}'; binding skipped (`zfin interactive --default-keys` lists them)", + .{rec.action}, + )); + } + return; + } + const scope = rec.scope.?; + const problem = tui.checkScopedBinding(scope, rec.action); + if (problem == .ok) return; + var aw: std.Io.Writer.Allocating = .init(sink.allocator); + errdefer aw.deinit(); + try tui.writeScopedBindingProblem(&aw.writer, scope, rec.action, problem); + try sink.addOwned(.semantic, if (problem == .unknown_scope) "scope" else "action", try aw.toOwnedSlice()); + } }; /// Load keybindings from an SRF file. Returns null if the file doesn't exist @@ -792,3 +825,44 @@ test "printSectionHeader writes a comment header line" { try printSectionHeader(&aw.writer, "Tab: portfolio"); try std.testing.expectEqualStrings("\n# ── Tab: portfolio ──\n", aw.written()); } + +// ── srf_schema.semanticCheck ── + +fn lintKeys(data: []const u8) ![]u8 { + const allocator = std.testing.allocator; + var r = try srf_lint.check(allocator, data, srf_lint.shapeOfSchema(srf_schema)); + defer r.deinit(); + try srf_lint.checkSemantic(srf_schema, &r, data, .{ .today = @import("../Date.zig").fromYmd(2026, 1, 1) }); + r.sort(); + var aw: std.Io.Writer.Allocating = .init(allocator); + errdefer aw.deinit(); + var buf: [srf_lint.Finding.describe_max]u8 = undefined; + for (r.findings) |f| try aw.writer.print("{d}: {s}\n", .{ f.first_line, f.describe(&buf) }); + return aw.toOwnedSlice(); +} + +test "srf_schema.semanticCheck: valid global and scoped bindings are clean" { + const got = try lintKeys( + \\#!srfv1 + \\action::quit,key::q + \\action::expand_collapse,key::x,scope::options + \\action::refresh,key::F5,scope::global + \\ + ); + defer std.testing.allocator.free(got); + try std.testing.expectEqualStrings("", got); +} + +test "srf_schema.semanticCheck: misspelled scope, scoped action, and global action" { + const got = try lintKeys( + \\#!srfv1 + \\action::expand_collapse,key::x,scope::optons + \\action::expand_colapse,key::y,scope::options + \\action::quitt,key::Q + \\ + ); + defer std.testing.allocator.free(got); + try std.testing.expect(std.mem.indexOf(u8, got, "2: scope 'optons' is not a tab") != null); + try std.testing.expect(std.mem.indexOf(u8, got, "3: action 'expand_colapse' is not a options tab action") != null); + try std.testing.expect(std.mem.indexOf(u8, got, "4: unknown global action 'quitt'") != null); +}