core: address review comments

- break formatting values out into a function so that we can
  catch errors and never fail
- eliminate the use of toOwnedSentinelSlice since we are using
  an arena to clean up memory

src/cli/args.zig

@@ -132,30 +132,8 @@ pub fn parse(
                 // track more error messages.
                 error.OutOfMemory => return err,
                 error.InvalidField => "unknown field",
-                error.ValueRequired => "value required",
+                error.ValueRequired => formatValueRequired(T, arena_alloc, key) catch "value required",
-                error.InvalidValue => msg: {
+                error.InvalidValue => formatInvalidValue(T, arena_alloc, key, value) catch "invalid value",
-                    var buf = std.ArrayList(u8).init(arena_alloc);
-                    errdefer buf.deinit();
-                    const writer = buf.writer();
-                    try writer.print("invalid value \"{?s}\"", .{value});
-                    const typeinfo = @typeInfo(T);
-                    inline for (typeinfo.Struct.fields) |f| {
-                        if (std.mem.eql(u8, key, f.name)) {
-                            switch (@typeInfo(f.type)) {
-                                .Enum => |e| {
-                                    try writer.print(", valid values are: ", .{});
-                                    inline for (e.fields, 0..) |field, i| {
-                                        if (i != 0) try writer.print(", ", .{});
-                                        try writer.print("{s}", .{field.name});
-                                    }
-                                },
-                                else => {},
-                            }
-                            break;
-                        }
-                    }
-                    break :msg try buf.toOwnedSliceSentinel(0);
-                },
                 else => try std.fmt.allocPrintZ(
                     arena_alloc,
                     "unknown error {}",
@@ -173,6 +151,54 @@ pub fn parse(
     }
 }
 
+fn formatValueRequired(
+    comptime T: type,
+    arena_alloc: std.mem.Allocator,
+    key: []const u8,
+) std.mem.Allocator.Error![:0]const u8 {
+    var buf = std.ArrayList(u8).init(arena_alloc);
+    errdefer buf.deinit();
+    const writer = buf.writer();
+    try writer.print("value required", .{});
+    try formatValues(T, key, writer);
+    try writer.writeByte(0);
+    return buf.items[0 .. buf.items.len - 1 :0];
+}
+
+fn formatInvalidValue(
+    comptime T: type,
+    arena_alloc: std.mem.Allocator,
+    key: []const u8,
+    value: ?[]const u8,
+) std.mem.Allocator.Error![:0]const u8 {
+    var buf = std.ArrayList(u8).init(arena_alloc);
+    errdefer buf.deinit();
+    const writer = buf.writer();
+    try writer.print("invalid value \"{?s}\"", .{value});
+    try formatValues(T, key, writer);
+    try writer.writeByte(0);
+    return buf.items[0 .. buf.items.len - 1 :0];
+}
+
+fn formatValues(comptime T: type, key: []const u8, writer: anytype) std.mem.Allocator.Error!void {
+    const typeinfo = @typeInfo(T);
+    inline for (typeinfo.Struct.fields) |f| {
+        if (std.mem.eql(u8, key, f.name)) {
+            switch (@typeInfo(f.type)) {
+                .Enum => |e| {
+                    try writer.print(", valid values are: ", .{});
+                    inline for (e.fields, 0..) |field, i| {
+                        if (i != 0) try writer.print(", ", .{});
+                        try writer.print("{s}", .{field.name});
+                    }
+                },
+                else => {},
+            }
+            break;
+        }
+    }
+}
+
 /// Returns true if this type can track diagnostics.
 fn canTrackDiags(comptime T: type) bool {
     return @hasField(T, "_diagnostics");