Commit graph

245 commits

Author SHA1 Message Date
Ed Page
befee6667b docs: Re-work examples
This creates distinct tutorial examples from complex feature examples
(more how-tos).  Both sets are getting builder / derive versions (at
least the critical ones).
2021-11-30 21:33:52 -06:00
Ed Page
b190a6a817 test: Consolidate clap tests
This reduces the need for us to have `clap` as a dependency in
`clap_derive`, preparing the way to fix #15.
2021-11-30 10:07:08 -06:00
Ed Page
54228ef7d7 docs: Prefer global_setting
I've been finding I've been setting `AppSettings` without it which is
likely leading to bugs.  This tries to raise the visibility by using it
based on the setting being used and not whether the application needs
it.
2021-11-29 16:10:54 -06:00
Ed Page
1cdb2250e8 fix(derive): Define multiple policy for Special Types
Before:
- `bool`: a flag
- `Option<_>`: not required
- `Option<Option<_>>` is not required and when it is present, the value
  is not required
- `Vec<_>`: multiple values, optional
- `Option<Vec<_>>`: multiple values, min values of 0, optional

After:
- `bool`: a flag
- `Option<_>`: not required
- `Option<Option<_>>` is not required and when it is present, the value
  is not required
- `Vec<_>`: multiple occurrences, optional
  - optional: `Vec` implies 0 or more, so should not imply required
- `Option<Vec<_>>`: multiple occurrences, optional
  - optional: Use over `Vec` to detect when no option being present when
    using multiple values

Motivations:

My priorities were:
1. Are we getting in the users way?
2. Does the API make sense?
3. Does the API encourage best practices?

I was originally concerned about the lack of composability with
`Option<Option<_>>` and `Option<Vec<_>>` (and eventually `Vec<Vec<_>>`).
It prescribes special meaning to each type depending on where it shows
up, rather than providing a single meaning for a type generally.  You
then can't do things like have `Option<_>` mean "required argument with
optional value" without hand constructing it.  However, in practice the
outer type correlates with the argument occurrence and the inner type
with the value.   It is rare to want the value behavior without also the
occurrence behavior.  So I figure it is probably fine as long as people
can set the flags to manually get the behavior they want.

`Vec<_>` implies multiple occurrences, rather than multiple values.
Anecdotally, whenever I've used the old `Arg::multiple`, I thought I was
getting `Arg::multiple_occurrences` only.  `Arg::multiple_values`,
without any bounds or delimiter requirement, can lead to a confusing
user experience and isn't a good default for these.  On top of that, if
someone does have an unbounded or a delimiter multiple values, they are
probably also using multiple occurrences.

`Vec<_>` is optional because a `Vec` implies 0 or more, so we stick to
the meaning of the rust type.  At least for me, I also rarely need a
required with multiple occurrences argument but more often need optional
with multiple occurrences.

`Option<Vec<_>>` ends up matching `Vec<_>` which can raise the question
of why have it.  Some users might prefer the type.  Otherwise, this is
so users can detect whether the argument is present or not when using
`min_values(0)`.  Rather than defining an entire policy around this and
having users customize it, or setting `min_values(0)` without the rest
of a default policy, this gives people a blank slate to work from.

Another design option would have been to not infer any special-type
settings if someone sets a handful of settings manually, which would
have avoided the confusion in Issue clap-rs/clap 2599 but I see that
being confusing (for someone who knows the default, they will be
expecting it to be additive; which flags disable inferred settings?) and
brittle (as flags are added or changed, how do we ensure we keep this
up?).

Tests were added to ensure we support people customizing the behavior to
match their needs.

This is not solving:
- `Vec<Vec<_>>`, see clap-rs/clap 2924
- `(T1, T2)`, `Vec<(T1, T2)>`, etc, see clap-rs/clap 1717
- `Vec<Option<_>>` and many other potential combinations

Fixes clap-rs/clap 1772
Fixes clap-rs/clap 2599
See also clap-rs/clap 2195
2021-11-29 13:16:12 -06:00
Ed Page
62d7a3a928 fix: Be consistent on hide/hidden
- `PossibleValue::hidden` -> `PossibleValue::hide` (new in clap3, no
  breakin change)

Fixes #33
2021-11-29 10:58:00 -06:00
Ed Page
57e5fc2b07 fix: Rename to Arg::ignore_case like ArgSettings
Fixes #32
2021-11-29 10:34:42 -06:00
Ed Page
eae3ffb599 docs: Emphasize Arg over ArgSettings 2021-11-29 09:54:04 -06:00
Ed Page
c15a0bf5b3 fix: Rename App's AllowLeadingHyphen to AllowHyphenValues
This aligns us with `Arg::allow_hyphen_values` in clap2.
2021-11-24 11:25:48 -06:00
Ed Page
7e899cd340 Revert "Deprecate Arg::help in favour of Arg::about"
This reverts commits 24cb8b1..d0abb37 from clap-rs/clap#1840

