Before, if we were in trailing values that aren't delimite, we wouldn't
respect this flag and end processing of the value, now we do.
This also has a slight perf benefit of us only splitting the value if
the delimiter is present. We checked for the delimiter anyways, so
doing it first removes a slight bit of work.
I also feel this helps clarify the intended behavior and ooches us
towards a unified code path for actions.
I wrote these tests expecting to highlight a bug but it turns out things
were structured just right to not exhibit it. The fact that the code
looks like its broken is a problem, so I restructured it (put it first,
changed the source) so it doesn't look suspicious anymore.
We were independently starting occurrences and starting value groups.
Now we do them at the same time.
COMPATIBILITY: This changes us from counting occurrences per positional
when using `multiple_values` to one occurrence. This is user visible
and tests were written against it but it goes against the documentation
and doesn't quite make sense.
Inferred flags can make it hard for a future action to trigger behavior
off of the selected alias, like we might want to do for negations, so we
are now translating to the intended arg.
This will also help for debugging.
For most users, this won't be worth doing, they can just specify the
parser if needed. Where this has value is crates that integrate custom
types into clap, like creating click-like file integration. See
https://click.palletsprojects.com/en/8.0.x/arguments/#file-arguments
Clap has focused on reporting development errors through assertions
rather than mixing user errors with development errors. Sometimes,
developers need to handle things more flexibly so included in #3732 was
the reporting of value accessor failures as internal errors with a
distinct type. I've been going back and forth on whether the extra
error pessimises the usability in the common case vs dealing with the
proliferation of different function combinations. In working on
deprecating the `value_of` functions, I decided that it was going to be
worth duplicating so long as we can keep the documentation focused.
The remove functions no longer return `Arc` but the core type, at the
cost of requiring `Clone`. I originally held off on this
in #3732 in the hope of gracefully transition the derive and requiring
`Clone` would have been a breaking change but when it came to #3734, I didn't
find a way to make it work without a breaking change, so I made it
opt-in. This means I can force the `Clone` requirement now.
I added the requirement for `Clone` everywhere else in the hopes that in
the future, we can drop the `Arc` without a breaking change.
When to show usage? We are currently mixed about it. For `validator`,
we didn't show it at all. Sometimes we show the used arguments and
sometimes we don't.
With `ValueParser`, I ran into the problem that we weren't showing the
used arguments like we had previously in some cases. In deciding how to
solve this, I went with the simplest route for now and removed it as the
usage likely doesn't add much context to help people solve their
problem, more so the recommendation for help. We'll see how the
feedback is on this and adjust.
