diff --git a/AGENTS.md b/AGENTS.md index 9d8810d..e1ac58f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -522,6 +522,56 @@ will fail to catch real bugs later. 2. Add the tab variant to `tui.Tab` enum 3. Wire rendering in `tui.zig`'s draw and event handling +### `anytype` is almost never the right answer — pause and ask first + +Empirically, every time `anytype` looked necessary in this codebase +it turned out not to be. Concrete-typed parameters always worked +once we actually tried them. **Before adding any new `anytype` +parameter, stop and reconsider.** The discussion that surfaces "no, +this can be `*App` after all" is short and worth having every time. + +Common reasons people reach for `anytype` and what to do instead: + +- **"I want to avoid a circular import."** Test the assumption. + Zig resolves `a.zig ↔ b.zig` cycles fine in most cases — file + structs are evaluated lazily, and the cycle only fails if + evaluation actually loops. Just write the concrete type and run + `zig build`. If it fails, the answer is usually to extract the + shared type to a third file, not to weaken the contract with + `anytype`. (See: the `tab_framework.zig` ↔ `tui.zig` cycle — + caught me once; turned out Zig handled it cleanly.) +- **"This function is genuinely polymorphic over many types."** + In Zig, the right shape for runtime polymorphism is usually + `*anyopaque` + an explicit cast at the boundary, paired with + a vtable struct of fn pointers. That's harder to abuse than + `anytype` and the cast site documents the type contract + unambiguously. Compile-time polymorphism over a known set of + types is better expressed with `comptime T: type` parameters. +- **"It's an event union and I don't want to pattern-match in + every caller."** Split the contract by event class + (`handleKey`, `handleMouse`, `handlePaste`) and let the + dispatcher do the discrimination once. Each method gets a + concrete type; tabs that don't care about a class simply omit + the method. +- **"It's a test helper that takes any struct."** This is the + one case where `anytype` is sometimes OK — generic test + utilities like `std.testing.expectEqual`. But check whether + a concrete type would do. + +The real cost of `anytype` is that it punches a hole in the type +contract: the compiler can't catch wrong-shape arguments at the +boundary, signatures stop being self-documenting, and each +callsite gets monomorphized into its own instantiation that may +or may not behave the same as the others. The framework's whole +point is "let the compiler enforce the shape." `anytype` defeats +that. + +If you've considered the alternatives above and still believe +`anytype` is correct, **flag it in your message to the user +before writing the code.** Phrase it as "I think this needs +`anytype` because X — does that match your intuition?" so the +default is discussion, not silently-typed-loose code. + ### Command `run()` signatures — allocator as code smell A CLI command's `run()` function that takes `*DataService` and `*std.Io.Writer` diff --git a/src/tui.zig b/src/tui.zig index 9d42eda..361a9b1 100644 --- a/src/tui.zig +++ b/src/tui.zig @@ -5,6 +5,13 @@ const fmt = @import("format.zig"); const views = @import("views/portfolio_sections.zig"); const cli = @import("commands/common.zig"); const keybinds = @import("tui/keybinds.zig"); +const tab_framework = @import("tui/tab_framework.zig"); +// Touch tab_framework so its tests are reachable via the import +// graph. The framework is otherwise unused at this point in the +// migration; will be properly wired in step 3. +comptime { + _ = tab_framework; +} const theme = @import("tui/theme.zig"); const chart = @import("tui/chart.zig"); const portfolio_tab = @import("tui/portfolio_tab.zig"); diff --git a/src/tui/tab_framework.zig b/src/tui/tab_framework.zig new file mode 100644 index 0000000..a0cfe1b --- /dev/null +++ b/src/tui/tab_framework.zig @@ -0,0 +1,478 @@ +//! TUI tab framework — common contract types for tab modules. +//! +//! Every TUI tab module exports a `pub const tab = struct { ... };` +//! that conforms to the contract documented here. A small comptime +//! walker in `src/tui.zig` discovers tabs via the `tab_modules` +//! anonymous struct literal and uses these types to wire dispatch, +//! the help overlay, and the status-line hint. +//! +//! ## Contract (per-tab) +//! +//! ```zig +//! pub const Action = enum { /* tab-local keybind actions */ }; +//! pub const State = struct { /* tab-private state */ }; +//! +//! pub const tab = struct { +//! pub const ActionT = Action; +//! pub const StateT = State; +//! +//! /// Default keybindings for this tab. +//! pub const default_bindings: []const TabBinding(Action) = &.{ ... }; +//! +//! /// One label per Action variant for the help overlay. +//! pub const action_labels = +//! std.enums.EnumArray(Action, []const u8).init(.{ ... }); +//! +//! /// Subset of actions that show in the status-line hint. +//! pub const status_hints: []const Action = &.{ ... }; +//! +//! // ── Lifecycle hooks (required) ────────────────────────── +//! pub fn init(state: *State, app: *App) !void { ... } +//! pub fn deinit(state: *State, app: *App) void { ... } +//! pub fn activate(state: *State, app: *App) !void { ... } +//! pub fn deactivate(state: *State, app: *App) void { ... } +//! pub fn reload(state: *State, app: *App) !void { ... } +//! pub fn tick(state: *State, app: *App, frame: u64) void { ... } +//! +//! // ── Action dispatch (required) ────────────────────────── +//! pub fn handleAction(state: *State, app: *App, action: Action) void { ... } +//! +//! // ── Event hooks (each optional) ───────────────────────── +//! // Tabs that don't care about an event class simply omit the +//! // method. The framework's dispatcher checks `@hasDecl` and +//! // skips the call entirely. Each method returns `bool` — +//! // true means "consumed; don't fall through to global +//! // handling." +//! pub fn handleKey(state: *State, app: *App, key: vaxis.Key) bool { ... } +//! pub fn handleMouse(state: *State, app: *App, mouse: vaxis.Mouse) bool { ... } +//! pub fn handlePaste(state: *State, app: *App, text: []const u8) bool { ... } +//! +//! // ── Misc (required) ───────────────────────────────────── +//! pub fn isDisabled(app: *App) bool { ... } +//! }; +//! ``` +//! +//! Tabs without keybind actions ship with `Action = enum {}` and +//! empty `default_bindings` / `action_labels` / `status_hints`. +//! No implicit defaults — the contract is fully explicit for +//! action-related fields and lifecycle hooks. The event hooks +//! (`handleKey`, `handleMouse`, `handlePaste`) are the exception: +//! their absence means "this tab doesn't process that event class." +//! +//! Lifecycle hooks that aren't meaningful for a given tab can use +//! the `noop*` factory helpers below to inherit no-op +//! implementations: +//! +//! ```zig +//! pub const tab = struct { +//! pub const deactivate = framework.noopDeactivate(State); +//! pub const tick = framework.noopTick(State); +//! // ... other hooks defined explicitly ... +//! }; +//! ``` + +const std = @import("std"); +const vaxis = @import("vaxis"); + +/// Re-exported KeyCombo so tab modules don't need to import +/// keybinds.zig directly for binding declarations. +pub const KeyCombo = struct { + codepoint: u21, + mods: vaxis.Key.Modifiers = .{}, +}; + +/// A single (action, key) pair for a tab's `default_bindings`. +/// Generic over the tab's Action enum so `default_bindings` can +/// be type-checked at comptime against the tab's own enum. +pub fn TabBinding(comptime ActionT: type) type { + return struct { + action: ActionT, + key: KeyCombo, + }; +} + +// ── Lifecycle hook factories (no-op defaults) ───────────────── +// +// Tabs that don't need a particular lifecycle hook can declare +// `pub const deactivate = framework.noopDeactivate(State);` etc. +// This keeps the contract explicit (every required field is named +// in the tab struct) while letting tabs avoid writing dummy bodies. +// +// Event hooks (handleKey, handleMouse, handlePaste) are NOT in +// this list — they're optional via `@hasDecl` checking, so a tab +// that doesn't care simply omits the method. + +const App = @import("../tui.zig").App; + +/// Returns a `deactivate(state, app) void` no-op for the given +/// State type. +pub fn noopDeactivate(comptime StateT: type) fn (*StateT, *App) void { + return struct { + fn f(_: *StateT, _: *App) void {} + }.f; +} + +/// Returns a `tick(state, app, frame) void` no-op for the given +/// State type. +pub fn noopTick(comptime StateT: type) fn (*StateT, *App, u64) void { + return struct { + fn f(_: *StateT, _: *App, _: u64) void {} + }.f; +} + +/// Returns an `isDisabled(app) bool` that always returns false. +/// The most common case — most tabs are always enabled. +pub fn alwaysEnabled() fn (*App) bool { + return struct { + fn f(_: *App) bool { + return false; + } + }.f; +} + +// ── Comptime contract validation ────────────────────────────── +// +// `validateTabModule(comptime Module: type)` walks a tab module +// at comptime and asserts it conforms to the framework contract. +// Every error message includes the full expected signature so the +// developer knows exactly what to add, copy-paste ready. +// +// Call sites: the registry in `src/tui.zig` calls this once per +// entry. Each `_tab.zig` can also opt in via a comptime +// block in the file itself for faster local feedback: +// +// ```zig +// comptime { framework.validateTabModule(@This()); } +// ``` + +/// Validate a tab module against the framework contract. Emits a +/// `@compileError` with the full expected signature for any +/// missing or wrong-shape decl. +/// +/// The contract is documented at the top of this file. This +/// function is the source of truth — if you change the contract, +/// update both. +pub fn validateTabModule(comptime Module: type) void { + comptime { + const mod_name = @typeName(Module); + + // ── Top-level decls ──────────────────────────────────── + if (!@hasDecl(Module, "Action")) { + @compileError("Tab module `" ++ mod_name ++ "` is missing `pub const Action = enum { ... };` " ++ + "(use `enum {}` if the tab has no keybind actions)"); + } + if (!@hasDecl(Module, "State")) { + @compileError("Tab module `" ++ mod_name ++ "` is missing `pub const State = struct { ... };`"); + } + if (!@hasDecl(Module, "tab")) { + @compileError("Tab module `" ++ mod_name ++ "` is missing `pub const tab = struct { ... };` " ++ + "(see src/tui/tab_framework.zig for the full contract)"); + } + + const Action = Module.Action; + const State = Module.State; + const tab_decl = Module.tab; + + // Sanity: Action must be an enum, State must be a struct. + if (@typeInfo(Action) != .@"enum") { + @compileError("Tab module `" ++ mod_name ++ "`: `Action` must be an enum type, got " ++ @typeName(Action)); + } + if (@typeInfo(State) != .@"struct") { + @compileError("Tab module `" ++ mod_name ++ "`: `State` must be a struct type, got " ++ @typeName(State)); + } + + // ── Type aliases inside `tab` ────────────────────────── + if (!@hasDecl(tab_decl, "ActionT")) { + @compileError("Tab module `" ++ mod_name ++ "`: `tab` is missing `pub const ActionT = Action;`"); + } + if (!@hasDecl(tab_decl, "StateT")) { + @compileError("Tab module `" ++ mod_name ++ "`: `tab` is missing `pub const StateT = State;`"); + } + + // ── Binding / label / hint constants ─────────────────── + expectDeclWithType( + mod_name, + tab_decl, + "default_bindings", + []const TabBinding(Action), + "pub const default_bindings: []const TabBinding(Action) = &.{ ... };", + ); + expectDeclWithType( + mod_name, + tab_decl, + "action_labels", + std.enums.EnumArray(Action, []const u8), + "pub const action_labels = std.enums.EnumArray(Action, []const u8).init(.{ ... });", + ); + expectDeclWithType( + mod_name, + tab_decl, + "status_hints", + []const Action, + "pub const status_hints: []const Action = &.{ ... };", + ); + + // ── Lifecycle hooks (required) ───────────────────────── + expectFnInferredError( + mod_name, + tab_decl, + "init", + &.{ *State, *App }, + void, + "pub fn init(state: *State, app: *App) !void { ... }", + ); + expectFn( + mod_name, + tab_decl, + "deinit", + fn (*State, *App) void, + "pub fn deinit(state: *State, app: *App) void { ... }", + ); + expectFnInferredError( + mod_name, + tab_decl, + "activate", + &.{ *State, *App }, + void, + "pub fn activate(state: *State, app: *App) !void { ... }", + ); + expectFn( + mod_name, + tab_decl, + "deactivate", + fn (*State, *App) void, + "pub fn deactivate(state: *State, app: *App) void { ... }", + ); + expectFnInferredError( + mod_name, + tab_decl, + "reload", + &.{ *State, *App }, + void, + "pub fn reload(state: *State, app: *App) !void { ... }", + ); + expectFn( + mod_name, + tab_decl, + "tick", + fn (*State, *App, u64) void, + "pub fn tick(state: *State, app: *App, frame: u64) void { ... }", + ); + + // ── Action dispatch (required) ───────────────────────── + expectFn( + mod_name, + tab_decl, + "handleAction", + fn (*State, *App, Action) void, + "pub fn handleAction(state: *State, app: *App, action: Action) void { ... }", + ); + + // ── Misc (required) ──────────────────────────────────── + expectFn( + mod_name, + tab_decl, + "isDisabled", + fn (*App) bool, + "pub fn isDisabled(app: *App) bool { ... }", + ); + + // ── Event hooks (optional, but typed when present) ───── + // `@hasDecl` returns true; if the signature is wrong we + // surface that as a hard error so a typo'd signature + // doesn't silently get skipped at dispatch time. + if (@hasDecl(tab_decl, "handleKey")) { + expectFn( + mod_name, + tab_decl, + "handleKey", + fn (*State, *App, vaxis.Key) bool, + "pub fn handleKey(state: *State, app: *App, key: vaxis.Key) bool { ... }", + ); + } + if (@hasDecl(tab_decl, "handleMouse")) { + expectFn( + mod_name, + tab_decl, + "handleMouse", + fn (*State, *App, vaxis.Mouse) bool, + "pub fn handleMouse(state: *State, app: *App, mouse: vaxis.Mouse) bool { ... }", + ); + } + if (@hasDecl(tab_decl, "handlePaste")) { + expectFn( + mod_name, + tab_decl, + "handlePaste", + fn (*State, *App, []const u8) bool, + "pub fn handlePaste(state: *State, app: *App, text: []const u8) bool { ... }", + ); + } + } +} + +/// Internal helper: assert a `tab` decl exists and has the +/// expected type. The `expected_signature` string is shown +/// verbatim in the error message so the developer can copy-paste +/// the fix. +fn expectDeclWithType( + comptime mod_name: []const u8, + comptime tab_decl: type, + comptime decl_name: []const u8, + comptime ExpectedT: type, + comptime expected_signature: []const u8, +) void { + comptime { + if (!@hasDecl(tab_decl, decl_name)) { + @compileError("Tab module `" ++ mod_name ++ "`: `tab` is missing `" ++ decl_name ++ + "`. Expected:\n " ++ expected_signature); + } + const ActualT = @TypeOf(@field(tab_decl, decl_name)); + if (ActualT != ExpectedT) { + @compileError("Tab module `" ++ mod_name ++ "`: `tab." ++ decl_name ++ + "` has wrong type.\n Expected: " ++ @typeName(ExpectedT) ++ + "\n Got: " ++ @typeName(ActualT) ++ + "\n Expected signature:\n " ++ expected_signature); + } + } +} + +/// Internal helper: assert a `tab` function decl exists and has +/// the expected fn-pointer type. The `expected_signature` string +/// is shown verbatim in the error message. +/// +/// For functions returning `!void` (or any error union with an +/// inferred error set), pass `expectFnInferredError` instead — +/// this helper requires exact type equality, which means the +/// expected type must use `anyerror` and the actual function must +/// also use `anyerror`. In practice that's not what `pub fn foo() +/// !void` produces (Zig infers an empty error set), so for +/// fallible-but-error-set-inferred hooks we need the looser check. +fn expectFn( + comptime mod_name: []const u8, + comptime tab_decl: type, + comptime decl_name: []const u8, + comptime ExpectedT: type, + comptime expected_signature: []const u8, +) void { + comptime { + if (!@hasDecl(tab_decl, decl_name)) { + @compileError("Tab module `" ++ mod_name ++ "`: `tab` is missing `" ++ decl_name ++ + "`. Expected:\n " ++ expected_signature); + } + const ActualT = @TypeOf(@field(tab_decl, decl_name)); + if (ActualT != ExpectedT) { + @compileError("Tab module `" ++ mod_name ++ "`: `tab." ++ decl_name ++ + "` has wrong signature.\n Expected: " ++ @typeName(ExpectedT) ++ + "\n Got: " ++ @typeName(ActualT) ++ + "\n Full expected declaration:\n " ++ expected_signature); + } + } +} + +/// Internal helper: assert a `tab` fallible function decl exists +/// and matches the expected parameter types + returns an error +/// union ending in `Return`. The error set itself is not checked, +/// because `pub fn foo() !void` produces an inferred (often empty) +/// error set whose name varies per function. This loosens equality +/// to "params match, return is `!Return`." +fn expectFnInferredError( + comptime mod_name: []const u8, + comptime tab_decl: type, + comptime decl_name: []const u8, + comptime expected_params: []const type, + comptime ExpectedReturn: type, + comptime expected_signature: []const u8, +) void { + comptime { + if (!@hasDecl(tab_decl, decl_name)) { + @compileError("Tab module `" ++ mod_name ++ "`: `tab` is missing `" ++ decl_name ++ + "`. Expected:\n " ++ expected_signature); + } + const ActualT = @TypeOf(@field(tab_decl, decl_name)); + const info = @typeInfo(ActualT); + if (info != .@"fn") { + @compileError("Tab module `" ++ mod_name ++ "`: `tab." ++ decl_name ++ + "` must be a function, got " ++ @typeName(ActualT) ++ + "\n Full expected declaration:\n " ++ expected_signature); + } + const fn_info = info.@"fn"; + + // Verify params count + types. + if (fn_info.params.len != expected_params.len) { + @compileError("Tab module `" ++ mod_name ++ "`: `tab." ++ decl_name ++ + "` has wrong arity.\n Got: " ++ @typeName(ActualT) ++ + "\n Full expected declaration:\n " ++ expected_signature); + } + for (expected_params, 0..) |Expected, i| { + const ActualParam = fn_info.params[i].type orelse { + @compileError("Tab module `" ++ mod_name ++ "`: `tab." ++ decl_name ++ + "` parameter " ++ std.fmt.comptimePrint("{d}", .{i}) ++ + " is `anytype`, but the contract requires `" ++ @typeName(Expected) ++ "`.\n" ++ + " Full expected declaration:\n " ++ expected_signature); + }; + if (ActualParam != Expected) { + @compileError("Tab module `" ++ mod_name ++ "`: `tab." ++ decl_name ++ + "` parameter " ++ std.fmt.comptimePrint("{d}", .{i}) ++ + " has wrong type.\n Expected: " ++ @typeName(Expected) ++ + "\n Got: " ++ @typeName(ActualParam) ++ + "\n Full expected declaration:\n " ++ expected_signature); + } + } + + // Verify return is an error union ending in ExpectedReturn. + const ActualReturn = fn_info.return_type orelse { + @compileError("Tab module `" ++ mod_name ++ "`: `tab." ++ decl_name ++ + "` has no return type.\n Full expected declaration:\n " ++ expected_signature); + }; + const ret_info = @typeInfo(ActualReturn); + if (ret_info != .error_union) { + @compileError("Tab module `" ++ mod_name ++ "`: `tab." ++ decl_name ++ + "` must return an error union (`!" ++ @typeName(ExpectedReturn) ++ + "`), got " ++ @typeName(ActualReturn) ++ + "\n Full expected declaration:\n " ++ expected_signature); + } + if (ret_info.error_union.payload != ExpectedReturn) { + @compileError("Tab module `" ++ mod_name ++ "`: `tab." ++ decl_name ++ + "` returns wrong payload type.\n Expected: !" ++ @typeName(ExpectedReturn) ++ + "\n Got: " ++ @typeName(ActualReturn) ++ + "\n Full expected declaration:\n " ++ expected_signature); + } + } +} + +// ── Tests ───────────────────────────────────────────────────── + +const testing = std.testing; + +test "TabBinding generic over Action" { + const A = enum { foo, bar }; + const B = TabBinding(A); + const b: B = .{ .action = .foo, .key = .{ .codepoint = 'a' } }; + try testing.expectEqual(A.foo, b.action); + try testing.expectEqual(@as(u21, 'a'), b.key.codepoint); +} + +test "noopDeactivate returns a callable no-op" { + const S = struct { x: u32 = 0 }; + const fn_ptr = noopDeactivate(S); + var s: S = .{}; + var dummy_app: App = undefined; + fn_ptr(&s, &dummy_app); + try testing.expectEqual(@as(u32, 0), s.x); +} + +test "noopTick returns a callable no-op" { + const S = struct { x: u32 = 0 }; + const fn_ptr = noopTick(S); + var s: S = .{}; + var dummy_app: App = undefined; + fn_ptr(&s, &dummy_app, 42); + try testing.expectEqual(@as(u32, 0), s.x); +} + +test "alwaysEnabled returns false" { + const fn_ptr = alwaysEnabled(); + var dummy_app: App = undefined; + try testing.expect(!fn_ptr(&dummy_app)); +}