parser: produce helpful error messages on command line errors

This is the first cut at providing human-readable context for command
line parsing failures. Since these failures are due to incorrect
input (normally produced by a human), closing the information loop at
the human layer makes a hell of a lot more sense than dumping an error
traceback with a (possibly cryptic) error name and calling it a day.

This approach doesn't print anything out by default and still depends
on the user to choose exactly how the handle and print the error
message. Errors are propagated from subcommands, though they end up
being copied, which shouldn't be strictly necessary. Maybe this can be
improved in the future. OutOfMemory has been added to ParseError to
simplify the code a bit.

The demo has been updated with a simplistic example of what presenting
error messages to the user may look like. I don't know that this
produces useful messages for every possible failure scenario, but it
does for the most common ones.

demo/demo.zig

@@ -8,15 +8,15 @@ const Choice = enum { first, second };
 const cli = cmd: {
     var cmd = CommandBuilder(*u32){
         .description =
-        \\The definitive noclip demonstration utility
+        \\The definitive noclip demonstration utility.
         \\
         \\This command demonstrates the functionality of the noclip library. cool!
         \\
-        \\> // implementing factorial recursively is a silly thing to do
-        \\> pub fn fact(n: u64) u64 {
-        \\>     if (n == 0) return 1;
-        \\>     return n*fact(n - 1);
-        \\> }
+        \\>     // implementing factorial recursively is a silly thing to do
+        \\>     pub fn fact(n: u64) u64 {
+        \\>         if (n == 0) return 1;
+        \\>         return n*fact(n - 1);
+        \\>     }
         \\
         \\Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor
         \\incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis
@@ -135,7 +135,20 @@ pub fn main() !u8 {
     try base.addSubcommand("main", try cli.createInterface(allocator, cliHandler, &context));
     try base.addSubcommand("other", try subcommand.createInterface(allocator, subHandler, &sc));
 
-    try base.execute();
+    const group = try noclip.commandGroup(allocator, .{ .description = "final level of a deeply nested subcommand" });
+    const subcon = try noclip.commandGroup(allocator, .{ .description = "third level of a deeply nested subcommand" });
+    const nested = try noclip.commandGroup(allocator, .{ .description = "second level of a deeply nested subcommand" });
+    const deeply = try noclip.commandGroup(allocator, .{ .description = "start of a deeply nested subcommand" });
+    try base.addSubcommand("deeply", deeply);
+    try deeply.addSubcommand("nested", nested);
+    try nested.addSubcommand("subcommand", subcon);
+    try subcon.addSubcommand("group", group);
+    try group.addSubcommand("run", try cli.createInterface(allocator, cliHandler, &context));
+
+    base.execute() catch |err| {
+        std.io.getStdErr().writeAll(base.getParseError()) catch {};
+        return err;
+    };
 
     return 0;
 }

source/errors.zig

@@ -12,6 +12,7 @@ pub const ParseError = error{
     UnknownLongTagParameter,
     UnknownShortTagParameter,
     RequiredParameterMissing,
+    OutOfMemory,
 };
 
 pub const NoclipError = ParseError || ConversionError;

source/parameters.zig

@@ -223,6 +223,26 @@ fn OptionType(comptime generics: ParameterGenerics) type {
         /// want weird things to happen.
         flag_bias: FlagBias,
 
+        pub fn describe(self: @This(), allocator: std.mem.Allocator) std.mem.Allocator.Error![]const u8 {
+            var buf = std.ArrayList(u8).init(allocator);
+
+            try buf.append('"');
+            try buf.appendSlice(self.name);
+            try buf.append('"');
+            if (self.short_tag != null or self.long_tag != null) {
+                try buf.appendSlice(" (");
+                if (self.short_tag) |short|
+                    try buf.appendSlice(short);
+                if (self.short_tag != null and self.long_tag != null)
+                    try buf.appendSlice(", ");
+                if (self.long_tag) |long|
+                    try buf.appendSlice(long);
+                try buf.append(')');
+            }
+
+            return try buf.toOwnedSlice();
+        }
+
         pub fn IntermediateValue(comptime _: @This()) type {
             return generics.IntermediateValue();
         }

source/parser.zig

@@ -10,8 +10,9 @@ const NoclipError = errors.NoclipError;
 pub const ParserInterface = struct {
     const Vtable = struct {
         execute: *const fn (parser: *anyopaque, context: *anyopaque) anyerror!void,
-        parse: *const fn (parser: *anyopaque, context: *anyopaque, name: []const u8, args: [][:0]u8, env: std.process.EnvMap) anyerror!void,
-        finish: *const fn (parser: *anyopaque, context: *anyopaque) anyerror!void,
+        parse: *const fn (parser: *anyopaque, context: *anyopaque, name: []const u8, args: [][:0]u8, env: std.process.EnvMap) anyerror!?ParseResult,
+        finish: *const fn (parser: *anyopaque, context: *anyopaque) anyerror!?ParserInterface,
+        getParseError: *const fn (parser: *anyopaque) []const u8,
         addSubcommand: *const fn (parser: *anyopaque, name: []const u8, subcommand: ParserInterface) std.mem.Allocator.Error!void,
         getSubcommand: *const fn (parser: *anyopaque, name: []const u8) ?ParserInterface,
         describe: *const fn () []const u8,
@@ -31,6 +32,7 @@ pub const ParserInterface = struct {
                 .execute = ParserType._wrapExecute,
                 .parse = ParserType._wrapParse,
                 .finish = ParserType._wrapFinish,
+                .getParseError = ParserType._wrapGetParseError,
                 .addSubcommand = ParserType._wrapAddSubcommand,
                 .getSubcommand = ParserType._wrapGetSubcommand,
                 .describe = ParserType._wrapDescribe,
@@ -44,14 +46,18 @@ pub const ParserInterface = struct {
         return try self.methods.execute(self.parser, self.context);
     }
 
-    pub fn parse(self: @This(), name: []const u8, args: [][:0]u8, env: std.process.EnvMap) anyerror!void {
+    pub fn parse(self: @This(), name: []const u8, args: [][:0]u8, env: std.process.EnvMap) anyerror!?ParseResult {
         return try self.methods.parse(self.parser, self.context, name, args, env);
     }
 
-    pub fn finish(self: @This()) anyerror!void {
+    pub fn finish(self: @This()) anyerror!?ParserInterface {
         return try self.methods.finish(self.parser, self.context);
     }
 
+    pub fn getParseError(self: @This()) []const u8 {
+        return self.methods.getParseError(self.parser);
+    }
+
     pub fn addSubcommand(self: @This(), name: []const u8, subcommand: ParserInterface) std.mem.Allocator.Error!void {
         return try self.methods.addSubcommand(self.parser, name, subcommand);
     }
@@ -74,6 +80,7 @@ pub const ParserInterface = struct {
 };
 
 pub const CommandMap = std.StringArrayHashMap(ParserInterface);
+const ParseResult = struct { name: []const u8, args: [][:0]u8, parser: ParserInterface };
 
 // the parser is generated by the bind method of the CommandBuilder, so we can
 // be extremely type-sloppy here, which simplifies the signature.
@@ -95,6 +102,7 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type {
         allocator: std.mem.Allocator,
         subcommands: CommandMap,
         subcommand: ?ParserInterface = null,
+        error_message: std.ArrayListUnmanaged(u8) = .{},
         help_builder: help.HelpBuilder(command),
 
         // This is a slightly annoying hack to work around the fact that there's no way
@@ -106,13 +114,59 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type {
         // cognitively clutter this struct.
         pub usingnamespace InterfaceWrappers(@This());
 
+        pub fn execute(self: *@This(), context: UserContext) anyerror!void {
+            const args = std.process.argsAlloc(self.allocator) catch |err| {
+                try self.error_message.appendSlice(
+                    self.allocator,
+                    "Failed to allocate process arg vector\n",
+                );
+                return err;
+            };
+            const env = std.process.getEnvMap(self.allocator) catch |err| {
+                try self.error_message.appendSlice(
+                    self.allocator,
+                    "Failed to allocate process environment variable map\n",
+                );
+                return err;
+            };
+
+            if (args.len < 1) {
+                try self.error_message.appendSlice(
+                    self.allocator,
+                    "The argument list for the base CLI entry point is empty.\n",
+                );
+                return ParseError.EmptyArgs;
+            }
+
+            self.progname = std.fs.path.basename(args[0]);
+
+            {
+                var subc = try self.subparse(context, self.progname.?, args[1..], env);
+                while (subc) |next| {
+                    subc = next.parser.parse(next.name, next.args, env) catch |err| {
+                        try self.error_message.appendSlice(self.allocator, next.parser.getParseError());
+                        return err;
+                    };
+                }
+            }
+            {
+                var subc = try self.finish(context);
+                while (subc) |next| {
+                    subc = next.finish() catch |err| {
+                        try self.error_message.appendSlice(self.allocator, next.getParseError());
+                        return err;
+                    };
+                }
+            }
+        }
+
         pub fn subparse(
             self: *@This(),
             context: UserContext,
             name: []const u8,
             args: [][:0]u8,
             env: std.process.EnvMap,
-        ) anyerror!void {
+        ) anyerror!?ParseResult {
             const sliceto = try self.parse(name, args);
             try self.readEnvironment(env);
             try self.convertEager(context);
@@ -121,21 +175,30 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type {
                 const grafted_name = try std.mem.join(
                     self.allocator,
                     " ",
-                    &[_][]const u8{ name, args[sliceto - 1] },
+                    &.{ name, args[sliceto - 1] },
                 );
-                try subcommand.parse(grafted_name, args[sliceto..], env);
+                return .{ .name = grafted_name, .args = args[sliceto..], .parser = subcommand };
             } else if (self.subcommands.count() > 0 and command.subcommand_required) {
                 const stderr = std.io.getStdErr().writer();
-                try stderr.writeAll("A subcommand is required.\n\n");
+                try stderr.print("'{s}' requires a subcommand.\n\n", .{name});
 
                 self.printHelp(name);
             }
+
+            return null;
         }
 
-        pub fn finish(self: *@This(), context: UserContext) anyerror!void {
+        pub fn finish(self: *@This(), context: UserContext) anyerror!?ParserInterface {
             try self.convert(context);
             try callback(context, self.output);
-            if (self.subcommand) |subcommand| try subcommand.finish();
+            return self.subcommand;
+        }
+
+        pub fn getParseError(self: @This()) []const u8 {
+            return if (self.error_message.items.len == 0)
+                "An unexpected error occurred.\n"
+            else
+                self.error_message.items;
         }
 
         pub fn deinit(self: @This()) void {
@@ -158,18 +221,6 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type {
             return self.subcommands.get(name);
         }
 
-        pub fn execute(self: *@This(), context: UserContext) anyerror!void {
-            const args = try std.process.argsAlloc(self.allocator);
-            const env = try std.process.getEnvMap(self.allocator);
-
-            if (args.len < 1) return ParseError.EmptyArgs;
-
-            self.progname = std.fs.path.basename(args[0]);
-
-            try self.subparse(context, self.progname.?, args[1..], env);
-            try self.finish(context);
-        }
-
         fn printValue(self: @This(), value: anytype, comptime indent: []const u8) void {
             if (comptime @hasField(@TypeOf(value), "items")) {
                 std.debug.print("{s}[\n", .{indent});
@@ -279,6 +330,10 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type {
                 }
             }
 
+            try self.error_message.writer(self.allocator).print(
+                "Could not parse command line: unknown option \"{s}\"\n",
+                .{arg},
+            );
             return ParseError.UnknownLongTagParameter;
         }
 
@@ -301,14 +356,23 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type {
 
                 if (arg == tag[1]) {
                     if (comptime !PType.is_flag)
-                        if (remaining > 0)
+                        if (remaining > 0) {
+                            try self.error_message.writer(self.allocator).print(
+                                "Could not parse command line: \"-{c}\" is fused to another flag, but it requires a value\n",
+                                .{arg},
+                            );
                             return ParseError.FusedShortTagValueMissing;
+                        };
 
                     try self.applyParamValues(param, argit, false);
                     return;
                 }
             }
 
+            try self.error_message.writer(self.allocator).print(
+                "Could not parse command line: unknown option \"-{c}\"\n",
+                .{arg},
+            );
             return ParseError.UnknownShortTagParameter;
         }
 
@@ -335,7 +399,20 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type {
                 arg_index += 1;
             }
 
-            return self.subcommands.get(arg) orelse ParseError.ExtraValue;
+            return self.subcommands.get(arg) orelse {
+                const writer = self.error_message.writer(self.allocator);
+                if (self.subcommands.count() > 0)
+                    try writer.print(
+                        "Could not parse command line: unknown subcommand \"{s}\"\n",
+                        .{arg},
+                    )
+                else
+                    try writer.print(
+                        "Could not parse command line: unexpected extra argument \"{s}\"\n",
+                        .{arg},
+                    );
+                return ParseError.ExtraValue;
+            };
         }
 
         fn pushIntermediateValue(
@@ -350,7 +427,7 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type {
                 if (@field(self.intermediate, param.name) == null) {
                     @field(self.intermediate, param.name) = gen.IntermediateType().init(self.allocator);
                 }
-                @field(self.intermediate, param.name).?.append(value) catch return ParseError.UnexpectedFailure;
+                try @field(self.intermediate, param.name).?.append(value);
             } else if (comptime @TypeOf(param).G.nonscalar()) {
                 if (@field(self.intermediate, param.name)) |list| list.deinit();
                 @field(self.intermediate, param.name) = value;
@@ -369,19 +446,54 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type {
                 .flag => try self.pushIntermediateValue(param, comptime param.flag_bias.string()),
                 .count => @field(self.intermediate, param.name) += 1,
                 .fixed => |count| switch (count) {
-                    0 => return ParseError.ExtraValue,
-                    1 => try self.pushIntermediateValue(param, argit.next() orelse return ParseError.MissingValue),
+                    0 => {
+                        const writer = self.error_message.writer(self.allocator);
+                        const desc = try param.describe(self.allocator);
+                        defer self.allocator.free(desc);
+                        try writer.print(
+                            "Could not parse command line: {s} takes no value.\n",
+                            .{desc},
+                        );
+                        return ParseError.ExtraValue;
+                    },
+                    1 => try self.pushIntermediateValue(param, argit.next() orelse {
+                        const writer = self.error_message.writer(self.allocator);
+                        const desc = try param.describe(self.allocator);
+                        defer self.allocator.free(desc);
+                        try writer.print(
+                            "Could not parse command line: {s} requires a value.\n",
+                            .{desc},
+                        );
+                        return ParseError.MissingValue;
+                    }),
                     else => |total| {
-                        var list = std.ArrayList([:0]const u8).initCapacity(self.allocator, total) catch
-                            return ParseError.UnexpectedFailure;
+                        var list = try std.ArrayList([:0]const u8).initCapacity(self.allocator, total);
 
                         var consumed: u32 = 0;
                         while (consumed < total) : (consumed += 1) {
-                            const next = argit.next() orelse return ParseError.MissingValue;
+                            const next = argit.next() orelse {
+                                const writer = self.error_message.writer(self.allocator);
+                                const desc = try param.describe(self.allocator);
+                                defer self.allocator.free(desc);
+                                try writer.print(
+                                    "Could not parse command line: {s} is missing one or more values (need {d}, got {d}).\n",
+                                    .{ desc, total, consumed },
+                                );
+                                return ParseError.MissingValue;
+                            };
 
-                            list.append(next) catch return ParseError.UnexpectedFailure;
+                            list.appendAssumeCapacity(next);
+                        }
+                        if (bounded and argit.next() != null) {
+                            const writer = self.error_message.writer(self.allocator);
+                            const desc = try param.describe(self.allocator);
+                            defer self.allocator.free(desc);
+                            try writer.print(
+                                "Could not parse command line: {s} has too many values (need {d}).\n",
+                                .{ desc, total },
+                            );
+                            return ParseError.ExtraValue;
                         }
-                        if (bounded and argit.next() != null) return ParseError.ExtraValue;
 
                         try self.pushIntermediateValue(param, list);
                     },
@@ -403,8 +515,7 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type {
                 if (comptime param.env_var) |env_var| blk: {
                     if (@field(self.intermediate, param.name) != null) break :blk;
 
-                    const val = self.allocator.dupeZ(u8, env.get(env_var) orelse break :blk) catch
-                        return ParseError.UnexpectedFailure;
+                    const val = try self.allocator.dupeZ(u8, env.get(env_var) orelse break :blk);
 
                     if (comptime @TypeOf(param).G.value_count == .flag) {
                         try self.pushIntermediateValue(param, val);
@@ -438,19 +549,27 @@ pub fn Parser(comptime command: anytype, comptime callback: anytype) type {
 
                 if (comptime @TypeOf(param).has_output) {
                     @field(self.output, param.name) = param.converter(context, intermediate, writer) catch |err| {
-                        const stderr = std.io.getStdErr().writer();
-                        stderr.print("Error parsing option \"{s}\": {s}\n", .{ param.name, buffer.items }) catch {};
+                        const err_writer = self.error_message.writer(self.allocator);
+                        const desc = try param.describe(self.allocator);
+                        defer self.allocator.free(desc);
+                        try err_writer.print("Error parsing option {s}: {s}\n", .{ desc, buffer.items });
                         return err;
                     };
                 } else {
                     param.converter(context, intermediate, writer) catch |err| {
-                        const stderr = std.io.getStdErr().writer();
-                        stderr.print("Error parsing option \"{s}\": {s}\n", .{ param.name, buffer.items }) catch {};
+                        const err_writer = self.error_message.writer(self.allocator);
+                        const desc = try param.describe(self.allocator);
+                        defer self.allocator.free(desc);
+                        try err_writer.print("Error parsing option {s}: {s}\n", .{ desc, buffer.items });
                         return err;
                     };
                 }
             } else {
                 if (comptime param.required) {
+                    const err_writer = self.error_message.writer(self.allocator);
+                    const desc = try param.describe(self.allocator);
+                    defer self.allocator.free(desc);
+                    try err_writer.print("Could not parse command line: required parameter {s} is missing\n", .{desc});
                     return ParseError.RequiredParameterMissing;
                 } else if (comptime @TypeOf(param).has_output) {
                     if (comptime param.default) |def| {
@@ -497,18 +616,23 @@ fn InterfaceWrappers(comptime ParserType: type) type {
             name: []const u8,
             args: [][:0]u8,
             env: std.process.EnvMap,
-        ) anyerror!void {
+        ) anyerror!?ParseResult {
             const self = castInterfaceParser(parser);
             const context = self.castContext(ctx);
             return try self.subparse(context, name, args, env);
         }
 
-        fn _wrapFinish(parser: *anyopaque, ctx: *anyopaque) anyerror!void {
+        fn _wrapFinish(parser: *anyopaque, ctx: *anyopaque) anyerror!?ParserInterface {
             const self = castInterfaceParser(parser);
             const context = self.castContext(ctx);
             return try self.finish(context);
         }
 
+        fn _wrapGetParseError(parser: *anyopaque) []const u8 {
+            const self = castInterfaceParser(parser);
+            return self.getParseError();
+        }
+
         fn _wrapAddSubcommand(parser: *anyopaque, name: []const u8, subcommand: ParserInterface) !void {
             const self = castInterfaceParser(parser);
             return self.addSubcommand(name, subcommand);