From 86106d291c6f4cc692df237540cb37c8326ed1ef Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Sun, 17 May 2026 17:50:12 -0700 Subject: [PATCH] add new TimeRange struct for handling all time specs in cli --- src/commands/TimeRange.zig | 556 +++++++++++++++++++++++++++++++++++++ src/main.zig | 11 +- 2 files changed, 563 insertions(+), 4 deletions(-) create mode 100644 src/commands/TimeRange.zig diff --git a/src/commands/TimeRange.zig b/src/commands/TimeRange.zig new file mode 100644 index 0000000..7eba9c8 --- /dev/null +++ b/src/commands/TimeRange.zig @@ -0,0 +1,556 @@ +//! Unified time-range flag parser for CLI commands. +//! +//! Several commands accept overlapping but slightly different +//! time-range inputs: +//! +//! - `contributions` accepts `--since` / `--until` (dates) and +//! `--commit-before` / `--commit-after` (commit specs). +//! - `compare` accepts `--snapshot-before` / `--snapshot-after` +//! plus `--commit-before` / `--commit-after`, with positional +//! dates as a happy-path shortcut. +//! - `history --since` / `--until` (range bounds, no commit specs). +//! +//! This module collects the shared parsing logic into one place so: +//! +//! 1. Each consumer declares which flags it accepts via `ParseSpec` +//! and gets a uniform parser + error-message format. +//! 2. Conflict detection (`--since` AND `--commit-before` both +//! describing the before side) is a single helper instead of +//! duplicated per-command. +//! 3. The grammar for what each endpoint accepts (`live` / date / +//! commit-spec) is enumerated once in `Endpoint`. +//! +//! `projections --as-of` / `--vs` are deliberately NOT handled here. +//! Their (now, then) re-roling logic is too command-specific to live +//! in a generic parser; projections parses them locally. +//! +//! ## File-as-struct shape +//! +//! This file IS a `TimeRange` value. Top-level fields are the struct's +//! fields; nested types live in `pub const ...`; methods are top-level +//! `pub fn`. Importers say `const TimeRange = @import("TimeRange.zig");` +//! and instances are `var range: TimeRange = ...`. +//! +//! ## Positional dates +//! +//! `compare`'s positional date arguments are NOT handled here — +//! compare's `parseArgs` consumes them, then constructs a TimeRange +//! from the resolved values. Keeping positionals out of `ParseSpec` +//! keeps the parser focused on flag grammar. + +const std = @import("std"); +const zfin = @import("../root.zig"); +const git = @import("../git.zig"); +const cli = @import("common.zig"); + +/// Self-reference for clarity inside method bodies and return types. +/// Equivalent to `@This()` but more readable at signatures like +/// `ParseResult.range: TimeRange`. +const TimeRange = @This(); + +// ── Struct fields ───────────────────────────────────────────── + +/// The before endpoint (typically the older side of the comparison). +/// `null` means the user did not supply a before-side flag — the +/// caller decides what default to apply (e.g. `compare` falls back +/// to the positional date; `contributions` falls back to `HEAD~1`). +before: ?Endpoint = null, + +/// The after endpoint (typically the newer side). +/// `null` means no after-side flag was supplied. +after: ?Endpoint = null, + +// ── Nested types ────────────────────────────────────────────── + +/// A single resolved endpoint. Each variant is a different +/// flavour of "where on the timeline does this side land?": +/// +/// - `date`: a concrete calendar date (from YYYY-MM-DD or relative +/// shortcuts like `1W` / `1M` / `1Q` / `1Y`). +/// - `commit_spec`: a git commit reference (HEAD / HEAD~N / SHA / +/// working-copy / date-at-or-before). +/// - `live`: the current portfolio (no snapshot lookup). Only the +/// after endpoint accepts this; `--snapshot-before` / `--since` +/// reject it explicitly. +pub const Endpoint = union(enum) { + date: zfin.Date, + commit_spec: git.CommitSpec, + live, +}; + +/// Which flags a parser invocation should recognize. Each command +/// fills this in based on its grammar; flags not listed are rejected. +/// +/// Only the flag NAMES are configured here — the underlying value +/// grammar (date / commit-spec / live) is fixed per flag because +/// each flag's user-facing meaning is fixed. +/// +/// `--as-of` and `--vs` are deliberately NOT here. Projections is +/// the only command using them and the (now, then) re-roling logic +/// is too command-specific to live in a generic parser. Projections +/// parses them locally. +pub const ParseSpec = struct { + accept_since: bool = false, + accept_until: bool = false, + accept_commit_before: bool = false, + accept_commit_after: bool = false, + accept_snapshot_before: bool = false, + accept_snapshot_after: bool = false, +}; + +/// Conflict-detection rules for `checkConflicts`. Different commands +/// have different expectations about what endpoint combinations make +/// sense: +/// +/// - `none`: no conflict checks beyond what `parse` itself rejects +/// (which is just "two flags on the same axis"). +/// - `reject_live_anywhere`: neither endpoint may be `.live` — +/// `contributions` uses this because there's no meaningful +/// "live contributions diff" against an unstaged working tree +/// that wasn't anchored to a commit. +/// - `reject_live_on_both`: at least one endpoint must be a +/// concrete date or commit. `compare --snapshot-before live +/// --snapshot-after live` is meaningless. +pub const ConflictRule = enum { none, reject_live_anywhere, reject_live_on_both }; + +/// Errors `parse` may surface. Every variant is a user-input +/// problem that maps to a stderr message + non-zero exit. +pub const ParseError = error{ + /// A flag's value was missing (e.g. `--since` at end of args). + MissingValue, + /// A flag's value couldn't be parsed in the grammar that flag + /// expects. + InvalidValue, + /// Two flags supplied for the same endpoint axis (e.g. both + /// `--since` and `--commit-before`). + DuplicateEndpoint, + /// `--commit-before working` (working-copy on the before side + /// is meaningless). + WorkingCopyOnBeforeSide, + /// A flag describes the same endpoint axis twice (e.g. + /// `--since 1W --since 2W`). + RepeatedFlag, + /// `--since`/`--until`/`--snapshot-before`/`--as-of` got `live`, + /// which isn't allowed for that flag. + LiveNotAllowed, + /// Allocator failed. + OutOfMemory, +}; + +/// Errors `checkConflicts` may surface. +pub const ConflictError = error{ + LiveOnBothEndpoints, + LiveNotAllowed, +}; + +// ── Top-level methods ───────────────────────────────────────── + +/// Result of `parse`: the populated TimeRange plus a slice of indices +/// into `cmd_args` that the parser consumed (each index is either a +/// flag itself or its value). The caller can use the index set to +/// drop those tokens from its own iteration when parsing other flags. +pub const ParseResult = struct { + range: TimeRange, + consumed: []const usize, +}; + +/// Parse the time-range flags out of `cmd_args` according to `spec`. +/// +/// `today` is the reference date for relative shortcuts (`1W`, `1M`, +/// etc). Pass the same `today` you pass to other parsers so +/// invocations are internally consistent. +/// +/// Allocation: `consumed` comes from `allocator`. Caller frees it +/// when done. Pass an arena and forget about it. +pub fn parse( + io: std.Io, + allocator: std.mem.Allocator, + today: zfin.Date, + cmd_args: []const []const u8, + spec: ParseSpec, +) ParseError!ParseResult { + var range: TimeRange = .{}; + var consumed: std.ArrayList(usize) = .empty; + errdefer consumed.deinit(allocator); + + // Track which axis each flag sets so we can detect duplicates + // and conflicts. + var before_set_by: ?[]const u8 = null; + var after_set_by: ?[]const u8 = null; + + var i: usize = 0; + while (i < cmd_args.len) : (i += 1) { + const a = cmd_args[i]; + + // Match against accepted flag names. + const flag_match = matchFlag(a, spec); + if (flag_match == null) continue; + const flag = flag_match.?; + + // Take the next arg as value. + if (i + 1 >= cmd_args.len) { + try emitMissingValue(io, a); + return error.MissingValue; + } + const value = cmd_args[i + 1]; + + // Resolve the endpoint per flag's grammar. + const endpoint = resolveEndpoint(io, today, a, flag, value) catch |err| return err; + + // Set the right side, rejecting duplicates. + switch (flag.side) { + .before => { + if (before_set_by) |prev| { + try emitDuplicateEndpoint(io, prev, a, "before"); + return if (std.mem.eql(u8, prev, a)) error.RepeatedFlag else error.DuplicateEndpoint; + } + range.before = endpoint; + before_set_by = a; + }, + .after => { + if (after_set_by) |prev| { + try emitDuplicateEndpoint(io, prev, a, "after"); + return if (std.mem.eql(u8, prev, a)) error.RepeatedFlag else error.DuplicateEndpoint; + } + range.after = endpoint; + after_set_by = a; + }, + } + + try consumed.append(allocator, i); + try consumed.append(allocator, i + 1); + i += 1; // skip value on next iteration + } + + return .{ .range = range, .consumed = try consumed.toOwnedSlice(allocator) }; +} + +/// Apply a conflict rule to a parsed range. Most consumers call this +/// immediately after `parse`. Errors emit a stderr message before +/// returning. +pub fn checkConflicts(io: std.Io, range: @This(), rule: ConflictRule) ConflictError!void { + switch (rule) { + .none => {}, + .reject_live_anywhere => { + if (endpointIsLive(range.before) or endpointIsLive(range.after)) { + cli.stderrPrint(io, "Error: this command does not accept 'live' as an endpoint.\n") catch {}; + return error.LiveNotAllowed; + } + }, + .reject_live_on_both => { + if (endpointIsLive(range.before) and endpointIsLive(range.after)) { + cli.stderrPrint(io, "Error: cannot compare 'live' against 'live' — at least one endpoint must be a concrete date or commit.\n") catch {}; + return error.LiveOnBothEndpoints; + } + }, + } +} + +// ── Internal helpers ────────────────────────────────────────── + +const FlagInfo = struct { + side: enum { before, after }, + grammar: enum { + /// YYYY-MM-DD or relative; rejects 'live'. + date_only, + /// YYYY-MM-DD or relative; accepts 'live' (only used by + /// after-side / single-sided flags). + date_or_live, + /// Full CommitSpec grammar (date / relative / HEAD / SHA / + /// working-copy). + commit_spec, + }, +}; + +fn matchFlag(arg: []const u8, spec: ParseSpec) ?FlagInfo { + if (spec.accept_since and std.mem.eql(u8, arg, "--since")) { + return .{ .side = .before, .grammar = .date_only }; + } + if (spec.accept_until and std.mem.eql(u8, arg, "--until")) { + return .{ .side = .after, .grammar = .date_only }; + } + if (spec.accept_commit_before and std.mem.eql(u8, arg, "--commit-before")) { + return .{ .side = .before, .grammar = .commit_spec }; + } + if (spec.accept_commit_after and std.mem.eql(u8, arg, "--commit-after")) { + return .{ .side = .after, .grammar = .commit_spec }; + } + if (spec.accept_snapshot_before and std.mem.eql(u8, arg, "--snapshot-before")) { + return .{ .side = .before, .grammar = .date_only }; + } + if (spec.accept_snapshot_after and std.mem.eql(u8, arg, "--snapshot-after")) { + return .{ .side = .after, .grammar = .date_or_live }; + } + return null; +} + +fn resolveEndpoint( + io: std.Io, + today: zfin.Date, + flag: []const u8, + info: FlagInfo, + value: []const u8, +) ParseError!Endpoint { + switch (info.grammar) { + .date_only => { + const parsed = cli.parseAsOfDate(value, today) catch |err| { + var buf: [256]u8 = undefined; + const msg = cli.fmtAsOfParseError(&buf, value, err); + cli.stderrPrint(io, "Error: ") catch {}; + cli.stderrPrint(io, flag) catch {}; + cli.stderrPrint(io, ": ") catch {}; + cli.stderrPrint(io, msg) catch {}; + cli.stderrPrint(io, "\n") catch {}; + return error.InvalidValue; + }; + const date = parsed orelse { + cli.stderrPrint(io, "Error: ") catch {}; + cli.stderrPrint(io, flag) catch {}; + cli.stderrPrint(io, " does not accept 'live'. Use an explicit date or relative offset.\n") catch {}; + return error.LiveNotAllowed; + }; + return .{ .date = date }; + }, + .date_or_live => { + const parsed = cli.parseAsOfDate(value, today) catch |err| { + var buf: [256]u8 = undefined; + const msg = cli.fmtAsOfParseError(&buf, value, err); + cli.stderrPrint(io, "Error: ") catch {}; + cli.stderrPrint(io, flag) catch {}; + cli.stderrPrint(io, ": ") catch {}; + cli.stderrPrint(io, msg) catch {}; + cli.stderrPrint(io, "\n") catch {}; + return error.InvalidValue; + }; + if (parsed) |d| return .{ .date = d }; + return .live; + }, + .commit_spec => { + const spec = cli.parseCommitSpec(value, today) catch |err| { + var buf: [256]u8 = undefined; + const msg = cli.fmtCommitSpecError(&buf, value, err); + cli.stderrPrint(io, "Error: ") catch {}; + cli.stderrPrint(io, flag) catch {}; + cli.stderrPrint(io, ": ") catch {}; + cli.stderrPrint(io, msg) catch {}; + cli.stderrPrint(io, "\n") catch {}; + return error.InvalidValue; + }; + // `working` on the before side is meaningless (you can't + // diff the working copy against itself). Reject early. + if (spec == .working_copy and std.mem.eql(u8, flag, "--commit-before")) { + cli.stderrPrint(io, "Error: --commit-before cannot be `working` — diffing the working copy against itself is meaningless.\n") catch {}; + return error.WorkingCopyOnBeforeSide; + } + return .{ .commit_spec = spec }; + }, + } +} + +fn endpointIsLive(ep: ?Endpoint) bool { + if (ep) |e| return e == .live; + return false; +} + +fn emitMissingValue(io: std.Io, flag: []const u8) ParseError!void { + cli.stderrPrint(io, "Error: ") catch {}; + cli.stderrPrint(io, flag) catch {}; + cli.stderrPrint(io, " requires a value.\n") catch {}; +} + +fn emitDuplicateEndpoint(io: std.Io, prev: []const u8, current: []const u8, side: []const u8) ParseError!void { + cli.stderrPrint(io, "Error: ") catch {}; + cli.stderrPrint(io, prev) catch {}; + cli.stderrPrint(io, " and ") catch {}; + cli.stderrPrint(io, current) catch {}; + cli.stderrPrint(io, " both specify the ") catch {}; + cli.stderrPrint(io, side) catch {}; + cli.stderrPrint(io, " side. Pick one.\n") catch {}; +} + +// ── Tests ───────────────────────────────────────────────────── + +const testing = std.testing; + +const test_today = zfin.Date.fromYmd(2026, 5, 9); + +test "parse: empty args produces empty range" { + const r = try parse(testing.io, testing.allocator, test_today, &.{}, .{}); + defer testing.allocator.free(r.consumed); + try testing.expect(r.range.before == null); + try testing.expect(r.range.after == null); + try testing.expectEqual(@as(usize, 0), r.consumed.len); +} + +test "parse: --since populates before with date" { + const args = [_][]const u8{ "--since", "2026-04-01" }; + const r = try parse(testing.io, testing.allocator, test_today, &args, .{ .accept_since = true }); + defer testing.allocator.free(r.consumed); + try testing.expect(r.range.before != null); + switch (r.range.before.?) { + .date => |d| try testing.expect(d.eql(zfin.Date.fromYmd(2026, 4, 1))), + else => try testing.expect(false), + } + try testing.expect(r.range.after == null); + try testing.expectEqual(@as(usize, 2), r.consumed.len); +} + +test "parse: --since with relative shortcut" { + const args = [_][]const u8{ "--since", "1W" }; + const r = try parse(testing.io, testing.allocator, test_today, &args, .{ .accept_since = true }); + defer testing.allocator.free(r.consumed); + switch (r.range.before.?) { + .date => |d| try testing.expect(d.eql(zfin.Date.fromYmd(2026, 5, 2))), + else => try testing.expect(false), + } +} + +test "parse: --since rejects 'live'" { + const args = [_][]const u8{ "--since", "live" }; + const r = parse(testing.io, testing.allocator, test_today, &args, .{ .accept_since = true }); + try testing.expectError(error.LiveNotAllowed, r); +} + +test "parse: --until populates after" { + const args = [_][]const u8{ "--until", "2026-04-01" }; + const r = try parse(testing.io, testing.allocator, test_today, &args, .{ .accept_until = true }); + defer testing.allocator.free(r.consumed); + try testing.expect(r.range.before == null); + switch (r.range.after.?) { + .date => |d| try testing.expect(d.eql(zfin.Date.fromYmd(2026, 4, 1))), + else => try testing.expect(false), + } +} + +test "parse: --since and --until together" { + const args = [_][]const u8{ "--since", "2026-04-01", "--until", "2026-05-01" }; + const r = try parse(testing.io, testing.allocator, test_today, &args, .{ .accept_since = true, .accept_until = true }); + defer testing.allocator.free(r.consumed); + try testing.expect(r.range.before != null); + try testing.expect(r.range.after != null); + try testing.expectEqual(@as(usize, 4), r.consumed.len); +} + +test "parse: --commit-before with HEAD" { + const args = [_][]const u8{ "--commit-before", "HEAD" }; + const r = try parse(testing.io, testing.allocator, test_today, &args, .{ .accept_commit_before = true }); + defer testing.allocator.free(r.consumed); + switch (r.range.before.?) { + .commit_spec => |spec| switch (spec) { + .git_ref => |ref| try testing.expectEqualStrings("HEAD", ref), + else => try testing.expect(false), + }, + else => try testing.expect(false), + } +} + +test "parse: --commit-before rejects 'working'" { + const args = [_][]const u8{ "--commit-before", "working" }; + const r = parse(testing.io, testing.allocator, test_today, &args, .{ .accept_commit_before = true }); + try testing.expectError(error.WorkingCopyOnBeforeSide, r); +} + +test "parse: --commit-after accepts 'working'" { + const args = [_][]const u8{ "--commit-after", "working" }; + const r = try parse(testing.io, testing.allocator, test_today, &args, .{ .accept_commit_after = true }); + defer testing.allocator.free(r.consumed); + switch (r.range.after.?) { + .commit_spec => |spec| try testing.expect(spec == .working_copy), + else => try testing.expect(false), + } +} + +test "parse: --since and --commit-before conflict (same axis)" { + const args = [_][]const u8{ "--since", "1W", "--commit-before", "HEAD" }; + const r = parse(testing.io, testing.allocator, test_today, &args, .{ .accept_since = true, .accept_commit_before = true }); + try testing.expectError(error.DuplicateEndpoint, r); +} + +test "parse: repeated --since rejected" { + const args = [_][]const u8{ "--since", "1W", "--since", "2W" }; + const r = parse(testing.io, testing.allocator, test_today, &args, .{ .accept_since = true }); + try testing.expectError(error.RepeatedFlag, r); +} + +test "parse: --snapshot-after accepts 'live'" { + const args = [_][]const u8{ "--snapshot-after", "live" }; + const r = try parse(testing.io, testing.allocator, test_today, &args, .{ .accept_snapshot_after = true }); + defer testing.allocator.free(r.consumed); + try testing.expect(r.range.after.? == .live); +} + +test "parse: --snapshot-before rejects 'live'" { + const args = [_][]const u8{ "--snapshot-before", "live" }; + const r = parse(testing.io, testing.allocator, test_today, &args, .{ .accept_snapshot_before = true }); + try testing.expectError(error.LiveNotAllowed, r); +} + +test "parse: missing value errors" { + const args = [_][]const u8{"--since"}; + const r = parse(testing.io, testing.allocator, test_today, &args, .{ .accept_since = true }); + try testing.expectError(error.MissingValue, r); +} + +test "parse: invalid date value errors" { + const args = [_][]const u8{ "--since", "not-a-date" }; + const r = parse(testing.io, testing.allocator, test_today, &args, .{ .accept_since = true }); + try testing.expectError(error.InvalidValue, r); +} + +test "parse: unknown flag is silently ignored (caller handles other flags)" { + // Confirms parse() doesn't mis-consume args it wasn't told about. + const args = [_][]const u8{ "--projections", "--since", "1W", "--no-events" }; + const r = try parse(testing.io, testing.allocator, test_today, &args, .{ .accept_since = true }); + defer testing.allocator.free(r.consumed); + try testing.expect(r.range.before != null); + // Consumed only the --since flag and its value (indices 1 and 2). + try testing.expectEqual(@as(usize, 2), r.consumed.len); + try testing.expectEqual(@as(usize, 1), r.consumed[0]); + try testing.expectEqual(@as(usize, 2), r.consumed[1]); +} + +test "parse: flag accepted only when spec opts in" { + // --since is in args but spec does NOT accept it → not consumed. + const args = [_][]const u8{ "--since", "1W" }; + const r = try parse(testing.io, testing.allocator, test_today, &args, .{ .accept_until = true }); + defer testing.allocator.free(r.consumed); + try testing.expect(r.range.before == null); + try testing.expect(r.range.after == null); + try testing.expectEqual(@as(usize, 0), r.consumed.len); +} + +test "checkConflicts: none always passes" { + const r: TimeRange = .{ .before = .live, .after = .live }; + try checkConflicts(testing.io, r, .none); +} + +test "checkConflicts: reject_live_anywhere catches before-side live" { + const r: TimeRange = .{ .before = .live }; + try testing.expectError(error.LiveNotAllowed, checkConflicts(testing.io, r, .reject_live_anywhere)); +} + +test "checkConflicts: reject_live_anywhere catches after-side live" { + const r: TimeRange = .{ .after = .live }; + try testing.expectError(error.LiveNotAllowed, checkConflicts(testing.io, r, .reject_live_anywhere)); +} + +test "checkConflicts: reject_live_anywhere passes for date endpoints" { + const r: TimeRange = .{ + .before = .{ .date = zfin.Date.fromYmd(2026, 4, 1) }, + .after = .{ .date = zfin.Date.fromYmd(2026, 5, 1) }, + }; + try checkConflicts(testing.io, r, .reject_live_anywhere); +} + +test "checkConflicts: reject_live_on_both catches double-live" { + const r: TimeRange = .{ .before = .live, .after = .live }; + try testing.expectError(error.LiveOnBothEndpoints, checkConflicts(testing.io, r, .reject_live_on_both)); +} + +test "checkConflicts: reject_live_on_both allows one-sided live" { + const r: TimeRange = .{ + .before = .{ .date = zfin.Date.fromYmd(2026, 4, 1) }, + .after = .live, + }; + try checkConflicts(testing.io, r, .reject_live_on_both); +} diff --git a/src/main.zig b/src/main.zig index f63667d..a95d16b 100644 --- a/src/main.zig +++ b/src/main.zig @@ -909,11 +909,14 @@ test "parseGlobals: subcommand-local flag NOT consumed as global" { test { std.testing.refAllDecls(@This()); - // TEMPORARY: force test discovery for the command framework - // module. Nothing in main.zig sema-touches framework.zig yet, so - // its `test` blocks would otherwise be skipped. Remove this line + // TEMPORARY: force test discovery for command-framework support + // modules. Nothing in main.zig sema-touches them yet, so their + // `test` blocks would otherwise be skipped. Remove these lines // once `runCli` references the framework directly (via the // comptime command-registry walk that replaces the legacy - // if-else dispatch chain). + // if-else dispatch chain) — at that point both TimeRange and + // framework.zig are reachable through the registry's command + // imports. _ = @import("commands/framework.zig"); + _ = @import("commands/TimeRange.zig"); }