-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
multiple=True for an option-type argument also sets max_values > 1? #415
Comments
@birkenfeld This is expected behavior and should be corrected in the documentation if it's unclear, which sadly is hard to tell from my optic because I'm familiar with all the internals so I take a lot of that knowledge for granted. Just to ensure we're speaking about the same thing. You'd like Explanation below. Bare with me here, explaining multiples gets hairy :) First, just to make sure we're understanding the same things I'll regress back a bit for terminology sake. When setting an option to What I think is the confusing part is that there are two ways to specify multiple values for an option. The way I believe you're thinking of, where each occurrence of that option is followed by a single (and only a single) value, such as
When you tell
So you might ask, what's the difference between
Hopefully this is more clear, please let me know if it's not, or where you found the documentation lacking. If everything I explained was already understood and you mean something different let me know, cause then it may be a bug. Also, if something I said is instead wrong (logically, not programatically) please let me know and we can find a way to correct it. Thanks for taking the time to file this! |
This did in fact point out a bug! Everything I said above is still true, just there is a current bug not respecting |
i.e. assume, option -O set to multiple, and number_of_values(1) set. And assume a *required* positional argument is also set. -O some -O other pos Would fail with pos arg not supplied. Relates to #415
#417 fixes the issue I was talking about...once that merges everything I said above will be for real 😉 |
Ah, so I should have been using |
Yeah looking back, this is one other slightly breaking change I wish I'd made during the bump to 2.x because I agree the largest source of confusion is that it's not 1 by default (well except those that expect that). But going the other way around (opt in to > 1) has less potential for confusion. |
Also #417 is about to merge, so I'll put out 2.0.5 right after that. |
2.0.5 is out on crates.io. I'm going to leave this issue open until I re-do the docs for these parts. |
Here is a small demo for the issue. Take this code:
which should run fine with arguments
-O x pos
.When you uncomment the
multiple(true)
, it should behave the same (I assume from the docs), but instead it complains that the positional arg is not given, which leads me to believe that the option now accepts multiple values. And indeed, also uncommenting themax_values(1)
makes it behave again.I hope this isn't expected behavior -- max_values > 1 arguments are thoroughly confusing in my opinion.
The text was updated successfully, but these errors were encountered: