-
Notifications
You must be signed in to change notification settings - Fork 152
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
what is the purpose of special-casing u64? #30
Comments
That's a very common pattern. You find it a lot with verbose option. -v for verbose, -vv for very verbose, -vvv for extremely verbose. That would be inconvenient to have to call .len() on a vector. If you want a u64, just do that: #[structopt(short = "i", parse(try_from_str))]
integer: u64, |
Ah, it wasn't clear to me from the documentation that "number of params" means the number of repetitions of that parameter, rather than the number of remaining parameters. Maybe that could be clarified. And does it really make sense for it to be a u64? Why not a u8 or u16? Any command-line interface that expects the user to repeat a parameter more than 255 times seems like it must be poorly designed. Anyway, thanks for structopt! I like it very much. |
That's a u64 because the corresponding method in clap returns a u64. I reopen the issue for the documentation clarification. I'll do it when I'll have the time. Feel free to open a PR if you are inspired. Thanks for the feedback, that's always motivating to know that what we do is useful. |
First thanks for structopt! I am new to it but I like it already. When I read the doc I was also confused by the special handling of Maybe such info can be added to doc:
|
Looks like that's not yet clear enough. I'll try to improve that. |
Wowwww this just bit me so hard. Considering that this is treated specially and switching from a u64 to a u32 completely changes the semantics of the command line parser i sincerely wish that you’d remove this, as it definitely violates the principle of least surprise in multiple ways, etc. This is not a user friendly feature at all. For something that drastically modifies the behavior like this it should definitely be typed, like adding a thin type wrapper like Repetition(u64). Also repurposing an (arbitrary) type to do something like reputation of arguments, which quite frankly is not common at all (I’ve never done this and I’ve written I don’t know how many command line tools using this crate at this point), and which is very likely only ever used with something like verbosity (if it’s used at all) just seems odd. If the underlying crate is to blame there is no reason to repeat that mistake, and is easily rectified by a type wrapper, etc. |
I agree. I've written 18 tools using structopt and none of them count repeated arguments. Also I can't remember the last time I used repeated arguments in any command. A type wrapper or an attribute would work just as well and avoid this pitfall. |
This was rather surprising, given that a field with the type u32 works the way you'd expect, while switching to u64 suddenly doesn't. It's feels rather unintuitive. A solution using Repetition(u64) as suggested by m4b would probably be better. |
I, too, wasted time because of this. It's incredibly surprising, and I skipped right past it in the documentation because I really wouldn't have expected this behavior. I also recommend making |
That will be I can't find the time to do that, but I hope to do it soon. |
@TeXitoi No worries! I find value in |
Isn't using Anyway special-casing I see this is already being addressed. |
I want to have parameter values parsed into
u64
s, but they get special treatment instead. Apparently au64
captures all remaining parameters and is set to the number of remaining parameters? How is that useful if the values aren't accessible? Wouldn't it make more sense for users to just use aVec<String>
here and calllen()
when they want the number of parameters?The text was updated successfully, but these errors were encountered: