diff --git a/src/tui.zig b/src/tui.zig index a2c1716..f42d516 100644 --- a/src/tui.zig +++ b/src/tui.zig @@ -809,9 +809,13 @@ pub const App = struct { } /// Tab-local keybind dispatch. Walks the active tab's - /// `default_bindings` looking for a key match. On match, - /// invokes `tab.handleAction(state, app, action)` and returns - /// `true`. Returns `false` if no binding matched. + /// keybindings looking for a key match. If the user's keys.srf + /// has a `scope::` override for the active tab, that + /// list is consulted (action names resolved against the tab's + /// local `Action` enum). Otherwise the tab module's + /// `default_bindings` is used. On match, invokes + /// `tab.handleAction(state, app, action)` and returns `true`. + /// Returns `false` if no binding matched. /// /// Called as a fallback AFTER the global keymap; under the /// "globals always win" rule, tabs are forbidden (by validator @@ -826,9 +830,34 @@ pub const App = struct { inline for (std.meta.fields(@TypeOf(tab_modules))) |field| { if (std.mem.eql(u8, field.name, @tagName(self.active_tab))) { const Module = @field(tab_modules, field.name); + const state_ptr = &@field(self.states, field.name); + + // Prefer user overrides if present for this tab. + if (self.keymap.tabOverridesFor(field.name)) |overrides| { + for (overrides) |ovr| { + if (key.matches(ovr.key.codepoint, ovr.key.mods)) { + // Resolve action_name against the tab's Action enum. + const ActionT = Module.Action; + inline for (std.meta.fields(ActionT)) |af| { + if (std.mem.eql(u8, af.name, ovr.action_name)) { + const action: ActionT = @enumFromInt(af.value); + Module.tab.handleAction(state_ptr, self, action); + return true; + } + } + // Action name didn't resolve; treat as + // unbound (silent — keys.srf parsing + // already validated the action exists, + // future work). + return false; + } + } + return false; + } + + // No user overrides — use the tab's default_bindings. for (Module.tab.default_bindings) |binding| { if (key.matches(binding.key.codepoint, binding.key.mods)) { - const state_ptr = &@field(self.states, field.name); Module.tab.handleAction(state_ptr, self, binding.action); return true; } @@ -1385,18 +1414,6 @@ pub const App = struct { return ctx.consumeAndRedraw(); } }, - .chart_timeframe_next => { - if (self.active_tab == .quote) { - tab_modules.quote.tab.handleAction(&self.states.quote, self, tab_modules.quote.Action.chart_timeframe_next); - return ctx.consumeAndRedraw(); - } - }, - .chart_timeframe_prev => { - if (self.active_tab == .quote) { - tab_modules.quote.tab.handleAction(&self.states.quote, self, tab_modules.quote.Action.chart_timeframe_prev); - return ctx.consumeAndRedraw(); - } - }, .history_metric_next => { if (self.active_tab == .history) { tab_modules.history.tab.handleAction(&self.states.history, self, tab_modules.history.Action.metric_next); @@ -2269,10 +2286,44 @@ pub fn run( const keys_path = std.fs.path.join(allocator, &.{ home, ".config", "zfin", "keys.srf" }) catch break :blk keybinds.defaults(); defer allocator.free(keys_path); - break :blk keybinds.loadFromFile(io, allocator, keys_path) orelse keybinds.defaults(); + switch (keybinds.loadFromFileChecked(io, allocator, keys_path)) { + .keymap => |km| break :blk km, + .fallback => break :blk keybinds.defaults(), + .err => |e| switch (e) { + error.TabBindingShadowsGlobal => { + // User keys.srf has a `scope::` record whose + // key is also bound globally. Globals always win, + // so the scoped binding would be dead. Refuse to + // start so the user knows their config is broken. + var stderr_buf: [4096]u8 = undefined; + var stderr_writer = std.Io.File.stderr().writer(io, &stderr_buf); + try stderr_writer.interface.print( + "zfin: keys.srf has a tab-scoped binding whose key is already " ++ + "bound in the global keymap. Tab-local bindings cannot override " ++ + "global keys.\n Edit {s} to remove the conflict, or remove the " ++ + "global binding.\n", + .{keys_path}, + ); + try stderr_writer.interface.flush(); + return error.KeyBindingConflict; + }, + }, + } }; 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) { + 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 stderr_writer.interface.flush(); + } + const loaded_theme = blk: { const home_opt = if (config.environ_map) |em| em.get("HOME") else null; const home = home_opt orelse break :blk theme.default_theme; diff --git a/src/tui/keybinds.zig b/src/tui/keybinds.zig index 9dbccda..b79d2f7 100644 --- a/src/tui/keybinds.zig +++ b/src/tui/keybinds.zig @@ -39,8 +39,6 @@ pub const Action = enum { options_filter_7, options_filter_8, options_filter_9, - chart_timeframe_next, - chart_timeframe_prev, history_metric_next, history_resolution_next, /// History tab: toggle inclusion of the row under the cursor in the @@ -72,6 +70,14 @@ pub const Action = enum { pub const KeyCombo = struct { codepoint: u21, mods: vaxis.Key.Modifiers = .{}, + + /// SRF custom parser. Used by `srf.Record.to(...)` to coerce a + /// `key::ctrl+c` field into a `KeyCombo` value. Returns + /// `error.CustomParseFailed` (via the srf-level wrapping) on + /// invalid input. + pub fn srfParse(val: []const u8) !KeyCombo { + return parseKeyCombo(val) orelse error.InvalidKeyCombo; + } }; pub const Binding = struct { @@ -79,8 +85,38 @@ pub const Binding = struct { key: KeyCombo, }; +/// A user-specified keybinding scoped to a particular tab. The +/// `action_name` is the bare tag name from the tab's local +/// `Action` enum (e.g. "collapse_all_calls" for scope "options"). +/// Resolution to the typed enum happens in the dispatcher at the +/// use site, where the tab's Action type is known. +pub const ScopedBinding = struct { + action_name: []const u8, + key: KeyCombo, +}; + +/// User overrides for a specific tab's keymap. When present in a +/// loaded `KeyMap`, the dispatcher consults these instead of the +/// tab module's `default_bindings`. Per the "user file replaces +/// defaults" semantics: presence of any record with this scope +/// fully replaces the tab's defaults (no merging). +pub const TabOverrides = struct { + /// Tag name matching a `Tab` enum variant / `tab_modules` + /// registry field (e.g. "options", "history"). + scope: []const u8, + bindings: []const ScopedBinding, +}; + pub const KeyMap = struct { bindings: []const Binding, + /// Per-tab user overrides loaded from keys.srf. Empty when no + /// scoped records were present (or when using defaults). + tab_overrides: []const TabOverrides = &.{}, + /// Per-record parse warnings (e.g. unknown action name, + /// malformed key string). Owned by `arena`. Empty when all + /// records parsed cleanly OR when this keymap is the built-in + /// defaults. Callers are expected to surface these to stderr. + warnings: []const []const u8 = &.{}, arena: ?*std.heap.ArenaAllocator = null, pub fn deinit(self: *KeyMap) void { @@ -97,6 +133,16 @@ pub const KeyMap = struct { } return null; } + + /// Returns the user-override bindings for `scope`, or null if + /// the scope has no overrides (caller should fall back to the + /// tab module's `default_bindings`). + pub fn tabOverridesFor(self: KeyMap, scope: []const u8) ?[]const ScopedBinding { + for (self.tab_overrides) |to| { + if (std.mem.eql(u8, to.scope, scope)) return to.bindings; + } + return null; + } }; // ── Defaults ───────────────────────────────────────────────── @@ -149,8 +195,6 @@ pub const global_default_bindings = [_]Binding{ .{ .action = .options_filter_7, .key = .{ .codepoint = '7', .mods = .{ .ctrl = true } } }, .{ .action = .options_filter_8, .key = .{ .codepoint = '8', .mods = .{ .ctrl = true } } }, .{ .action = .options_filter_9, .key = .{ .codepoint = '9', .mods = .{ .ctrl = true } } }, - .{ .action = .chart_timeframe_next, .key = .{ .codepoint = ']' } }, - .{ .action = .chart_timeframe_prev, .key = .{ .codepoint = '[' } }, .{ .action = .history_metric_next, .key = .{ .codepoint = 'm' } }, .{ .action = .history_resolution_next, .key = .{ .codepoint = 't' } }, // Compare bindings live AFTER the existing `s`/`c`/space bindings @@ -318,12 +362,19 @@ pub fn printDefaults(io: std.Io) !void { try out.writeAll("# If this file is removed, built-in defaults are used.\n"); try out.writeAll("# Regenerate: zfin interactive --default-keys > ~/.config/zfin/keys.srf\n"); try out.writeAll("#\n"); - try out.writeAll("# Format: action::ACTION_NAME,key::KEY_STRING\n"); + try out.writeAll("# Format: action::ACTION_NAME,key::KEY_STRING[,scope::SCOPE]\n"); try out.writeAll("# Modifiers: ctrl+, alt+, shift+ (e.g. ctrl+c)\n"); try out.writeAll("# Special keys: tab, enter, escape, space, backspace,\n"); try out.writeAll("# left, right, up, down, page_up, page_down, home, end,\n"); try out.writeAll("# F1-F12, insert, delete\n"); try out.writeAll("# Multiple lines with the same action = multiple bindings.\n"); + try out.writeAll("#\n"); + try out.writeAll("# scope:: is optional. Omitted (or `scope::global`) = global\n"); + try out.writeAll("# binding (this section). `scope::` (e.g. scope::options,\n"); + try out.writeAll("# scope::history) = tab-local binding; the action name then\n"); + try out.writeAll("# refers to that tab's local Action enum. Tab-local bindings\n"); + try out.writeAll("# cannot use a key that's globally bound — zfin will refuse\n"); + try out.writeAll("# to start if you create that conflict.\n"); for (global_default_bindings) |b| { var key_buf: [32]u8 = undefined; @@ -337,65 +388,190 @@ pub fn printDefaults(io: std.Io) !void { // ── SRF loading ────────────────────────────────────────────── fn parseAction(name: []const u8) ?Action { - inline for (std.meta.fields(Action)) |f| { - if (std.mem.eql(u8, name, f.name)) return @enumFromInt(f.value); - } - return null; + return std.meta.stringToEnum(Action, name); } +/// Errors from `loadFromData` that the caller should surface to +/// the user (stderr) rather than silently fall back to defaults. +pub const LoadError = error{ + /// A user record under `scope::` binds a key that is also + /// bound in the global keymap. Globals always win, so the + /// scoped binding would be dead. The TUI refuses to start. + TabBindingShadowsGlobal, +}; + +/// Outcome of `loadFromData`. Either a successfully-built KeyMap, +/// a hard error the caller should surface (stderr + refuse to +/// start), or a soft `null` result (file unparseable) that means +/// "fall back to defaults silently" — the existing behavior. +/// +/// Successful results may include `warnings` describing per-record +/// problems (e.g. unknown action name, malformed key string). Each +/// warning is a string allocated in the keymap's arena. The caller +/// is expected to surface these to stderr; they aren't fatal +/// because the rest of the file is usable. +pub const LoadOutcome = union(enum) { + keymap: KeyMap, + err: LoadError, + fallback, +}; + +/// One record from keys.srf, decoded via `srf.Record.to`. +/// Field names match the SRF field names; `scope` is optional and +/// defaults to null (= global). +const RawRecord = struct { + action: []const u8, + key: KeyCombo, + scope: ?[]const u8 = null, +}; + /// Load keybindings from an SRF file. Returns null if the file doesn't exist /// or can't be parsed. On success, the caller owns the returned KeyMap and /// must call deinit(). +/// +/// For richer error reporting (specifically the "tab binding +/// shadows a global" case), use `loadFromFileChecked` which +/// returns a `LoadOutcome`. pub fn loadFromFile(io: std.Io, allocator: std.mem.Allocator, path: []const u8) ?KeyMap { - const data = std.Io.Dir.cwd().readFileAlloc(io, path, allocator, .limited(64 * 1024)) catch return null; + return switch (loadFromFileChecked(io, allocator, path)) { + .keymap => |km| km, + .err, .fallback => null, + }; +} + +/// Load with detailed outcome. Used by the TUI startup path so +/// `TabBindingShadowsGlobal` can be surfaced as a hard error +/// (stderr + exit) rather than silently falling back to defaults. +pub fn loadFromFileChecked(io: std.Io, allocator: std.mem.Allocator, path: []const u8) LoadOutcome { + const data = std.Io.Dir.cwd().readFileAlloc(io, path, allocator, .limited(64 * 1024)) catch return .fallback; defer allocator.free(data); - return loadFromData(allocator, data); + return loadFromDataChecked(allocator, data); } pub fn loadFromData(allocator: std.mem.Allocator, data: []const u8) ?KeyMap { - const arena = allocator.create(std.heap.ArenaAllocator) catch return null; - arena.* = std.heap.ArenaAllocator.init(allocator); + return switch (loadFromDataChecked(allocator, data)) { + .keymap => |km| km, + .err, .fallback => null, + }; +} + +pub fn loadFromDataChecked(allocator: std.mem.Allocator, data: []const u8) LoadOutcome { + var reader = std.Io.Reader.fixed(data); + const parsed = srf.parse(&reader, allocator, .{}) catch return .fallback; + // Don't deinit `parsed` until the end — its arena owns the + // string slices we'll borrow into the returned KeyMap. We + // transfer ownership to the KeyMap's arena instead. + + // Move parsed.arena into our own KeyMap so it outlives this + // call. The `Parsed` struct holds the arena by pointer; we + // claim it directly. + const arena = parsed.arena; errdefer { arena.deinit(); allocator.destroy(arena); } const aa = arena.allocator(); - var reader = std.Io.Reader.fixed(data); - var it = srf.iterator(&reader, aa, .{}) catch return null; - // Don't defer it.deinit() -- arena owns everything + var globals = std.ArrayList(Binding).empty; + var scopes = std.ArrayList(ScopeBuilder).empty; + var warnings = std.ArrayList([]const u8).empty; - var bindings = std.ArrayList(Binding).empty; + for (parsed.records, 0..) |record, idx| { + const raw = record.to(RawRecord) catch |err| { + // Per-record parse failure (missing field, bad key + // string, unknown action). Don't drop the whole file — + // skip the record and warn the user. Record index is + // 0-based; the user-facing message uses 1-based. + const msg = std.fmt.allocPrint( + aa, + "keys.srf: record {d}: {s}; binding skipped", + .{ idx + 1, @errorName(err) }, + ) catch return .fallback; + warnings.append(aa, msg) catch return .fallback; + continue; + }; - while (it.next() catch return null) |fields| { - var action: ?Action = null; - var key: ?KeyCombo = null; - - while (fields.next() catch return null) |field| { - if (std.mem.eql(u8, field.key, "action")) { - if (field.value) |v| switch (v) { - .string => |s| action = parseAction(s), - else => {}, - }; - } else if (std.mem.eql(u8, field.key, "key")) { - if (field.value) |v| switch (v) { - .string => |s| key = parseKeyCombo(s), - else => {}, - }; + const is_global = raw.scope == null or std.mem.eql(u8, raw.scope.?, "global"); + if (is_global) { + const a = parseAction(raw.action) orelse { + const msg = std.fmt.allocPrint( + aa, + "keys.srf: record {d}: unknown global action `{s}`; binding skipped", + .{ idx + 1, raw.action }, + ) catch return .fallback; + warnings.append(aa, msg) catch return .fallback; + continue; + }; + globals.append(aa, .{ .action = a, .key = raw.key }) catch return .fallback; + } else { + // Find or create the scope bucket. Action-name validation + // against the tab's local `Action` enum happens in the + // dispatcher at use-time — keybinds.zig doesn't know + // about tab modules. + const scope_name = raw.scope.?; + var bucket: ?*ScopeBuilder = null; + for (scopes.items) |*sb| { + if (std.mem.eql(u8, sb.scope, scope_name)) { + bucket = sb; + break; + } } - } - - if (action != null and key != null) { - bindings.append(aa, .{ .action = action.?, .key = key.? }) catch return null; + if (bucket == null) { + scopes.append(aa, .{ + .scope = scope_name, + .bindings = std.ArrayList(ScopedBinding).empty, + }) catch return .fallback; + bucket = &scopes.items[scopes.items.len - 1]; + } + bucket.?.bindings.append(aa, .{ + .action_name = raw.action, + .key = raw.key, + }) catch return .fallback; } } - return .{ - .bindings = bindings.toOwnedSlice(aa) catch return null, + // Conflict check: any tab-scoped binding whose key collides + // with the (post-load) global keymap = hard error. Caller will + // print the conflict to stderr + refuse to start the TUI. + for (scopes.items) |sb| { + for (sb.bindings.items) |b| { + for (globals.items) |gb| { + if (b.key.codepoint == gb.key.codepoint and + std.meta.eql(b.key.mods, gb.key.mods)) + { + arena.deinit(); + allocator.destroy(arena); + return .{ .err = error.TabBindingShadowsGlobal }; + } + } + } + } + + // Materialize tab overrides into owned slices. + var tab_overrides = std.ArrayList(TabOverrides).empty; + for (scopes.items) |*sb| { + const slice = sb.bindings.toOwnedSlice(aa) catch return .fallback; + tab_overrides.append(aa, .{ + .scope = sb.scope, + .bindings = slice, + }) catch return .fallback; + } + + return .{ .keymap = .{ + .bindings = globals.toOwnedSlice(aa) catch return .fallback, + .tab_overrides = tab_overrides.toOwnedSlice(aa) catch return .fallback, + .warnings = warnings.toOwnedSlice(aa) catch return .fallback, .arena = arena, - }; + } }; } +/// Internal scratch type used by `loadFromDataChecked` to bucket +/// scoped bindings during parsing. +const ScopeBuilder = struct { + scope: []const u8, + bindings: std.ArrayList(ScopedBinding), +}; + // ── Tests ──────────────────────────────────────────────────── test "parseKeyCombo single char" { @@ -448,6 +624,97 @@ test "loadFromData basic" { try std.testing.expectEqual(@as(usize, 3), km.bindings.len); try std.testing.expectEqual(Action.quit, km.bindings[0].action); try std.testing.expectEqual(Action.refresh, km.bindings[2].action); + try std.testing.expectEqual(@as(usize, 0), km.tab_overrides.len); +} + +test "loadFromData scoped binding" { + const data = + \\#!srfv1 + \\action::quit,key::q + \\scope::options,action::collapse_all_calls,key::C + ; + var km = loadFromData(std.testing.allocator, data) orelse return error.ParseFailed; + defer km.deinit(); + try std.testing.expectEqual(@as(usize, 1), km.bindings.len); + try std.testing.expectEqual(@as(usize, 1), km.tab_overrides.len); + try std.testing.expectEqualStrings("options", km.tab_overrides[0].scope); + try std.testing.expectEqual(@as(usize, 1), km.tab_overrides[0].bindings.len); + try std.testing.expectEqualStrings("collapse_all_calls", km.tab_overrides[0].bindings[0].action_name); + try std.testing.expectEqual(@as(u21, 'C'), km.tab_overrides[0].bindings[0].key.codepoint); +} + +test "loadFromData scope::global is treated as global" { + const data = + \\#!srfv1 + \\scope::global,action::quit,key::q + ; + var km = loadFromData(std.testing.allocator, data) orelse return error.ParseFailed; + defer km.deinit(); + try std.testing.expectEqual(@as(usize, 1), km.bindings.len); + try std.testing.expectEqual(Action.quit, km.bindings[0].action); + try std.testing.expectEqual(@as(usize, 0), km.tab_overrides.len); +} + +test "loadFromDataChecked rejects tab binding shadowing global" { + // Both `q` (global quit) and a scoped binding for the same key. + const data = + \\#!srfv1 + \\action::quit,key::q + \\scope::options,action::collapse_all_calls,key::q + ; + const outcome = loadFromDataChecked(std.testing.allocator, data); + switch (outcome) { + .err => |e| try std.testing.expectEqual(LoadError.TabBindingShadowsGlobal, e), + else => return error.ExpectedConflictError, + } +} + +test "loadFromData scoped bindings group by scope" { + const data = + \\#!srfv1 + \\scope::options,action::collapse_all_calls,key::C + \\scope::options,action::collapse_all_puts,key::P + \\scope::history,action::metric_next,key::M + ; + var km = loadFromData(std.testing.allocator, data) orelse return error.ParseFailed; + defer km.deinit(); + try std.testing.expectEqual(@as(usize, 2), km.tab_overrides.len); + // Order matches first-occurrence in input. + try std.testing.expectEqualStrings("options", km.tab_overrides[0].scope); + try std.testing.expectEqual(@as(usize, 2), km.tab_overrides[0].bindings.len); + try std.testing.expectEqualStrings("history", km.tab_overrides[1].scope); + try std.testing.expectEqual(@as(usize, 1), km.tab_overrides[1].bindings.len); +} + +test "loadFromData unknown action emits warning" { + const data = + \\#!srfv1 + \\action::quit,key::q + \\action::not_a_real_action,key::z + ; + var km = loadFromData(std.testing.allocator, data) orelse return error.ParseFailed; + defer km.deinit(); + try std.testing.expectEqual(@as(usize, 1), km.bindings.len); + try std.testing.expectEqual(@as(usize, 1), km.warnings.len); + try std.testing.expect(std.mem.indexOf(u8, km.warnings[0], "not_a_real_action") != null); +} + +// NOTE: there's no `loadFromData malformed key emits warning` test +// here. That path goes through srf's `coerce` → `KeyCombo.srfParse`, +// and srf logs at `log.err` when our srfParse returns an error — +// which the Zig 0.16 test runner treats as a test failure even when +// the test itself passes its asserts. The malformed-key parsing +// path is covered directly by the `parseKeyCombo` tests above. + +test "tabOverridesFor lookup" { + const data = + \\#!srfv1 + \\scope::options,action::collapse_all_calls,key::C + ; + var km = loadFromData(std.testing.allocator, data) orelse return error.ParseFailed; + defer km.deinit(); + try std.testing.expect(km.tabOverridesFor("options") != null); + try std.testing.expect(km.tabOverridesFor("history") == null); } test "defaults returns valid keymap" {