In #2851, we moved color from an AppSetting to function (with some
tweaks in #2907). When doing this, we documented `App::color` to be
equivalent of `App::global_settings(Color...)` but never actually
propagated it.
We are now propagating it. A test is added to ensure that no matter
how we store the color choice, we continue to propagate it. This
required exposing `App::get_color`.
In experimenting on #1772, I want to write test cases for various
combinations of required or not, values vs occurrences, etc. There
wasn't really a clear place to put these.
On top of that, I wanted there to be a clear place in the tests for
describing the behavior of special types, to make it easier to audit and
easier to see how a PR for #1772 changes things.
As part of this effort in organizing these tests, I reduced the number
of tests that use special types. This better focuses these tests on the
cases they are intending to cover, rather than pulling in unrelated
features. This makes it easier to audit special types and makes it so
failures give more focused results, making it easier to see what broke.
In #2985, I noticed #2834 was incomplete, there were case-insensitive
comparisons we were doing without being unicode aware (when compile
options are set).
The downside is that each comparison will require a UTF-8 validation.
These seem to be in more of corners of the API, rather than in common
calls in common usages, so hopefully that isn't too much of a problem.
Similar to #2977, this changes positional argument `<subcmd>` in
`help <subcmd>` to be multiple occurrences, from being multiple values.
This is what identified the usage generation bug fixed in #2978 and was
isolated into the test case `positional_multiple_occurrences_is_dotted`.
This is part of #2692 where we re-evaluate the usage of multiple values
for positionals now that we accept multiple occurrences.
PR #2979 reduced what dependencies we push on users and made us one step
closer to being able to test more with fewer features. Looks like I
also need to update `clap_derive`. I verified locally that #2976 fails
with this.
`clap_derive` still needs `env` because of examples / tests that use it.
I feel like moving `clap_derive`s tests out to `clap` would be the way
to fix this.
The tests are using `to_string` which maps to `Display::fmt` and the
`Colorizer` version is colorless. To get color, it is only supported
with `print()` which has to go to stdout / stderr and can't be directed
to an arbitrary stream.
Though we store a lot of values as `&'help str`, we return them as
`&'self str`, making it so they can not be used programmatically as part
of a `App::mut_arg` call.
This loosens the lifetimes so they can be used with `App::mut_arg`.
This also includes a test simulating the desired workflow described in #2966
I skipped `get_all_aliases`. I ran into problems with lifetimes with
`all_subcommand_names` and didn't quickly resolve it. Rather than hold
this up, I punted on it for now.
We'll have to tighten these back up with #1041 but that will also enable
turning them into owned strings, so this will still be possible after
that issue is resolved, just the calls will be slightly different.
Like with #2903, we are depending on more features than needed, forcing
them onto users. This also impacts our testing since we aren't truly
getting minimal test runs (see #2976).
When supporting multiple occurrences for positional arguments in #2804,
I added some tests to cover this but apparently simpler cases fail
despite those more complicated cases being tested.
This adds more multiple-occurrences tests for positional arguments,
fixes them, and in general equates multiple values with occurrences for
positional arguments as part of #2692. There are a couple more points
for consideration for #2692 for us to decide on once this unblocks them
(usage special case in #2977 and how subcommand help should be handled).
I fully admit I have not fully quantified the impact of all of these
changes and am heavily relying on the quality of our tests to carry this
forward.
I noticed this while investigating #2692. Since we are making
multiple-occurrences a thing for positional arguments, this allows us to
remove a special case.
Another way to look at this is that we should make the default whatever
we do for dervies (#1772). I'm going to propose we make the derive
always turn `Vec<i32>` into multiple occurences and not multiple values
(with users being able to change it through attributes), but that is an
in-work proposal and not decided yet.
BREAKING CHANGE: `Arg::from(...)` will now use `multiple_occurrences`
for a positional `...`, rather than `multiple_values`.
In looking at multiple occurrences and values for issues like #2692, I
noticed that `...` can mean both multiple values and multiple
occurrences, like before we split them.
Pros
- No syntax change with clap3
Cons
- All the reasons we split `multiple` into two
Uncertain
- I originally started this as part of another branch but I lost track
if something depended on this. I'll have to do more digging
BREAKING CHANGE: If `--opt [val]...` was meant for
- only multiple occurrences, see `[opt]... --opt [val]`
- both multiple occurrences and values, see `[opt]... --opt [val]...`
Besides being more explicit / clear, this makes it easier to experiment
with tweaking the logic and keeping the test updates minimal, so the
change of behavior stands out more.