This is part of #16.  clap-rs/clap#1840 wasn't the right call but we
don't have time to make the decision now, so instead of having one
option and changing it in 4.0, this reverts back to clap2 behavior.
2021-11-18 12:25:49 -06:00
kuviman
aa6594b334 feat(derive): Generic support
This is a port of clap-rs/clap#3023
2021-11-17 15:33:59 -06:00
Ed Page
4479282b11 style: Address warnings 2021-11-17 14:23:32 -06:00
Ed Page
9167fdebf3 fix(derive): Don't emit warnings
Looks like this is coming from `update_from_arg_matches` where we do a
ladder of `if __clap_arg_matches.is_present(...)` that clippy wants to
be `else if`s.  While for human edited code, that does clarify intent,
for machine generated code that is rarely read, its a pain to do, so
silencing it.

Unfortunately, it isn't in a group we can overall silence.

Fixes #3017
2021-11-12 12:52:18 -06:00
Ed Page
78e4c90326 refactor(tests): Prepare for Special Type experiments
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.
2021-11-04 13:12:01 -05:00
Ed Page
844251c478 test: Easier to test with minimal features
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.
2021-11-02 16:40:31 -05:00
Ed Page
a9c6a5b531 refactor(tests): Remove unnecessary color disabling
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.
2021-11-02 16:40:31 -05:00
Ed Page
4b7ed54d7e test(derive): Provide better error info
`Parser::parse_from` will call `exit` on failure and we don't just lose
backtrace information but we don't even know which of the tests running
in parallel panicked.  I ran into this when experimenting with
`clap_derive` and I couldn't tell what actually failed.

So let's switch to `Parse::try_parse_from`.

Errors went from:
```
test option_option ... ok
error: Found argument 'bar' which wasn't expected, or isn't valid in this context

USAGE:
    clap_derive [OPTIONS]

For more information try --help
error: test failed, to rerun pass '--test arg_enum'
```
To:
```
test option_option ... ok
test variant_with_defined_casing ... ok
test skip_variant ... ok
test default_value ... ok
test vector ... FAILED
test option_vector ... ok

failures:

---- vector stdout ----
thread 'vector' panicked at 'called `Result::unwrap()` on an `Err` value: Error { message: Formatted(Colorizer { use_stderr: true, color_when: Auto
, pieces: [("error:", Some(Red)), (" ", None), ("Found argument '", None), ("bar", Some(Yellow)), ("' which wasn't expected, or isn't valid in this
 context", None), ("\n\n", None), ("USAGE:\n    clap_derive [OPTIONS]", None), ("\n\nFor more information try ", None), ("--help", Some(Green)), ("
\n", None)] }), kind: UnknownArgument, info: ["bar"], source: None, backtrace: Backtrace }', clap_derive/tests/arg_enum.rs:388:56
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

failures:
    vector

test result: FAILED. 15 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass '--test arg_enum'
```
2021-10-30 10:00:34 -05:00
Ed Page
93c26dd420 test(derive): Verify derive-genned errors are formatted
This is to help verify behavior added in #2943.  We separated the error
raising site from the error formatting site and this verifies that the
formatting actually happens.
2021-10-29 16:09:07 -05:00
bors[bot]
a802662341
Merge #2952 #2953
2952: fix(derive): Don't duplicate subcommand aliases r=pksunkara a=epage



2953: docs(app): Correlate help_heading and subcommand_placeholder r=pksunkara a=epage



Co-authored-by: Ed Page <eopage@gmail.com>
2021-10-26 21:56:54 +00:00
Ed Page
d05622d015 fix(derive): Don't duplicate subcommand aliases
When an anonymous struct is inside of an enum, we end up applying the
App methods twice, once for the `augment_args` and once for variant.

This consolidates those calls.

Fixes #2898
2021-10-26 16:01:05 -05:00
bors[bot]
f9e074e554
Merge #2926 #2948
2926: Put `grouped_values_of` behind a feature gate r=pksunkara a=epage



2948: docs(generate): Move derive example to generate r=pksunkara a=epage



