Someone should not reasonably expect a coun flag to go up to billions,
millions, or even thousands. 255 should be sufficient for anyone,
right?
The original type was selected to be consistent with
`ArgMatches::occurrences_of` but that is also used for tracking how
many values appear which can be large with `xargs`.
I'm still conflicted on what the "right type" is an wish we could
support any numeric type. When I did a search on github though, every
case was for debug/quiet flags and only supported 2-3 occurrences,
making a `u8` overkill.
This came out of a discussion on #3792
We aren't enumerating arguments but values for an argument, so the name
should reflect that.
This will be important as part of #1807 when we have more specific
attribute names.
This mostly exist for
- Knowing of the value came from the command-line but we now have
`ArgMatches::source`
- Counting the number of flags but we now have `ArgAction::Count`
This shouldn't be needed anymore now that this is effectively the new
behavior for the non-deprecated actions.
This was briefly talked about in
https://github.com/clap-rs/clap/discussions/2627 but I wasn't familiar
enough with the implementation to know how safe it is. Now, maintainrs
and users can be more confident because they are explicitly opting into
it.
See also #3795
Dropping these will help simplify a lot, including removing of
occurrences.
These come at the cost of the derive not yet supporting types that impl
`From`.
This is the derive support for #3774 (see also #3775, #3777)
This combined with `value_parser` replaces `parser`. The main
frustration with this is that `ArgAction::Count` (the replacement for
`parse(from_occurrences)` must be a `u64`. We could come up with a
magic attribute that is meant to be the value parser's parsed type. We
could then use `TryFrom` to convert the parsed type to the user's type
to allow more. That is an exercise for the future. Alternatively, we
have #3792.
Prep for this included
- #3782
- #3783
- #3786
- #3789
- #3793
This is a follow up to #3420. Its easy to overlook this because it is only
useful for the conditionals (we actually prevent applying unconditional
defaults to unconditional requireds). This became apparent with the
increased use of defaults with `SetTrue`.
As always, there is the question of when is a bug fix a breaking change.
I'm going to consider this safe since we prevent some instances of this
from even happening and we already did #3420 and this is in line with
those.
This is mostly about avoiding criterion's build times when just
developing clap itself.
I'm assuming the derive test changed because criterion's clap v2 isn't
in the dependency tree anymore.
Actions were inspired by Python and Python does not implicitly default
any field when an action is given. From a Builder API perspective, this
seemed fine because we tend to focus the Builder API on giving the user
all information so they can make their own decisions. When working on
the Derive API, this became a problem because users were going to have
to migrate from an implied default to an explicit default when a common
default is good enough most of the time. This shouldn't interfere with
Builder users getting more details when needed.
This also highlighted two problems
- We set the index for defaults
- We don't debug_assert when applying conditional requirements with a
default present
This will make it easier to divide off parser logic for adding in
actions.
This does mean we can't provide error reporting on bad values with
`bool` but
- We should have also been doing that for `from_flag`
- We'll be dropping this soon in clap4 anyways
This round out the new style actions and allow us to start deprecating
occurrences.
As part of an effort to unify code paths, this does change flag parsing
to do splits. This will only be a problem if the user enables splits
but we'll at least not crash. Once we also address #3776, we'll be able
to have envs all work the same.
If we felt this was important long-term, we should fix this outside of
the Action. Since we might be changing up occurrences (#3772), we can
probably get away with a hack.
This changes how occurrences and values are grouped for multiple values.
Today, it appears as a bug. If we move forward with #3772, then this
can make sense.
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.
