cli: skip argv0 and actions when parsing CLI flags

This fixes a regression from #2454. In that PR, we added an error when
positional arguments are detected. I believe that's correct, but we
were silently relying on the previous behavior in the CLI commands.

This commit changes the CLI commands to use a new argsIterator function
that creates an iterator that skips the first argument (argv0). This is
the same behavior that the config parsing does and now uses this shared
logic.

This also makes it so the argsIterator ignores actions (`+things`)
and we document that we expect those to be handled earlier.

src/App.zig

@@ -238,7 +238,7 @@ fn drainMailbox(self: *App, rt_app: *apprt.App) !void {
             // If we're quitting, then we set the quit flag and stop
             // draining the mailbox immediately. This lets us defer
             // mailbox processing to the next tick so that the apprt
-            // can try to quick as quickly as possible.
+            // can try to quit as quickly as possible.
             .quit => {
                 log.info("quit message received, short circuiting mailbox drain", .{});
                 self.setQuit();

src/cli/args.zig

@@ -4,6 +4,7 @@ const assert = std.debug.assert;
 const Allocator = mem.Allocator;
 const ArenaAllocator = std.heap.ArenaAllocator;
 const diags = @import("diagnostics.zig");
+const internal_os = @import("../os/main.zig");
 const Diagnostic = diags.Diagnostic;
 const DiagnosticList = diags.DiagnosticList;
 
@@ -894,6 +895,9 @@ test "parseIntoField: tagged union missing tag" {
 /// An iterator that considers its location to be CLI args. It
 /// iterates through an underlying iterator and increments a counter
 /// to track the current CLI arg index.
+///
+/// This also ignores any argument that starts with `+`. It assumes that
+/// actions were parsed out before this iterator was created.
 pub fn ArgsIterator(comptime Iterator: type) type {
     return struct {
         const Self = @This();
@@ -906,9 +910,21 @@ pub fn ArgsIterator(comptime Iterator: type) type {
         /// values yet.
         index: usize = 0,
 
+        pub fn deinit(self: *Self) void {
+            if (@hasDecl(Iterator, "deinit")) {
+                self.iterator.deinit();
+            }
+        }
+
         pub fn next(self: *Self) ?[]const u8 {
             const value = self.iterator.next() orelse return null;
             self.index += 1;
+
+            // We ignore any argument that starts with "+". This is used
+            // to indicate actions and are expected to be parsed out before
+            // this iterator is created.
+            if (value.len > 0 and value[0] == '+') return self.next();
+
             return value;
         }
 
@@ -919,6 +935,31 @@ pub fn ArgsIterator(comptime Iterator: type) type {
     };
 }
 
+/// Create an args iterator for the process args. This will skip argv0.
+pub fn argsIterator(alloc_gpa: Allocator) internal_os.args.ArgIterator.InitError!ArgsIterator(internal_os.args.ArgIterator) {
+    var iter = try internal_os.args.iterator(alloc_gpa);
+    errdefer iter.deinit();
+    _ = iter.next(); // skip argv0
+    return .{ .iterator = iter };
+}
+
+test "ArgsIterator" {
+    const testing = std.testing;
+
+    const child = try std.process.ArgIteratorGeneral(.{}).init(
+        testing.allocator,
+        "--what +list-things --a=42",
+    );
+    const Iter = ArgsIterator(@TypeOf(child));
+    var iter: Iter = .{ .iterator = child };
+    defer iter.deinit();
+
+    try testing.expectEqualStrings("--what", iter.next().?);
+    try testing.expectEqualStrings("--a=42", iter.next().?);
+    try testing.expectEqual(@as(?[]const u8, null), iter.next());
+    try testing.expectEqual(@as(?[]const u8, null), iter.next());
+}
+
 /// Returns an iterator (implements "next") that reads CLI args by line.
 /// Each CLI arg is expected to be a single line. This is used to implement
 /// configuration files.

src/cli/crash_report.zig

@@ -33,9 +33,9 @@ pub fn run(alloc_gpa: Allocator) !u8 {
     defer opts.deinit();
 
     {
-        var iter = try std.process.argsWithAllocator(alloc);
+        var iter = try args.argsIterator(alloc_gpa);
         defer iter.deinit();
-        try args.parse(Options, alloc, &opts, &iter);
+        try args.parse(Options, alloc_gpa, &opts, &iter);
     }
 
     const crash_dir = try crash.defaultDir(alloc);

src/cli/list_actions.zig

@@ -29,7 +29,7 @@ pub fn run(alloc: Allocator) !u8 {
     defer opts.deinit();
 
     {
-        var iter = try std.process.argsWithAllocator(alloc);
+        var iter = try args.argsIterator(alloc);
         defer iter.deinit();
         try args.parse(Options, alloc, &opts, &iter);
     }

src/cli/list_colors.zig

@@ -22,7 +22,7 @@ pub fn run(alloc: std.mem.Allocator) !u8 {
     defer opts.deinit();
 
     {
-        var iter = try std.process.argsWithAllocator(alloc);
+        var iter = try args.argsIterator(alloc);
         defer iter.deinit();
         try args.parse(Options, alloc, &opts, &iter);
     }

src/cli/list_fonts.zig

@@ -53,7 +53,7 @@ pub const Config = struct {
 /// specific styles. It is not guaranteed that only those styles are returned,
 /// it will just prioritize fonts that match those styles.
 pub fn run(alloc: Allocator) !u8 {
-    var iter = try std.process.argsWithAllocator(alloc);
+    var iter = try args.argsIterator(alloc);
     defer iter.deinit();
     return try runArgs(alloc, &iter);
 }

src/cli/list_keybinds.zig

@@ -52,7 +52,7 @@ pub fn run(alloc: Allocator) !u8 {
     defer opts.deinit();
 
     {
-        var iter = try std.process.argsWithAllocator(alloc);
+        var iter = try args.argsIterator(alloc);
         defer iter.deinit();
         try args.parse(Options, alloc, &opts, &iter);
     }

src/cli/list_themes.zig

@@ -91,7 +91,7 @@ pub fn run(gpa_alloc: std.mem.Allocator) !u8 {
     defer opts.deinit();
 
     {
-        var iter = try std.process.argsWithAllocator(gpa_alloc);
+        var iter = try args.argsIterator(gpa_alloc);
         defer iter.deinit();
         try args.parse(Options, gpa_alloc, &opts, &iter);
     }

src/cli/show_config.zig

@@ -60,7 +60,7 @@ pub fn run(alloc: Allocator) !u8 {
     defer opts.deinit();
 
     {
-        var iter = try std.process.argsWithAllocator(alloc);
+        var iter = try args.argsIterator(alloc);
         defer iter.deinit();
         try args.parse(Options, alloc, &opts, &iter);
     }

src/cli/validate_config.zig

@@ -32,7 +32,7 @@ pub fn run(alloc: std.mem.Allocator) !u8 {
     defer opts.deinit();
 
     {
-        var iter = try std.process.argsWithAllocator(alloc);
+        var iter = try args.argsIterator(alloc);
         defer iter.deinit();
         try args.parse(Options, alloc, &opts, &iter);
     }
@@ -46,7 +46,6 @@ pub fn run(alloc: std.mem.Allocator) !u8 {
     if (opts.@"config-file") |config_path| {
         var buf: [std.fs.max_path_bytes]u8 = undefined;
         const abs_path = try std.fs.cwd().realpath(config_path, &buf);
-
         try cfg.loadFile(alloc, abs_path);
         try cfg.loadRecursiveFiles(alloc);
     } else {

src/config/Config.zig

@@ -2365,18 +2365,10 @@ pub fn loadCliArgs(self: *Config, alloc_gpa: Allocator) !void {
         counter[i] = @field(self, field).list.items.len;
     }
 
-    // Initialize our CLI iterator. The first argument is always assumed
-    // to be the program name so we skip over that.
-    var iter = try internal_os.args.iterator(alloc_gpa);
+    // Initialize our CLI iterator.
+    var iter = try cli.args.argsIterator(alloc_gpa);
     defer iter.deinit();
-    if (iter.next()) |argv0| log.debug("skipping argv0 value={s}", .{argv0});
-
-    // Parse the config from the CLI args
-    {
-        const ArgsIter = cli.args.ArgsIterator(@TypeOf(iter));
-        var args_iter: ArgsIter = .{ .iterator = iter };
-        try self.loadIter(alloc_gpa, &args_iter);
-    }
+    try self.loadIter(alloc_gpa, &iter);
 
     // If we are not loading the default files, then we need to
     // replay the steps up to this point so that we can rebuild