Co-authored-by: Ed Page <eopage@gmail.com>
2021-10-26 20:56:20 +00:00
Ed Page
a065702205 docs(generate): Move derive example to generate
From a users perspective, `clap_derive` is baked into `clap`.
`clap_generate is an add on to `clap`.  So it seems best to have all
`clap_generate` examples in `clap_generate` where a user will look for
them.

Fixes #2939
2021-10-26 14:26:50 -05:00
Ed Page
53e10b41e3 fix(derive)!: Error, don't panic!
The derive is generating `Error::raw` (from scratch or by converting
existing erors) and then the inherent `Parser` methods format them.

Fixes #2255
2021-10-26 14:26:50 -05:00
Ed Page
69a7b7133a fix(derive): Use variants help_heading
Due to a copy/paste bug, we were reading the `help_heading` for
Subcommands from the enum's attribute and not the variant's attribute.

It doesn't make sense for the outer command's help_heading to control
the subcommands help_heading.

This does raise an interesting question on inheriting / propagating help_heading,
which I originally wrote the tests for.  We'd first need to answer
whether it should be built-in to the builder or derive-specific.
2021-10-26 12:49:43 -05:00
Ed Page
5d036d4d34 fix(derive): Support arg_enum everywhere
In working on converting unwraps to errors, I noticed that we did not
spport `arg_enum` for  `Option<Option<_>>` and `Option<Vec<_>>`, so this
addresses that.

My main motivation was to consolidate and make the logic more
consistent, the bug fix just fell out of that work.
2021-10-25 14:12:08 -05:00
bors[bot]
d6462884ed
Merge #2941
2941: Fix a path incompatible with re-exports r=pksunkara a=epage



Co-authored-by: Ed Page <eopage@gmail.com>
2021-10-25 15:40:33 +00:00
Ed Page
345694fea8 fix(derive): Allow subcommands with re-exporting
Our approach to re-exporting is to not fully qualify symbols in `clap`.
This is a place where a leading `::` got introduced, so its being
removed.
2021-10-25 09:44:38 -05:00
Ed Page
92fea91dfe doc(derive): Note derive policies 2021-10-25 09:44:05 -05:00
Ed Page
ed987933cf test: Verify clap_derive README
Fixes #2920
2021-10-25 09:21:45 -05:00
Ed Page
e5f10c81bc feat(derive): Allow help_heading on flattening fields
Fixes #2928
2021-10-23 16:05:08 -05:00
Ed Page
3a697af253 fix(derive): Avoid name collisions with users
PR #2751 highlighted a problem we have where the variable names we use
could collide with users.  Rather than parse out when or not to use
special names, and worry about people keeping that up to date through
refactors, I globally renamed all variables by adding a `__clap_`
prefix, which looks like what serde does to solve this problem.

I audited the result with `cargo expand`.  I didn't add any tests
because any tests would be reactionary and would give us a false sense
of protection since any new code could hit this with anything we do.
Our best route for naming is consistency so people are likely to notice
and copy.

Fixes #2934
2021-10-23 12:55:30 -05:00
Jake Shadle
db2a25473a Fix packaging of LICENSE-* files 2021-10-21 14:57:49 +02:00
bors[bot]
c4dfb1aede
Merge #2916
2916: fix ArgEnum multiline doc comment r=epage a=marjakm



Co-authored-by: Mattis Marjak <mattis.marjak@gmail.com>
2021-10-19 17:06:14 +00:00
Mattis Marjak
cba7ba5369 fix ArgEnum multiline doc comment 2021-10-19 18:34:54 +03:00
Ed Page
9f12bfec47 fix!: Rename ArgValue to PossibleValue
In considering potential work for #2683, I realized we might need a type to carry data for
each of the `multiple_values`.  `ArgValue` works both for that and for
possible values, so we need to come up with a better name for one or
both.  Changing `ArgValue`s name now would be ideal since its new in
clap3 and by renaming it, we can reduce churn for users.

While thinking about this, I realized I regularly get these mixed
up, so renaming `ArgValue` to `PossibleValue` I think will help clear
things up, regardless of #2683.
2021-10-19 10:10:37 -05:00
rhysd
012f318c97 feat(doc): Fix many typos in docs, comments and codes found by typos-cli 2021-10-19 10:38:22 +09:00
Pavan Kumar Sunkara
585e995811 Release 3.0.0-beta.5 2021-10-17 17:01:18 +01:00
Julius Michaelis
16af4f230a cosmetic: spec_vals on separate lines for long help 2021-10-17 22:43:06 +09:00
Ed Page
65e4bf232d fix(derive): Don't leak help headings
Before, when `flatten`ing, the struct you flattened in could set the
help heading for all the following arguments.  Now, we scope each
`flatten` to not do that.

I had originally intended to bake this into the initial / final methods
but that would have required some re-work to allow capturing state
between them that seemed unnecessary.

Fixes #2803
2021-10-16 11:21:16 -05:00
bors[bot]
6eacd8a747
Merge #2882
2882: fix(derive): Ensure App help_heading is applied r=epage a=epage



Co-authored-by: Ed Page <eopage@gmail.com>
2021-10-16 02:47:34 +00:00
Ed Page
7b56184175 refactor(derive): Simplify 'error' handling
Our goal is to not panic inside of the macro (e.g. #2255).  Currently,
we `.unwrap()` everywhere except when turning an `ArgMatches` into an
`enum`.  To handle `flatten`, we walk through each `flatten`ed enum to
see if it can be instantiated.  We don't want to mix this up with any of
the other eror cases (including further nested versions of this).

If we went straight to `Result<T>`, we'd have to differentiate this case
through the `ErrorKind` and `map_err` it in all other cases to prevent
it from bubbling up and confusing us.

Alternatively, we could do a more complicated type `Result<Option<T>>`
where the `Option` exists purely for this case and we can use type
checking to make sure we properly turn the `None`s into errors when
needed.

Or we can bypass all of this and just ask the `flatten`ed` subcommand if
it supports the current command.
2021-10-15 15:43:02 -05:00
Ed Page
22edac66d9 fix(derive): Ensure App help_heading is applied
We normally set all app attributes at the end.  This can be changed but
will require some work to ensure
- Top-level item's doc cmment ins our over flattened
- We still support `Args` / `Subcommand` be used to initialize an `App` when
  creating a subcommand

