make command flag parsing more strict
This commit is contained in:
parent
a875fc3b32
commit
bd0483b26e
GPG key ID:
A7B62D657EF764F8
9 changed files with 479 additions and 114 deletions
22
TODO.md
22
TODO.md
|
|
@ -597,28 +597,6 @@ likely culprits:
|
|||
No reproducer yet. When the user has a more specific lead
|
||||
(which tab, which interaction), this entry should narrow.
|
||||
|
||||
## CLI dispatch / arg-parsing bugs (found May 2026)
|
||||
|
||||
Found during a post-framework-refactor sanity check of all 20
|
||||
commands plus interactive. The framework dispatch itself is
|
||||
working correctly; these are gaps in command-level behavior or in
|
||||
the global `--refresh-data` flag's coverage.
|
||||
|
||||
### `zfin interactive --default-keys`/`--default-theme` swallow trailing flags — priority LOW
|
||||
|
||||
`zfin interactive --default-keys --bogus-flag` is silently
|
||||
accepted: `--default-keys` prints its output and `return`s from
|
||||
`tui.run` before the rest of the args are parsed. The flag
|
||||
parser itself now rejects unknown flags (`error.InvalidArgs`
|
||||
+ exit 1), but only when control reaches the parsing loop —
|
||||
which it doesn't if `--default-keys` or `--default-theme`
|
||||
short-circuits first.
|
||||
|
||||
Fix: validate the entire arg list before honoring the
|
||||
print-and-exit flags, or restructure so the parser runs first
|
||||
and the print-and-exit flags fire from inside the loop after
|
||||
all args have been validated.
|
||||
|
||||
### `etf <SYMBOL>` warns `failed to serialize ETF profile: WriteFailed` — priority LOW
|
||||
|
||||
Every `zfin etf VTI` invocation prints
|
||||
|
|
|
|||
|
|
@ -2002,26 +2002,41 @@ pub const meta: framework.Meta = .{
|
|||
\\
|
||||
,
|
||||
.uppercase_first_arg = false,
|
||||
.user_errors = error{ UnexpectedArg, EmptyFile, NoAccountsFound, UnexpectedHeader },
|
||||
.user_errors = error{ UnexpectedArg, EmptyFile, NoAccountsFound, UnexpectedHeader, MissingFlagValue },
|
||||
};
|
||||
|
||||
pub fn parseArgs(_: *framework.RunCtx, cmd_args: []const []const u8) !ParsedArgs {
|
||||
pub fn parseArgs(ctx: *framework.RunCtx, cmd_args: []const []const u8) !ParsedArgs {
|
||||
var parsed: ParsedArgs = .{};
|
||||
var i: usize = 0;
|
||||
while (i < cmd_args.len) : (i += 1) {
|
||||
if (std.mem.eql(u8, cmd_args[i], "--fidelity") and i + 1 < cmd_args.len) {
|
||||
i += 1;
|
||||
parsed.fidelity_csv = cmd_args[i];
|
||||
} else if (std.mem.eql(u8, cmd_args[i], "--schwab") and i + 1 < cmd_args.len) {
|
||||
i += 1;
|
||||
parsed.schwab_csv = cmd_args[i];
|
||||
} else if (std.mem.eql(u8, cmd_args[i], "--schwab-summary")) {
|
||||
const a = cmd_args[i];
|
||||
if (std.mem.eql(u8, a, "--fidelity")) {
|
||||
parsed.fidelity_csv = try cli.requireFlagValue(ctx.io, cmd_args, &i, a);
|
||||
} else if (std.mem.eql(u8, a, "--schwab")) {
|
||||
parsed.schwab_csv = try cli.requireFlagValue(ctx.io, cmd_args, &i, a);
|
||||
} else if (std.mem.eql(u8, a, "--schwab-summary")) {
|
||||
parsed.schwab_summary = true;
|
||||
} else if (std.mem.eql(u8, cmd_args[i], "--verbose")) {
|
||||
} else if (std.mem.eql(u8, a, "--verbose")) {
|
||||
parsed.verbose = true;
|
||||
} else if (std.mem.eql(u8, cmd_args[i], "--stale-days") and i + 1 < cmd_args.len) {
|
||||
i += 1;
|
||||
parsed.stale_days = std.fmt.parseInt(u32, cmd_args[i], 10) catch default_stale_days;
|
||||
} else if (std.mem.eql(u8, a, "--stale-days")) {
|
||||
const v = try cli.requireFlagValue(ctx.io, cmd_args, &i, a);
|
||||
// Reject a non-integer value loudly instead of silently
|
||||
// falling back to the default; a typo'd threshold that
|
||||
// silently reverts to 3 is a foot-gun.
|
||||
parsed.stale_days = std.fmt.parseInt(u32, v, 10) catch {
|
||||
cli.stderrPrint(ctx.io, "Error: --stale-days requires a non-negative integer, got: ");
|
||||
cli.stderrPrint(ctx.io, v);
|
||||
cli.stderrPrint(ctx.io, "\n");
|
||||
return error.UnexpectedArg;
|
||||
};
|
||||
} else {
|
||||
// Unknown flag or stray positional: reject explicitly
|
||||
// rather than silently ignoring it (which previously let
|
||||
// `audit --bogus` run flagless hygiene mode with no error).
|
||||
cli.stderrPrint(ctx.io, "Error: unknown argument to 'audit': ");
|
||||
cli.stderrPrint(ctx.io, a);
|
||||
cli.stderrPrint(ctx.io, "\nRun 'zfin audit --help' for usage.\n");
|
||||
return error.UnexpectedArg;
|
||||
}
|
||||
}
|
||||
return parsed;
|
||||
|
|
@ -2233,6 +2248,58 @@ test "parseArgs: --stale-days parses integer" {
|
|||
try std.testing.expectEqual(@as(u32, 5), parsed.stale_days);
|
||||
}
|
||||
|
||||
test "parseArgs: unknown flag is rejected (not silently ignored)" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = std.testing.io;
|
||||
const args = [_][]const u8{"--bogus"};
|
||||
try std.testing.expectError(error.UnexpectedArg, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: stray positional is rejected" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = std.testing.io;
|
||||
const args = [_][]const u8{"AAPL"};
|
||||
try std.testing.expectError(error.UnexpectedArg, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --fidelity without a value is rejected (no silent mode switch)" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = std.testing.io;
|
||||
const args = [_][]const u8{"--fidelity"};
|
||||
try std.testing.expectError(error.MissingFlagValue, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --fidelity followed by a flag does not swallow the flag" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = std.testing.io;
|
||||
const args = [_][]const u8{ "--fidelity", "--verbose" };
|
||||
try std.testing.expectError(error.MissingFlagValue, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --schwab without a value is rejected" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = std.testing.io;
|
||||
const args = [_][]const u8{"--schwab"};
|
||||
try std.testing.expectError(error.MissingFlagValue, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --stale-days with a non-integer value is rejected (not a silent default)" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = std.testing.io;
|
||||
const args = [_][]const u8{ "--stale-days", "abc" };
|
||||
try std.testing.expectError(error.UnexpectedArg, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: combined valid flags parse together" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = std.testing.io;
|
||||
const args = [_][]const u8{ "--fidelity", "/tmp/fid.csv", "--verbose", "--stale-days", "7" };
|
||||
const parsed = try parseArgs(&ctx, &args);
|
||||
try std.testing.expectEqualStrings("/tmp/fid.csv", parsed.fidelity_csv.?);
|
||||
try std.testing.expect(parsed.verbose);
|
||||
try std.testing.expectEqual(@as(u32, 7), parsed.stale_days);
|
||||
}
|
||||
|
||||
test "consolidateBySymbol: distinct symbols pass through unchanged" {
|
||||
const allocator = std.testing.allocator;
|
||||
const rows = [_]BrokeragePosition{
|
||||
|
|
|
|||
|
|
@ -537,6 +537,56 @@ pub fn parseRequiredDateOrStderr(
|
|||
};
|
||||
}
|
||||
|
||||
/// Consume the value argument for a value-taking flag during a
|
||||
/// command's `parseArgs` loop.
|
||||
///
|
||||
/// Call with `i` pointing at the flag token. On success, advances
|
||||
/// `i.*` past the value (so the caller's `: (i += 1)` loop step
|
||||
/// lands on the token after the value) and returns the value slice.
|
||||
///
|
||||
/// Enforces the two invariants every value-flag wants:
|
||||
/// 1. a value must follow the flag (not end-of-args), and
|
||||
/// 2. the value must not be flag-shaped (a leading `-` with more
|
||||
/// characters after it), which almost always means the user
|
||||
/// forgot the value and the next flag got silently swallowed
|
||||
/// as the "value".
|
||||
///
|
||||
/// The lone `-` is deliberately ALLOWED through: it's the
|
||||
/// conventional stdin/stdout sentinel (e.g. `import --wells-fargo
|
||||
/// -`), not a flag. An empty-string value (`--flag ""`) is allowed
|
||||
/// too (not flag-shaped); a deliberate empty argument is the
|
||||
/// caller's to interpret.
|
||||
///
|
||||
/// On violation, prints a specific stderr message naming `flag`
|
||||
/// (and the offending token) and returns `error.MissingFlagValue`;
|
||||
/// `i.*` is left unchanged. Callers list `MissingFlagValue` in
|
||||
/// their `meta.user_errors` so the dispatcher maps it to a clean
|
||||
/// exit 1.
|
||||
pub fn requireFlagValue(
|
||||
io: std.Io,
|
||||
cmd_args: []const []const u8,
|
||||
i: *usize,
|
||||
flag: []const u8,
|
||||
) error{MissingFlagValue}![]const u8 {
|
||||
if (i.* + 1 >= cmd_args.len) {
|
||||
stderrPrint(io, "Error: ");
|
||||
stderrPrint(io, flag);
|
||||
stderrPrint(io, " requires a value\n");
|
||||
return error.MissingFlagValue;
|
||||
}
|
||||
const value = cmd_args[i.* + 1];
|
||||
if (value.len > 1 and value[0] == '-') {
|
||||
stderrPrint(io, "Error: ");
|
||||
stderrPrint(io, flag);
|
||||
stderrPrint(io, " requires a value, got flag: ");
|
||||
stderrPrint(io, value);
|
||||
stderrPrint(io, "\n");
|
||||
return error.MissingFlagValue;
|
||||
}
|
||||
i.* += 1;
|
||||
return value;
|
||||
}
|
||||
|
||||
// ── Commit-spec parsing (shared by contributions / compare) ──
|
||||
|
||||
/// Re-export of `git.CommitSpec` so call sites already using `cli.*`
|
||||
|
|
@ -716,6 +766,51 @@ test "parseCommitSpec: trims whitespace" {
|
|||
try std.testing.expect((try parseCommitSpec(" working ", today)) == .working_copy);
|
||||
}
|
||||
|
||||
test "requireFlagValue: returns value and advances index past it" {
|
||||
const args = [_][]const u8{ "--out", "snap.srf" };
|
||||
var i: usize = 0;
|
||||
const v = try requireFlagValue(std.testing.io, &args, &i, "--out");
|
||||
try std.testing.expectEqualStrings("snap.srf", v);
|
||||
try std.testing.expectEqual(@as(usize, 1), i);
|
||||
}
|
||||
|
||||
test "requireFlagValue: missing value at end of args is rejected, index unchanged" {
|
||||
const args = [_][]const u8{"--out"};
|
||||
var i: usize = 0;
|
||||
try std.testing.expectError(error.MissingFlagValue, requireFlagValue(std.testing.io, &args, &i, "--out"));
|
||||
try std.testing.expectEqual(@as(usize, 0), i);
|
||||
}
|
||||
|
||||
test "requireFlagValue: flag-shaped value is rejected (next flag not swallowed), index unchanged" {
|
||||
const args = [_][]const u8{ "--out", "--force" };
|
||||
var i: usize = 0;
|
||||
try std.testing.expectError(error.MissingFlagValue, requireFlagValue(std.testing.io, &args, &i, "--out"));
|
||||
try std.testing.expectEqual(@as(usize, 0), i);
|
||||
}
|
||||
|
||||
test "requireFlagValue: empty-string value is accepted (not flag-shaped)" {
|
||||
const args = [_][]const u8{ "--out", "" };
|
||||
var i: usize = 0;
|
||||
const v = try requireFlagValue(std.testing.io, &args, &i, "--out");
|
||||
try std.testing.expectEqualStrings("", v);
|
||||
try std.testing.expectEqual(@as(usize, 1), i);
|
||||
}
|
||||
|
||||
test "requireFlagValue: a value containing a non-leading dash is fine" {
|
||||
const args = [_][]const u8{ "--out", "year-end.srf" };
|
||||
var i: usize = 0;
|
||||
const v = try requireFlagValue(std.testing.io, &args, &i, "--out");
|
||||
try std.testing.expectEqualStrings("year-end.srf", v);
|
||||
}
|
||||
|
||||
test "requireFlagValue: lone '-' stdin sentinel is accepted" {
|
||||
const args = [_][]const u8{ "--wells-fargo", "-" };
|
||||
var i: usize = 0;
|
||||
const v = try requireFlagValue(std.testing.io, &args, &i, "--wells-fargo");
|
||||
try std.testing.expectEqualStrings("-", v);
|
||||
try std.testing.expectEqual(@as(usize, 1), i);
|
||||
}
|
||||
|
||||
/// Snap a requested snapshot date to the nearest earlier snapshot
|
||||
/// that exists in `hist_dir`, printing CLI-friendly stderr messages
|
||||
/// when resolution fails.
|
||||
|
|
|
|||
|
|
@ -227,6 +227,7 @@ pub const meta: framework.Meta = .{
|
|||
.uppercase_first_arg = false,
|
||||
.user_errors = error{
|
||||
UnexpectedArg,
|
||||
MissingFlagValue,
|
||||
MissingSource,
|
||||
ConflictingSources,
|
||||
MissingPortfolioPath,
|
||||
|
|
@ -254,33 +255,13 @@ pub fn parseArgs(ctx: *framework.RunCtx, cmd_args: []const []const u8) !ParsedAr
|
|||
while (i < cmd_args.len) : (i += 1) {
|
||||
const a = cmd_args[i];
|
||||
if (std.mem.eql(u8, a, "--fidelity")) {
|
||||
if (i + 1 >= cmd_args.len) {
|
||||
cli.stderrPrint(ctx.io, "Error: --fidelity requires a CSV path\n");
|
||||
return error.UnexpectedArg;
|
||||
}
|
||||
i += 1;
|
||||
fidelity_path = cmd_args[i];
|
||||
fidelity_path = try cli.requireFlagValue(ctx.io, cmd_args, &i, a);
|
||||
} else if (std.mem.eql(u8, a, "--schwab")) {
|
||||
if (i + 1 >= cmd_args.len) {
|
||||
cli.stderrPrint(ctx.io, "Error: --schwab requires a CSV path\n");
|
||||
return error.UnexpectedArg;
|
||||
}
|
||||
i += 1;
|
||||
schwab_path = cmd_args[i];
|
||||
schwab_path = try cli.requireFlagValue(ctx.io, cmd_args, &i, a);
|
||||
} else if (std.mem.eql(u8, a, "--wells-fargo")) {
|
||||
if (i + 1 >= cmd_args.len) {
|
||||
cli.stderrPrint(ctx.io, "Error: --wells-fargo requires a path (or '-' for stdin)\n");
|
||||
return error.UnexpectedArg;
|
||||
}
|
||||
i += 1;
|
||||
wells_fargo_path = cmd_args[i];
|
||||
wells_fargo_path = try cli.requireFlagValue(ctx.io, cmd_args, &i, a);
|
||||
} else if (std.mem.eql(u8, a, "--account")) {
|
||||
if (i + 1 >= cmd_args.len) {
|
||||
cli.stderrPrint(ctx.io, "Error: --account requires a name\n");
|
||||
return error.UnexpectedArg;
|
||||
}
|
||||
i += 1;
|
||||
account_override = cmd_args[i];
|
||||
account_override = try cli.requireFlagValue(ctx.io, cmd_args, &i, a);
|
||||
} else if (std.mem.eql(u8, a, "-y") or std.mem.eql(u8, a, "--yes")) {
|
||||
yes = true;
|
||||
} else {
|
||||
|
|
@ -1418,7 +1399,25 @@ test "parseArgs: --fidelity without value errors" {
|
|||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = testing.io;
|
||||
const args = [_][]const u8{"--fidelity"};
|
||||
try testing.expectError(error.UnexpectedArg, parseArgs(&ctx, &args));
|
||||
try testing.expectError(error.MissingFlagValue, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --fidelity followed by a flag does not swallow the flag" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = testing.io;
|
||||
const args = [_][]const u8{ "--fidelity", "--yes" };
|
||||
try testing.expectError(error.MissingFlagValue, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --wells-fargo accepts the lone '-' stdin sentinel" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = testing.io;
|
||||
const args = [_][]const u8{ "--wells-fargo", "-" };
|
||||
const parsed = try parseArgs(&ctx, &args);
|
||||
switch (parsed.source) {
|
||||
.wells_fargo => |wf| try testing.expectEqualStrings("-", wf.path),
|
||||
else => try testing.expect(false),
|
||||
}
|
||||
}
|
||||
|
||||
test "Source.label: returns broker name" {
|
||||
|
|
|
|||
|
|
@ -140,12 +140,7 @@ pub fn parseArgs(ctx: *framework.RunCtx, cmd_args: []const []const u8) !ParsedAr
|
|||
} else if (std.mem.eql(u8, a, "--real")) {
|
||||
real_mode = true;
|
||||
} else if (std.mem.eql(u8, a, "--export-chart")) {
|
||||
if (i + 1 >= cmd_args.len) {
|
||||
cli.stderrPrint(io, "Error: --export-chart requires a path argument.\n");
|
||||
return error.MissingFlagValue;
|
||||
}
|
||||
export_chart = cmd_args[i + 1];
|
||||
i += 1;
|
||||
export_chart = try cli.requireFlagValue(io, cmd_args, &i, a);
|
||||
} else if (std.mem.eql(u8, a, "--as-of") or std.mem.eql(u8, a, "--vs")) {
|
||||
if (i + 1 >= cmd_args.len) {
|
||||
cli.stderrPrint(io, "Error: ");
|
||||
|
|
@ -1642,6 +1637,18 @@ test "parseArgs: unknown flag errors" {
|
|||
try testing.expectError(error.UnexpectedArg, parseArgsForTest(today, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --export-chart without a value is rejected" {
|
||||
const today = Date.fromYmd(2026, 5, 9);
|
||||
const args = [_][]const u8{"--export-chart"};
|
||||
try testing.expectError(error.MissingFlagValue, parseArgsForTest(today, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --export-chart followed by a flag does not swallow the flag" {
|
||||
const today = Date.fromYmd(2026, 5, 9);
|
||||
const args = [_][]const u8{ "--export-chart", "--real" };
|
||||
try testing.expectError(error.MissingFlagValue, parseArgsForTest(today, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --overlay-actuals carries into bands" {
|
||||
const today = Date.fromYmd(2026, 5, 9);
|
||||
const args = [_][]const u8{ "--as-of", "2026-04-01", "--overlay-actuals" };
|
||||
|
|
|
|||
|
|
@ -67,13 +67,12 @@ pub fn parseArgs(ctx: *framework.RunCtx, cmd_args: []const []const u8) !ParsedAr
|
|||
while (i < cmd_args.len) : (i += 1) {
|
||||
const a = cmd_args[i];
|
||||
if (std.mem.eql(u8, a, "--export-chart")) {
|
||||
if (i + 1 >= cmd_args.len) {
|
||||
cli.stderrPrint(ctx.io, "Error: --export-chart requires a path argument.\n");
|
||||
return error.MissingFlagValue;
|
||||
}
|
||||
export_chart = cmd_args[i + 1];
|
||||
i += 1;
|
||||
} else if (std.mem.startsWith(u8, a, "--")) {
|
||||
export_chart = try cli.requireFlagValue(ctx.io, cmd_args, &i, a);
|
||||
} else if (a.len > 0 and a[0] == '-') {
|
||||
// Reject ANY leading-dash token we don't recognize,
|
||||
// including single-dash ones like `-x`. Previously only
|
||||
// `--`-prefixed flags were caught, so `-x` slipped through
|
||||
// and became the symbol.
|
||||
cli.stderrPrint(ctx.io, "Error: 'quote': unexpected flag ");
|
||||
cli.stderrPrint(ctx.io, a);
|
||||
cli.stderrPrint(ctx.io, "\n");
|
||||
|
|
@ -323,6 +322,36 @@ test "parseArgs: extra args error" {
|
|||
try std.testing.expectError(error.UnexpectedArg, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --export-chart captures the path" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = std.testing.io;
|
||||
const args = [_][]const u8{ "AAPL", "--export-chart", "aapl.png" };
|
||||
const parsed = try parseArgs(&ctx, &args);
|
||||
try std.testing.expectEqualStrings("AAPL", parsed.symbol);
|
||||
try std.testing.expectEqualStrings("aapl.png", parsed.export_chart.?);
|
||||
}
|
||||
|
||||
test "parseArgs: single-dash unknown flag is rejected (not swallowed as symbol)" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = std.testing.io;
|
||||
const args = [_][]const u8{"-x"};
|
||||
try std.testing.expectError(error.UnexpectedArg, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --export-chart without a value is rejected" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = std.testing.io;
|
||||
const args = [_][]const u8{ "AAPL", "--export-chart" };
|
||||
try std.testing.expectError(error.MissingFlagValue, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --export-chart followed by a flag does not swallow the flag" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = std.testing.io;
|
||||
const args = [_][]const u8{ "AAPL", "--export-chart", "--bogus" };
|
||||
try std.testing.expectError(error.MissingFlagValue, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "display with candles only" {
|
||||
var buf: [8192]u8 = undefined;
|
||||
var w: std.Io.Writer = .fixed(&buf);
|
||||
|
|
|
|||
|
|
@ -108,7 +108,7 @@ pub const meta: framework.Meta = .{
|
|||
\\
|
||||
,
|
||||
.uppercase_first_arg = false,
|
||||
.user_errors = error{ UnexpectedArg, BadMetadata, NoCommitBeforeDate, NoMetadata, PathMissingInRev, WriteFailed },
|
||||
.user_errors = error{ UnexpectedArg, BadMetadata, NoCommitBeforeDate, NoMetadata, PathMissingInRev, WriteFailed, MissingFlagValue },
|
||||
};
|
||||
|
||||
pub fn parseArgs(ctx: *framework.RunCtx, cmd_args: []const []const u8) !ParsedArgs {
|
||||
|
|
@ -121,12 +121,7 @@ pub fn parseArgs(ctx: *framework.RunCtx, cmd_args: []const []const u8) !ParsedAr
|
|||
} else if (std.mem.eql(u8, a, "--dry-run")) {
|
||||
parsed.dry_run = true;
|
||||
} else if (std.mem.eql(u8, a, "--out")) {
|
||||
i += 1;
|
||||
if (i >= cmd_args.len) {
|
||||
cli.stderrPrint(ctx.io, "Error: --out requires a path argument\n");
|
||||
return error.UnexpectedArg;
|
||||
}
|
||||
parsed.out_override = cmd_args[i];
|
||||
parsed.out_override = try cli.requireFlagValue(ctx.io, cmd_args, &i, a);
|
||||
} else if (std.mem.eql(u8, a, "--as-of")) {
|
||||
i += 1;
|
||||
if (i >= cmd_args.len) {
|
||||
|
|
@ -1010,7 +1005,15 @@ test "parseArgs: --out without value errors" {
|
|||
ctx.io = std.testing.io;
|
||||
ctx.now_s = 0;
|
||||
const args = [_][]const u8{"--out"};
|
||||
try std.testing.expectError(error.UnexpectedArg, parseArgs(&ctx, &args));
|
||||
try std.testing.expectError(error.MissingFlagValue, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --out followed by a flag does not swallow the flag" {
|
||||
var ctx: framework.RunCtx = undefined;
|
||||
ctx.io = std.testing.io;
|
||||
ctx.now_s = 0;
|
||||
const args = [_][]const u8{ "--out", "--force" };
|
||||
try std.testing.expectError(error.MissingFlagValue, parseArgs(&ctx, &args));
|
||||
}
|
||||
|
||||
test "parseArgs: --as-of with explicit date" {
|
||||
|
|
|
|||
34
src/main.zig
34
src/main.zig
|
|
@ -231,7 +231,14 @@ fn parseGlobals(allocator: std.mem.Allocator, args: []const []const u8) GlobalPa
|
|||
}
|
||||
if (std.mem.eql(u8, a, "-p") or std.mem.eql(u8, a, "--portfolio")) {
|
||||
if (i + 1 >= args.len) return error.MissingValue;
|
||||
try patterns.append(allocator, args[i + 1]);
|
||||
// Reject a flag-shaped value (leading '-' with more after
|
||||
// it): almost certainly the user forgot the value and the
|
||||
// next flag would otherwise be silently consumed as the
|
||||
// pattern. The lone '-' is left alone (harmless; resolves
|
||||
// to nothing).
|
||||
const value = args[i + 1];
|
||||
if (value.len > 1 and value[0] == '-') return error.MissingValue;
|
||||
try patterns.append(allocator, value);
|
||||
// Detect the unquoted-glob shape: we just consumed `-p VALUE`,
|
||||
// and the next args are more `.srf` files with no flag in
|
||||
// between. That's almost always the shell expanding `-p
|
||||
|
|
@ -244,7 +251,9 @@ fn parseGlobals(allocator: std.mem.Allocator, args: []const []const u8) GlobalPa
|
|||
}
|
||||
if (std.mem.eql(u8, a, "-w") or std.mem.eql(u8, a, "--watchlist")) {
|
||||
if (i + 1 >= args.len) return error.MissingValue;
|
||||
g.watchlist_path = args[i + 1];
|
||||
const value = args[i + 1];
|
||||
if (value.len > 1 and value[0] == '-') return error.MissingValue;
|
||||
g.watchlist_path = value;
|
||||
i += 2;
|
||||
continue;
|
||||
}
|
||||
|
|
@ -600,6 +609,27 @@ test "parseGlobals: flag missing value errors" {
|
|||
try std.testing.expectError(error.MissingValue, parseGlobals(allocator, &argv));
|
||||
}
|
||||
|
||||
test "parseGlobals: -p with a flag-shaped value errors (next flag not swallowed)" {
|
||||
const allocator = std.testing.allocator;
|
||||
const argv = [_][]const u8{ "zfin", "-p", "--no-color", "portfolio" };
|
||||
try std.testing.expectError(error.MissingValue, parseGlobals(allocator, &argv));
|
||||
}
|
||||
|
||||
test "parseGlobals: -w with a flag-shaped value errors" {
|
||||
const allocator = std.testing.allocator;
|
||||
const argv = [_][]const u8{ "zfin", "-w", "--no-color", "portfolio" };
|
||||
try std.testing.expectError(error.MissingValue, parseGlobals(allocator, &argv));
|
||||
}
|
||||
|
||||
test "parseGlobals: -p accepts the lone '-' (not treated as a flag)" {
|
||||
const allocator = std.testing.allocator;
|
||||
const argv = [_][]const u8{ "zfin", "-p", "-", "portfolio" };
|
||||
const g = try parseGlobals(allocator, &argv);
|
||||
defer allocator.free(g.portfolio_patterns);
|
||||
try std.testing.expectEqual(@as(usize, 1), g.portfolio_patterns.len);
|
||||
try std.testing.expectEqualStrings("-", g.portfolio_patterns[0]);
|
||||
}
|
||||
|
||||
test "parseGlobals: --help stops scanning" {
|
||||
const allocator = std.testing.allocator;
|
||||
const argv = [_][]const u8{ "zfin", "--help" };
|
||||
|
|
|
|||
219
src/tui.zig
219
src/tui.zig
|
|
@ -2214,31 +2214,54 @@ fn printDefaultKeys(io: std.Io) !void {
|
|||
try out.flush();
|
||||
}
|
||||
|
||||
/// Entry point for the interactive TUI.
|
||||
/// `args` contains only command-local tokens (everything after `interactive`).
|
||||
pub fn run(
|
||||
io: std.Io,
|
||||
allocator: std.mem.Allocator,
|
||||
config: zfin.Config,
|
||||
portfolio_patterns: []const []const u8,
|
||||
global_watchlist_path: ?[]const u8,
|
||||
args: []const []const u8,
|
||||
today: zfin.Date,
|
||||
) !void {
|
||||
const watchlist_path: ?[]const u8 = global_watchlist_path;
|
||||
var symbol: []const u8 = "";
|
||||
var symbol_upper_buf: [32]u8 = undefined;
|
||||
var has_explicit_symbol = false;
|
||||
var skip_watchlist = false;
|
||||
var chart_config: chart.ChartConfig = .{};
|
||||
/// Result of parsing the `interactive` command's local args
|
||||
/// (everything after `interactive`). Produced by `parseArgs`,
|
||||
/// consumed by `run`. The symbol is uppercased into an inline
|
||||
/// buffer so the struct owns its storage and stays copy-safe (no
|
||||
/// borrow into the caller's `args`).
|
||||
const ParsedArgs = struct {
|
||||
/// Set when a print-and-exit flag (`--default-keys` /
|
||||
/// `--default-theme`) was seen. Recorded on the FIRST
|
||||
/// occurrence so `--default-keys --default-theme` keeps the
|
||||
/// historical first-token-wins behavior. `run` honors this
|
||||
/// only after the whole arg list has validated, so trailing
|
||||
/// bogus flags are still rejected (`--default-keys --nope`
|
||||
/// exits non-zero rather than silently printing).
|
||||
print_and_exit: ?PrintAndExit = null,
|
||||
/// Uppercased symbol storage; `symbol()` returns the live slice.
|
||||
symbol_buf: [32]u8 = @splat(0),
|
||||
symbol_len: usize = 0,
|
||||
has_explicit_symbol: bool = false,
|
||||
skip_watchlist: bool = false,
|
||||
chart_config: chart.ChartConfig = .{},
|
||||
|
||||
const PrintAndExit = enum { keys, theme };
|
||||
|
||||
/// The uppercased symbol, or "" when none was supplied.
|
||||
fn symbol(self: *const ParsedArgs) []const u8 {
|
||||
return self.symbol_buf[0..self.symbol_len];
|
||||
}
|
||||
};
|
||||
|
||||
/// Parse the `interactive` command's local args. Validates the
|
||||
/// ENTIRE list before returning: unknown flags, missing flag
|
||||
/// values, and flag-shaped values all yield `error.InvalidArgs`
|
||||
/// even when they trail a print-and-exit flag. This is the fix for
|
||||
/// `interactive --default-keys --bogus-flag` silently succeeding:
|
||||
/// the print-and-exit flags no longer short-circuit the loop, so
|
||||
/// the bogus flag reaches the catch-all reject branch.
|
||||
///
|
||||
/// Error messages go to stderr (suppressed under test). Pure apart
|
||||
/// from those best-effort writes, so it's unit-testable without a
|
||||
/// terminal.
|
||||
fn parseArgs(io: std.Io, args: []const []const u8) error{InvalidArgs}!ParsedArgs {
|
||||
var result: ParsedArgs = .{};
|
||||
var i: usize = 0;
|
||||
while (i < args.len) : (i += 1) {
|
||||
if (std.mem.eql(u8, args[i], "--default-keys")) {
|
||||
try printDefaultKeys(io);
|
||||
return;
|
||||
if (result.print_and_exit == null) result.print_and_exit = .keys;
|
||||
} else if (std.mem.eql(u8, args[i], "--default-theme")) {
|
||||
try theme.printDefaults(io);
|
||||
return;
|
||||
if (result.print_and_exit == null) result.print_and_exit = .theme;
|
||||
} else if (std.mem.eql(u8, args[i], "--symbol") or std.mem.eql(u8, args[i], "-s")) {
|
||||
// -s / --symbol require a non-flag value. Bare `-s` and
|
||||
// `-s --chart …` are both user errors — surface them
|
||||
|
|
@ -2260,11 +2283,11 @@ pub fn run(
|
|||
stderr.print(io, "\n");
|
||||
return error.InvalidArgs;
|
||||
}
|
||||
const len = @min(value.len, symbol_upper_buf.len);
|
||||
_ = std.ascii.upperString(symbol_upper_buf[0..len], value[0..len]);
|
||||
symbol = symbol_upper_buf[0..len];
|
||||
has_explicit_symbol = true;
|
||||
skip_watchlist = true;
|
||||
const len = @min(value.len, result.symbol_buf.len);
|
||||
_ = std.ascii.upperString(result.symbol_buf[0..len], value[0..len]);
|
||||
result.symbol_len = len;
|
||||
result.has_explicit_symbol = true;
|
||||
result.skip_watchlist = true;
|
||||
} else if (std.mem.eql(u8, args[i], "--chart")) {
|
||||
// Same shape as -s / --symbol: require a value, reject
|
||||
// flag-shaped values.
|
||||
|
|
@ -2281,7 +2304,7 @@ pub fn run(
|
|||
return error.InvalidArgs;
|
||||
}
|
||||
if (chart.ChartConfig.parse(value)) |cc| {
|
||||
chart_config = cc;
|
||||
result.chart_config = cc;
|
||||
} else {
|
||||
stderr.print(io, "Error: --chart value is not a valid WIDTHxHEIGHT spec: ");
|
||||
stderr.print(io, value);
|
||||
|
|
@ -2297,12 +2320,43 @@ pub fn run(
|
|||
stderr.print(io, "\nRun 'zfin interactive --help' for usage.\n");
|
||||
return error.InvalidArgs;
|
||||
} else if (args[i].len > 0) {
|
||||
const len = @min(args[i].len, symbol_upper_buf.len);
|
||||
_ = std.ascii.upperString(symbol_upper_buf[0..len], args[i][0..len]);
|
||||
symbol = symbol_upper_buf[0..len];
|
||||
has_explicit_symbol = true;
|
||||
const len = @min(args[i].len, result.symbol_buf.len);
|
||||
_ = std.ascii.upperString(result.symbol_buf[0..len], args[i][0..len]);
|
||||
result.symbol_len = len;
|
||||
result.has_explicit_symbol = true;
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
/// Entry point for the interactive TUI.
|
||||
/// `args` contains only command-local tokens (everything after `interactive`).
|
||||
pub fn run(
|
||||
io: std.Io,
|
||||
allocator: std.mem.Allocator,
|
||||
config: zfin.Config,
|
||||
portfolio_patterns: []const []const u8,
|
||||
global_watchlist_path: ?[]const u8,
|
||||
args: []const []const u8,
|
||||
today: zfin.Date,
|
||||
) !void {
|
||||
const watchlist_path: ?[]const u8 = global_watchlist_path;
|
||||
|
||||
// Parse + validate the full arg list first. Print-and-exit
|
||||
// flags (`--default-keys` / `--default-theme`) are honored only
|
||||
// after validation, so trailing bogus flags still exit non-zero.
|
||||
const parsed = try parseArgs(io, args);
|
||||
if (parsed.print_and_exit) |which| {
|
||||
switch (which) {
|
||||
.keys => try printDefaultKeys(io),
|
||||
.theme => try theme.printDefaults(io),
|
||||
}
|
||||
return;
|
||||
}
|
||||
const symbol = parsed.symbol();
|
||||
const has_explicit_symbol = parsed.has_explicit_symbol;
|
||||
const skip_watchlist = parsed.skip_watchlist;
|
||||
const chart_config = parsed.chart_config;
|
||||
|
||||
var keymap = blk: {
|
||||
const home_opt = if (config.environ_map) |em| em.get("HOME") else null;
|
||||
|
|
@ -2668,6 +2722,109 @@ test "formatStatusHint: single fragment has no separator" {
|
|||
try testing.expectEqualStrings("ctrl+s save", out);
|
||||
}
|
||||
|
||||
// ── interactive parseArgs ─────────────────────────────────────
|
||||
|
||||
test "parseArgs: no args yields defaults" {
|
||||
const p = try parseArgs(std.testing.io, &.{});
|
||||
try testing.expect(p.print_and_exit == null);
|
||||
try testing.expectEqualStrings("", p.symbol());
|
||||
try testing.expect(!p.has_explicit_symbol);
|
||||
try testing.expect(!p.skip_watchlist);
|
||||
try testing.expectEqual(chart.ChartConfig{}, p.chart_config);
|
||||
}
|
||||
|
||||
test "parseArgs: positional symbol is uppercased and marked explicit" {
|
||||
const p = try parseArgs(std.testing.io, &.{"vti"});
|
||||
try testing.expectEqualStrings("VTI", p.symbol());
|
||||
try testing.expect(p.has_explicit_symbol);
|
||||
// A bare positional does NOT skip the watchlist (only -s/--symbol does).
|
||||
try testing.expect(!p.skip_watchlist);
|
||||
}
|
||||
|
||||
test "parseArgs: --symbol sets symbol and skips watchlist" {
|
||||
const p = try parseArgs(std.testing.io, &.{ "--symbol", "aapl" });
|
||||
try testing.expectEqualStrings("AAPL", p.symbol());
|
||||
try testing.expect(p.has_explicit_symbol);
|
||||
try testing.expect(p.skip_watchlist);
|
||||
}
|
||||
|
||||
test "parseArgs: -s short form behaves like --symbol" {
|
||||
const p = try parseArgs(std.testing.io, &.{ "-s", "msft" });
|
||||
try testing.expectEqualStrings("MSFT", p.symbol());
|
||||
try testing.expect(p.skip_watchlist);
|
||||
}
|
||||
|
||||
test "parseArgs: --default-keys requests the keys template" {
|
||||
const p = try parseArgs(std.testing.io, &.{"--default-keys"});
|
||||
try testing.expectEqual(ParsedArgs.PrintAndExit.keys, p.print_and_exit.?);
|
||||
}
|
||||
|
||||
test "parseArgs: --default-theme requests the theme template" {
|
||||
const p = try parseArgs(std.testing.io, &.{"--default-theme"});
|
||||
try testing.expectEqual(ParsedArgs.PrintAndExit.theme, p.print_and_exit.?);
|
||||
}
|
||||
|
||||
test "parseArgs: --default-keys with a trailing bogus flag is rejected" {
|
||||
// Regression: print-and-exit flags used to short-circuit before
|
||||
// the rest of the args were validated, so `--default-keys
|
||||
// --bogus-flag` silently printed and exited 0.
|
||||
try testing.expectError(error.InvalidArgs, parseArgs(std.testing.io, &.{ "--default-keys", "--bogus-flag" }));
|
||||
}
|
||||
|
||||
test "parseArgs: --default-theme with a trailing bogus flag is rejected" {
|
||||
try testing.expectError(error.InvalidArgs, parseArgs(std.testing.io, &.{ "--default-theme", "--bogus-flag" }));
|
||||
}
|
||||
|
||||
test "parseArgs: --default-keys with a valid trailing positional still requests keys (no error)" {
|
||||
// A trailing *positional* is harmless: print-and-exit takes
|
||||
// precedence and the positional is ignored. Only bogus flags error.
|
||||
const p = try parseArgs(std.testing.io, &.{ "--default-keys", "VTI" });
|
||||
try testing.expectEqual(ParsedArgs.PrintAndExit.keys, p.print_and_exit.?);
|
||||
}
|
||||
|
||||
test "parseArgs: first print-and-exit flag wins" {
|
||||
const p = try parseArgs(std.testing.io, &.{ "--default-theme", "--default-keys" });
|
||||
try testing.expectEqual(ParsedArgs.PrintAndExit.theme, p.print_and_exit.?);
|
||||
}
|
||||
|
||||
test "parseArgs: unknown flag is rejected" {
|
||||
try testing.expectError(error.InvalidArgs, parseArgs(std.testing.io, &.{"--nope"}));
|
||||
}
|
||||
|
||||
test "parseArgs: --symbol without a value is rejected" {
|
||||
try testing.expectError(error.InvalidArgs, parseArgs(std.testing.io, &.{"--symbol"}));
|
||||
}
|
||||
|
||||
test "parseArgs: --symbol followed by a flag is rejected" {
|
||||
try testing.expectError(error.InvalidArgs, parseArgs(std.testing.io, &.{ "-s", "--chart" }));
|
||||
}
|
||||
|
||||
test "parseArgs: --chart parses a valid WxH spec" {
|
||||
const p = try parseArgs(std.testing.io, &.{ "--chart", "800x600" });
|
||||
try testing.expectEqual(chart.ChartMode.kitty, p.chart_config.mode);
|
||||
try testing.expectEqual(@as(u32, 800), p.chart_config.max_width);
|
||||
try testing.expectEqual(@as(u32, 600), p.chart_config.max_height);
|
||||
}
|
||||
|
||||
test "parseArgs: --chart without a value is rejected" {
|
||||
try testing.expectError(error.InvalidArgs, parseArgs(std.testing.io, &.{"--chart"}));
|
||||
}
|
||||
|
||||
test "parseArgs: --chart with an invalid spec is rejected" {
|
||||
try testing.expectError(error.InvalidArgs, parseArgs(std.testing.io, &.{ "--chart", "nonsense" }));
|
||||
}
|
||||
|
||||
test "parseArgs: --chart with a flag-shaped value is rejected" {
|
||||
try testing.expectError(error.InvalidArgs, parseArgs(std.testing.io, &.{ "--chart", "-x" }));
|
||||
}
|
||||
|
||||
test "parseArgs: a symbol longer than the buffer is truncated, not overflowed" {
|
||||
const long = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"; // 36 chars > 32
|
||||
const p = try parseArgs(std.testing.io, &.{long});
|
||||
try testing.expectEqual(@as(usize, 32), p.symbol().len);
|
||||
try testing.expectEqualStrings(long[0..32], p.symbol());
|
||||
}
|
||||
|
||||
test "writeDefaultKeys: includes preamble, global section, and per-tab sections" {
|
||||
var aw: std.Io.Writer.Allocating = .init(testing.allocator);
|
||||
defer aw.deinit();
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue