Merge pull request #3650 from lightpanda-io/cli-error-ergonomics

cli: report usage errors once, accept -h, show tips only on a terminal

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 <url>... [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.

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,12 +518,7 @@ 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;
+            return unknownCommand(cmd_str);
         }
 
         /// Turns a snake_case string to kebab-case in comptime.
@@ -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;
                     }
 

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) {