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.