In the mean time, this special cases `help_heading` to happen first.
We'll need this special casing anyways to address #2803 since we'll need
to capture the old help heading before addings args and then restore it
after.  I guess we could unconditionally do that but its extra work /
boilerplate for when people have to dig into their what the derives do.

Fixes #2785
2021-10-15 14:19:16 -05:00
Ed Page
7b3d3401c0 fix(derive): Support 'update' with 'flatten'
When working on #2803, I noticed a disprecancy in the augment
behavior between `Arg`s and `Subcommand`s that makes it so `Arg`s can't
be flattened with the update functionality.
2021-10-15 14:19:16 -05:00
Ed Page
61c9e6265b fix(help)!: Merge OPTIONS / FLAGS default groups
For those that want the original behavior, you can usxe
`arg.help_heading(Some("FLAGS"))` on your flags.  Limitations:
- This will not give you a special sort order
- This will not get a `[FLAGS]` added to usage

For templates, we removed `{unified}` and `{flags}`.  To help people
catch these, a debug_assert was added.

I'm unsure but I think there might be a change in behavior in calcuating
when to show `[OPTION]` in usage.  The old code only looked at
`required` while flags looked only at arg groups.  We now look at both.

Ideally we'd add these in `_build` and remove special casing for
no-groups except in the sort order of groups.  I feel like thats best
left for later.

This also reduced the scope of `App`s public API.
`get_*_with_no_heading` seemed a bit specialized to be in the public
API.  #2853 looks at splitting it out into its own PR.

BREAKING CHANGE: Multiple
- `UnifiedHelpMessage` removed
- `{flags}` and `{unified}` are removed and will assert when present.
- `get_*_with_no_heading` removed

Fixes #2807
2021-10-13 11:42:10 -05:00
Pavan Kumar Sunkara
efeb02cd34 fix!: Make color settings an enum 2021-10-13 13:54:44 +01:00
Ed Page
60c9c2e59a docs(derive): Use more-specific traits
This helps to raise visibility of the new derive traits.

I didn't touch ui tests because there were a lot and they didn't seem to
be as high value.
2021-10-12 07:51:11 -05:00
bors[bot]
74cbe0bf09
Merge #2721
2721: feat(generate): Give `Shell` superpowers to simplify getting started r=pksunkara a=epage



Co-authored-by: Ed Page <eopage@gmail.com>
2021-10-11 23:48:17 +00:00
Pavan Kumar Sunkara
685601e002 fix: Add test for flattening struct in enum 2021-10-11 21:54:44 +01:00
Ed Page
96f2343e1b feat(generate): 'impl Generator for Shell'
This includes updating the example but does not include improving the
jumping-off documentation.
2021-10-11 10:46:44 -05:00
Ed Page
1bbfd956aa test: Harden tests against trailing whitespace
In a follow up commit, my editor stripped trailing whitespace.  Rather
than have everyone fight this, let's not make a deal over the small
stuff.
2021-10-11 10:44:48 -05:00
Ed Page
35d53d9dcf feat(generate): 'impl ArgEnum for Shell'
These keeps `FromStr` for ease of use with `value_of_t`.

This includes adding test to make sure everything works as expected.
2021-10-11 10:44:48 -05:00