Hejsil / zig-clap

Command line argument parsing library
MIT License
939 stars 67 forks source link

Support for required parameter/argument #82

Open lkadalski opened 1 year ago

lkadalski commented 1 year ago

Hi there, Great job with this library so far!

I'm missing required parameter functionality. Would it make sense for you to add it ? I see that we could (in rough words) extend this function

pub fn Param(comptime Id: type) type {
    return struct {
        id: Id,
        names: Names = Names{},
        takes_value: Values = .none,
    };
}

with a field .required: bool = false and extend Errors with something like this:

pub const Error = error{
    MissingValue,
    InvalidArgument,
    DoesntTakeValue,
    MissingRequiredParam
};

I think I can prepare PR with such change. This is only proposition of changes. What do you think ?

Hejsil commented 1 year ago

I'll expand on this a little. The current way you do this, is something like this:

    const option = args.args.option orelse fatal("Missing argument `--option`");

I think this is perfectly reasonable, but I do understand that if clap understood this concept, then the Diagnostic could report this error like other clap errors, and the args.args.option would not be an optional type.

If we really want this it needs to be fleshed out. How would one then specify that the parameter is required using the recommended way of constructing parameters? This is the currently recommended way if you need nothing advanced:

    const params = comptime clap.parseParamsComptime(
        \\-h, --help             Display this help and exit.
        \\-n, --number <usize>   An option parameter, which takes a value.
        \\-s, --string <str>...  An option parameter which can be specified multiple times.
        \\<str>...
        \\
    );
lkadalski commented 1 year ago

Great question @Hejsil. I see we need something solid here. I'll light some examples, which would you prefer?

        \\-n, --number <usize> (r) An required parameter, which takes a value.
        \\-n, --number <usize> [r] An required parameter, which takes a value.
        \\-n, --number <usize> [required] An required parameter, which takes a value.
        \\-n, --number <usize> (required) An required parameter, which takes a value.
        \\-n, --number <usize> (R) An required parameter, which takes a value.
        \\-n, --number <usize> <required> An required parameter, which takes a value.
        \\<str> ...
        \\
    );

On the other hand maybe we could invert it?(breaking)

        \\-n, --number <usize> (optional) An option parameter, which takes a value.
        \\-n, --number <usize> [o] An option parameter, which takes a value.
        \\-n, --number <usize> [optional] An option parameter, which takes a value.
.....
    );

Side note: I see that currently in clap-rs there is a pattern where required arguments are marked as <ARGUMENT> whereas optional are marked as [ARGUMENT](similar to clap.usage(...). But I can't find any good looking, non-intrusive alternative with current approach.

TBH I have no good answer for such change. Is there any variation you like? I would love to have possibility to generate such help response with just custom struct parsing. Is there some plan for such feature?

Hejsil commented 1 year ago

I'll light some examples, which would you prefer?

Geeh, don't really like any of them, but if we really wanted to pick one, then it should at least be the none breaking required one. Most program parameters tend to be optional, so let's make that the none friction path.

I would love to have possibility to generate such help response with just custom struct parsing. Is there some plan for such feature?

Are you referring to something similar to what zig-args does? Generate everything based on a struct? This is not something I have a need for, but if someone can come up with a good api for it and implements it, then I wouldn't be against it.

Also, I don't mind breaking changes if they are really an improvement over what was there before. Since Zig is not a stable language, I don't think it makes sense for libraries to care about stability yet either.

lkadalski commented 1 year ago

Are you referring to something similar to what zig-args does? Generate everything based on a struct? This is not something I have a need for, but if someone can come up with a good api for it and implements it, then I wouldn't be against it.

Yes, that's what I was looking for! Thanks @Hejsil I'll dig into this, maybe there is some common ground.

voroskoi commented 1 year ago

Hi,

I would go with

\\-s, --switch [bool]        An optional parameter without any value.
\\-n, --number [usize]    An optional parameter, which takes a value.
\\-n, --number <usize> A mandatory parameter, which takes a value.

And I would even go further with this:

\\-s, --switch [bool=false]        An optional parameter without any value. Off by default.
\\-n, --number [usize=33]    An optional parameter, which takes a value, 33 by default.
\\-n, --number <usize> A mandatory parameter, which takes a value. (Default value makes no sense)

I would like an OK, before I start working on it.

It would be a breaking change, but [optional] and is the way everybody does this.

Hejsil commented 1 year ago

@voroskoi Yep, that looks good to me :+1:

Hejsil commented 12 months ago

Aah, ok that accept might have been a little hasty. Looking into what tends to be done here I've found out the following.

Programs tends to define Usage and Options when running --help. Options tend to only describe the options available, but nothing about whether they are required or not.

Usage describes optional/required things. So if we had Usage: program [OPTIONS] then all options are optional, but Usage: program --option <v> [OPTIONS] specifies that --option is required. So only in Usage does the [] specify if something is optional.

Also, thinking more about it, I really don't want to break anything existing here. Most arguments will be optional, so breaking the <> syntax for something that is rarely used seems quite weird.