c-blake / cligen

Nim library to infer/generate command-line-interfaces / option / argument parsing; Docs at
https://c-blake.github.io/cligen/
ISC License
501 stars 23 forks source link

Feature Request: Required arguments without prefix #161

Closed JackSabbath closed 3 years ago

JackSabbath commented 3 years ago

I'm a bit puzzled that a prefix (a - and at least a parameter letter) is always required except for the subcommand and the variable length arguments. Let's assume I'm having a proc that requires exactly two required arguments like this:

proc befriend(friendA, friendB: string)

I'd be happy if cligen would accept the following program call (using subcommand syntax):

myApp befriend Anna Ben (instead of myApp befriend --friendA=Anna --friendB=Ben.. so without a prefix that names the parameter)

That's currently only possible using variable length arguments and I think that's no good, because it passes the responsibility to check the arguments validity to the framework user. Yet, the user actually uses the framework to not have to do that.

So what I'd like the cligen framework to is:

Implementing that would also solve this issue. It's a common pattern to just pass arguments without telling their names, but by matching their position, so I think cligen should support that.

c-blake commented 3 years ago

This request comes up from time to time. When I first wrote cligen I also thought what you stated as the "natural binding". Then someone came by with a bunch of feature requests and https://github.com/c-blake/cligen/issues/20 was in the first batch (he later asked for that feature back, sorta). It's ambiguous since "in Nim" you can also say app(friendB="Anna", friendA="Ben"). That ambiguity probably means no default can make everyone happy. I'm past 1.0 now and probably shouldn't change too much.

So, the 2nd item in TODO.md is actually to maybe revive that somehow in a backward compatible way. I'm happy to try to guide a PR process if you want to work on it. I realize that's not what you were asking for, but I suspect the identifier situation strongly influences what CLauthors want to happen.

Your example with long identifiers distinguished only at the end prevents "shortest unique prefix" from helping much and is kind of the worst case for CLusers. It also makes only one param get the "automatic short" option. So, for today you should probably pass short = {"friendA": 'a', "friendB": 'b'} to dispatch & friends in these circumstances. Then CLusers can just type myApp -aAnna -bBen. If you're good enough with that and/or don't want to do a PR we should probably close this issue.

SolitudeSF commented 3 years ago

cligen automatically treats the first non-defaulted seq[T] proc parameter as such an optional sequence.

maybe if it encounters array[N, T] first, then it would act as nonoptional arguments?

c-blake commented 3 years ago

Hey, @SolitudeSF. Thanks for chiming in. I think @JackSabbath and others want the real proc signature variables in there, not an array they need to manage. So, really they want a different CL <-> proc signature binding. So, in the TODO/https://github.com/c-blake/cligen/issues/62, the idea was to have dispatch take a list of identifiers like dispatch(myApp, positional = [ "friendA", "friendB" ]..(or maybe positionals if we really need a distinct named parameter). Without this or with positional=someParam, all would just work as it has for a very long time. So the only visible change to feature non-users is not failing on a new (kind of arg|plural arg). At the CLauthor level we just need to know which are positional or not. The order inside the proc signature would probably be the enforced CL arg order, though I guess we could have a permutation array. Gack. And the parser generated by dispatchGen will get yet more complex.

Also, @JackSabbath using seq[int] or seq[float] or seq[enum], etc. all do validate data already today, though this wasn't shown in https://github.com/c-blake/cligen/issues/160 and does require your likely few args to be the same Nim type. Maybe you already knew. You do still need to check the length.

There are some other ideas for CL positionals in TODO.md I would likely work on first, personally, but I'm not so against this as to reject good PRs or anything.

c-blake commented 3 years ago

Also, in @JackSabbath befriend example specifically, it maybe bears mentioning that there is no validation to be done. Any string is valid for a string parameter either as a seq[string] or the proposed proc befriend(param: string). So, if the fixed & variable params are the same type (string or otherwise) you only really have to check the len and do a few lets against few-by-practicality seq indexes which isn't so much code. Admittedly, it's a very special type-homogeneous case which why I first answered about all the general stuff, but I'd be remiss to not mention it works in that use case.

JackSabbath commented 3 years ago

@c-blake Generally I would love to implement that. An configuration option as you described sounds like a good and backwards compatible solution. However, I'm afraid I'm the wrong person for that, since I'm really new to nim and looking at the code of cligen I have absolutely no idea what's going on where. It's quite impressive and you're a wizard in my eyes.

It's ambiguous since "in Nim" you can also say app(friendB="Anna", friendA="Ben").

I don't see any ambiguity here. In my example those parameters are required, which most likely means that it's impossible to even guess a reasonable default value. Any call that doesn't include them is incorrect. In your example those parameters are optional and the app user can skip them completely. It's very reasonable to expect the user to specify which default parameter he wants to alter by using its prefix, but it feels unnecessarily verbose to specify that I want to pass a parameter that has to be provided anyway.

Also, @JackSabbath using seq[int] or seq[float] or seq[enum], etc. all do validate data already today, though this wasn't shown in #160 and does require your likely few args to be the same Nim type.

Yes, that would require all required arguments to be of the same nim type, though. If I need an int and an enum that quickly becomes complicated.

@SolitudeSF Yes, @c-blake perfectly described my intention.

c-blake commented 3 years ago

I don't see any ambiguity here. In my example those parameters are required

Sorry. I meant the call site, not the definition site for app(friendB="Anna", ..). With no default values, you can still call by name in Nim, in any order or by position. Apologies for the unclearness. So, the ambiguity is the correspondence between Nim calls and CLI calls inferred from the Nim calls.

Maybe the best correspondence would be to have both in the CLI simultaneously, too, with various controls. That might confuse CLI users, though, as it's really non-standard to be able to "optionally drop the --param= only if you enter things in a specific order". It's hard to even imagine a usage message I think would make that immediately apparent. ;) OTOH, it is kinda nicely flexible. So, you know..tradeoffs. Similarity to the Nim calls (CLauthor expectations) vs. CLuser expectations vs. giving CLusers brand new hopes & dreams. :)

Anyway, it's too bad you cannot help. Maybe someday. It's been an item for a loooong time. I think it's do-able, but more than a few hours of work to do it all and I have some other Nim projects I should give priority to and those other positional features that I think are maybe higher value (subjective, I know). I may not even get to it in 2020. So, I'm going to close this issue for now, but refer back to it in the github commits when/if I get to it.