Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for required parameter/argument #82

Open
lkadalski opened this issue Nov 7, 2022 · 7 comments
Open

Support for required parameter/argument #82

lkadalski opened this issue Nov 7, 2022 · 7 comments

Comments

@lkadalski
Copy link

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
Copy link
Owner

Hejsil commented Nov 7, 2022

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
Copy link
Author

lkadalski commented Nov 7, 2022

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
Copy link
Owner

Hejsil commented Nov 7, 2022

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
Copy link
Author

lkadalski commented Nov 7, 2022

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
Copy link

voroskoi commented Nov 1, 2023

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
Copy link
Owner

Hejsil commented Nov 1, 2023

@voroskoi Yep, that looks good to me 👍

@Hejsil
Copy link
Owner

Hejsil commented Nov 3, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants