Revamp how arguments after "--" are handled.

[jwhear]

I don't know how this PR aligns with your vision for the project, but we (the Bun team) make good use of zig-clap and so want to offer this change for your consideration.

Motivation

Node.js allows running scripts like so node --node-arg1 app.js --app-arg1 For Bun to provide similar functionality, we need to stop clap from parsing after the 1st positional (in this case, but it could be a different index in other invocations), and return all remaining arguments unparsed so that they can be passed through to the invoked script.

Alternatives Considered

I looked at using parseEx and providing a custom iterator that inserted a "--" arg at the correct position, but determining the correct position amounted to reimplementing a substantial portion of zig-clap's private implementation.

Change

Previous behavior: when clap/streaming saw the "--" argument, it processed all subsequent arguments as positional. New behavior: when clap/streaming sees "--", it stops parsing altogether, leaving the remaining arguments up to the caller.

This change in behavior is leveraged in the high-level interface:

• Both Result and ResultEx have a passthroughs field which now contains all remaining arguments after the parser finishes.
• Adds ParseOptions.passthrough_after_nth_positional which causes the streaming parser to stop after n positionals have been encountered; the remaining arguments are placed into passthroughs.

I chose to implement a separate passthroughs list instead of simply providing a slice into positionals because it seems more robust in the event that other mechanisms for not parsing are brought forward in the future (e.g. skip all arguments with a prefix).

API Breakage

• This PR has the major impact of breaking code which depends on arguments following "--" being in positionals as these are now in passthroughs.
• Code which leverages the streaming parser directly is now responsible for dealing with trailing unparsed arguments.

[Hejsil]

I see the use case, but I think the change is quite invasive and doesn't seem that general. I've tried playing with the idea using claps current features and here is what I came up with:

const clap = @import("clap");
const std = @import("std");

const debug = std.debug;
const io = std.io;

pub fn main() !void {
    var gba_state = std.heap.GeneralPurposeAllocator(.{}){};
    const gba = gba_state.allocator();
    defer _ = gba_state.deinit();

    const params = comptime clap.parseParamsComptime(
        \\--node-arg1
        \\<str>
        \\
    );

    // Get all arguments in a slice, so we can iterate over them multiple times
    const full_args = try std.process.argsAlloc(gba);
    defer gba.free(full_args);
    const args = full_args[1..];

    var it = clap.args.SliceIterator{ .args = args };
    var streaming_clap = clap.streaming.Clap(clap.Help, clap.args.SliceIterator){
        .params = &params,
        .iter = &it,
    };

    // First, use `clap.streaming.Clap` to find the first positional argument.
    var args_to_parse: usize = 0;
    while (try streaming_clap.next()) |arg| : (args_to_parse += 1) {
        if (arg.param.names.longest().kind == .positional) {
            args_to_parse += 1;
            break;
        }
    }

    // Next, pass only the args + first positional to `clap.parseEx`
    const args_rest = args[args_to_parse..];
    it = clap.args.SliceIterator{ .args = args[0..args_to_parse] };

    var diag = clap.Diagnostic{};
    var res = clap.parseEx(clap.Help, &params, clap.parsers.default, &it, .{
        .diagnostic = &diag,
    }) catch |err| {
        diag.report(io.getStdErr().writer(), err) catch {};
        return err;
    };
    defer res.deinit();

    if (res.args.@"node-arg1")
        debug.print("--node-arg1\n", .{});

    debug.print("pos: ", .{});
    for (res.positionals) |pos|
        debug.print("{s}, ", .{pos});

    debug.print("\nrest: ", .{});
    for (args_rest) |pos|
        debug.print("{s}, ", .{pos});
    debug.print("\n", .{});
}

$ ./node --node-arg1 app.js --app-arg1
--node-arg1
pos: app.js,
rest: --app-arg1,

[jwhear]

This seems like a reasonable workaround and I've successfully adapted it to our use case, so I'm closing this PR. One thing to note for others implementing this pattern in the future: you'll want to implement diagnostics and error handling for this bit:

    var streaming_clap = clap.streaming.Clap(clap.Help, clap.args.SliceIterator){
        .params = &params,
        .iter = &it,
+       .diagnostics = &diag,
    };

    // First, use `clap.streaming.Clap` to find the first positional argument.
    var args_to_parse: usize = 0;
+ // add error handling for the next() call, probably in an outer function
    while (try streaming_clap.next()) |arg| : (args_to_parse += 1) {

These changes are necessary to support good error reporting for the case node --not-an-arg app.js

[Hejsil]

One thing to note for others implementing this pattern in the future: you'll want to implement diagnostics and error handling for this bit

Yes, agreed. Mostly just through this together to show how I might have solved it. Happy that is a good solution for you need :+1: