From 1f582d46743a2e4cd0c63f9ae1fc75becac64373 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Sat, 26 Sep 2026 23:52:34 +0200 Subject: [PATCH] cli: report usage errors once, accept -h, show tips only on a terminal A flag given without its value, or an extra positional, failed with only `FATAL exit err=MissingArgument`, not naming the flag. The parser now logs which flag is missing its value or which argument is extra, with a hint pointing at the command's help, and a missing fetch URL is caught while parsing, next to run's missing script. Since each of these is logged where it's found, main exits without the generic `exit` line. `-h` works wherever `--help` does, instead of being taken as a URL. The --obey-robots tip is for a person at a terminal, so it's skipped when stderr isn't one; scripts capturing stderr no longer get it on every run. --- src/Config.zig | 9 +++++++-- src/cli.zig | 34 ++++++++++++++++++++++++++-------- src/main.zig | 20 ++++++++++++++------ 3 files changed, 47 insertions(+), 16 deletions(-) diff --git a/src/Config.zig b/src/Config.zig index 8bbad8b10..095976a72 100644 --- a/src/Config.zig +++ b/src/Config.zig @@ -42,7 +42,7 @@ pub const CDP_TCP_USER_TIMEOUT_MS: c_int = 10_000; const Config = @This(); fn logFilterValidator(allocator: Allocator, args: *std.process.Args.Iterator, list: *std.ArrayList(log.FilterRule)) !void { - const str = args.next() orelse return error.InvalidOption; + const str = args.next() orelse return error.MissingArgument; defer log.opts.scope_enabled = log.resolveFilters(list.items); @@ -730,7 +730,7 @@ var stderr_tty_once = lp.once(initStderrTty); fn initStderrTty() void { stderr_tty_cached = std.Io.File.stderr().isTty(lp.io) catch false; } -fn stderrIsTty() bool { +pub fn stderrIsTty() bool { stderr_tty_once.call(); return stderr_tty_cached; } @@ -1230,6 +1230,11 @@ pub fn parseArgs(allocator: Allocator, proc_args: std.process.Args) !Config { command = .{ .agent = agent_opts }; } + if (command == .fetch and command.fetch.url.items.len == 0) { + log.fatal(.app, "missing URL", .{ .hint = "usage: lightpanda fetch ... [OPTIONS]" }); + return error.MissingArgument; + } + // Agent mode quiets page-driven `console.error` noise unless // verbosity=high. Depends on --verbosity/--task, so it can only be // resolved after the options are parsed; an explicit --log-level wins. diff --git a/src/cli.zig b/src/cli.zig index e5e9abedc..9ee5d4d6d 100644 --- a/src/cli.zig +++ b/src/cli.zig @@ -485,6 +485,10 @@ pub fn Builder(comptime commands: anytype) type { /// Try to sniff the command out of given option. /// Only exists for legacy reasons; hence hardcoded. fn sniffCommand(cmd_str: []const u8) error{UnknownCommand}!Enum { + if (std.mem.eql(u8, cmd_str, "--help") or std.mem.eql(u8, cmd_str, "-h")) { + return .help; + } + if (std.mem.startsWith(u8, cmd_str, "--") == false) { return .fetch; } @@ -514,11 +518,6 @@ pub fn Builder(comptime commands: anytype) type { } } - // Legacy `--help` flag maps to the `help` command. - if (std.mem.eql(u8, cmd_str, "--help")) { - return .help; - } - return error.UnknownCommand; } @@ -724,6 +723,18 @@ pub fn Builder(comptime commands: anytype) type { }; } + fn helpHint(comptime command_name: []const u8) []const u8 { + return "see 'lightpanda help " ++ command_name ++ "'"; + } + + /// Validators return `error.MissingArgument` without logging when a + /// flag is the last argument, since only the parser knows its name. + fn logMissingValue(err: anyerror, arg: []const u8, comptime command_name: []const u8) void { + if (err == error.MissingArgument) { + log.fatal(.app, "missing argument value", .{ .arg = arg, .hint = helpHint(command_name) }); + } + } + /// Parses the command with its options. fn parseCommand( allocator: Allocator, @@ -761,7 +772,10 @@ pub fn Builder(comptime commands: anytype) type { std.mem.eql(u8, option_name, "--" ++ comptime toKebabCase(name)) or (matches_short and std.mem.eql(u8, option_name, "-" ++ [_]u8{option.short}))) { - try parseValue(allocator, args, &@field(c, field_name), option); + parseValue(allocator, args, &@field(c, field_name), option) catch |err| { + logMissingValue(err, option_name, command.name); + return err; + }; continue :iter_args; } @@ -786,7 +800,10 @@ pub fn Builder(comptime commands: anytype) type { break :blk .{ .name = variant.name, .type = option.type, .multiple = is_multiple }; }; - try parseValue(allocator, args, &@field(c, field_name), opts); + parseValue(allocator, args, &@field(c, field_name), opts) catch |err| { + logMissingValue(err, option_name, command.name); + return err; + }; continue :iter_args; } } @@ -794,7 +811,7 @@ pub fn Builder(comptime commands: anytype) type { } // Subcommand help: `lightpanda fetch help` or `lightpanda fetch --help`. - if (std.mem.eql(u8, option_name, "help") or std.mem.eql(u8, option_name, "--help")) { + if (std.mem.eql(u8, option_name, "help") or std.mem.eql(u8, option_name, "--help") or std.mem.eql(u8, option_name, "-h")) { return @unionInit(Union, "help", std.meta.stringToEnum(Enum, command.name).?); } @@ -820,6 +837,7 @@ pub fn Builder(comptime commands: anytype) type { // A single (non-multiple) positional may only be given once. if (!is_multiple and @field(c, positional.name) != null) { + log.fatal(.app, "too many arguments", .{ .mode = command.name, .arg = option_name, .hint = helpHint(command.name) }); return error.TooManyPositionalArguments; } diff --git a/src/main.zig b/src/main.zig index 3d10ff3a7..a669571de 100644 --- a/src/main.zig +++ b/src/main.zig @@ -63,7 +63,17 @@ fn run(allocator: Allocator, main_arena: Allocator, proc_args: std.process.Args) lp.core_dump.disableIfRequested(); lp.crash_handler.attachSignalHandlers(); - const args = try Config.parseArgs(main_arena, proc_args); + const args = Config.parseArgs(main_arena, proc_args) catch |err| switch (err) { + // Already logged where they were found. + error.UnknownCommand, + error.UnknownOption, + error.InvalidOption, + error.InvalidArgument, + error.MissingArgument, + error.TooManyPositionalArguments, + => std.process.exit(1), + else => return err, + }; defer args.deinit(main_arena); switch (args.mode) { @@ -132,11 +142,6 @@ fn run(allocator: Allocator, main_arena: Allocator, proc_args: std.process.Args) .fetch => |opts| { const urls = opts.url.items; - if (urls.len == 0) { - log.fatal(.app, "missing URL", .{}); - return error.MissingArgument; - } - // Plain (non-JSON) dump writes one document to stdout with no // framing, so it can't disambiguate more than one page. if (urls.len == 1) { @@ -401,6 +406,9 @@ fn mcpThread(allocator: std.mem.Allocator, app: *App, cdp_server: ?*lp.Server, e } fn logConfigTips(config: *const Config) void { + // Only for a person reading the terminal, not for scripts capturing stderr. + if (!Config.stderrIsTty()) return; + var count: usize = 0; var tips: [2]log.KV = undefined; if (config.obeyRobots() == false) {