This contains two types of re-ordering:
- Internal, putting the focus of each file first.
- Public, re-arranging items and grouping impl blocks to try to better
organize the documentation and find related items.
The main weakness of the docs work is in `Args`. Its just a mess. In
particular, we should probably link the simple casesm like `required` to
the advanced impl block.
Fixes#55
Adding in a `StructOpt` derive with `structopt` attributes, with
deprecation notices. Unofrtunately, not as many deprecation warnings as
I would like. Apparently, you can't do them for a `use` of a derive? I
also wanted to inject code that would trigger a deprecation notice for
attributes but that would require enough of a refactor that I didn't
consider it worth it. We are at least providing a transition window
even if it means we'll have to remvoe it next major release without a
deprecation warning.
This creates distinct tutorial examples from complex feature examples
(more how-tos). Both sets are getting builder / derive versions (at
least the critical ones).
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.
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
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.
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
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.
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.
`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'
```
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.
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
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>
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
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.
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.
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
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.
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
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.
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
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.
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
A lot of users expected `color` feature flag and `ColorAuto` etc to
control all colors. Having this extra flag around is easy to miss and
adds to our overall settings bloat, making it harder to find settings
people want.
This completely removes it, rather than make it deprecated like
functions in #2617, because there is extra work to mark things
deprecated as Settings and we should decide on our strategy first before
investing time in addressing that issue.
Fixes#2806
Before #2005, `Clap` was a special trait that derived all clap traits it
detected were relevant (including an enum getting both `ArgEnum`,
`Clap`, and `Subcommand`). Now, we have elevated `Clap`, `Args`,
`Subcommand`, and `ArgEnum` to be user facing but the name `Clap` isn't
very descriptive.
This also helps further clarify the relationships so a crate providing
an item to be `#[clap(flatten)]` or `#[clap(subcommand)]` is more likely
to choose the needed trait to derive.
Also, my proposed fix fo #2785 includes making `App` attributes almost
exclusively for `Clap`. Clarifying the names/roles will help
communicate this.
For prior discussion, see #2583
2823: fix(derive): Subcommands not working within macro_rules r=epage a=epage
2824: docs(derive): Fix explanation on optional string list argument r=epage a=epage
2827: feat: Add backtraces to errors r=epage a=epage
Co-authored-by: Ed Page <eopage@gmail.com>
2821: test(derive): Port structopt flatten coverage r=epage a=epage
2822: feat(derive): Add support for lower/upper in rename_all r=epage a=epage
Co-authored-by: Ed Page <eopage@gmail.com>
This carries over a test case from
https://github.com/TeXitoi/structopt/pull/448, and re-fixes it according
to the changes we've made since we forked. I also tried to identify
other cases and quote them to avoid playing whack-a-mole with this.
This is a part of #2809
Some programs do not use anything to separate word boundaries.
For example a struct may contain the field `build_dir` while the flag is
`--builddir`.
This is a port of https://github.com/TeXitoi/structopt/pull/412
This is part of #2809
https://github.com/TeXitoi/structopt/pull/325 special cased `version`
because a default method would be added if the user did nothing, which
caused problems when nesting subcommands. We no longer apply that
default method and the highest item in the chain always has precedence,
so this can be simplified / clarified.
This will unblock us from removing the `version` hack because we'll
always get the right precedence.
Later we can explore ways of moving the app methods to being done first.
The big problem is with `#[clap(subcommand)]` because we need to call
those app methods on the subcommands `App` *and* override them with app
methods on the variant, requiring the app methods to be last to get
precedence.
Before there was no way to make `SubcommandsNegateReqs` with
`clap_derive` because it required a required field with a sentinel value
for when the required part was negated. We blocked that.
This turned out simpler than I expected.
This came out of the discussion for #2255 but that issue is more
specifically about the panic, so not closing it.
In #2270, its brought up to solve this at the `cargo workspace publish`
level. However, copying the files is a 5s fix without any design work,
it doesn't seem like support within an external tool should block us
from getting this fix into users hands. Once `cargo workspace publish`
supports this and if we are still using it, we can re-evaluate.
Fixes#2270
* feat(arg_value): ArgValue can be used for possible_values
Through the ArgValue it is possible:
* `hide` possible_values from showing in completion, help and validation
* add `about` to possible_values in completion
* Resolved a few change-requests by epage
* make clippy happy
* add ArgValue::get_visible_value
* remove verbose destructering
* rename ArgValue::get_hidden to ArgValue::is_hidden
* add test for help output of hidden ArgValues
* Documentation for ArgValue
There is an issue that required to implement From<&ArgValue> for
ArgValue. We should probably find a solution without that.
* fix requested changes by epage
* fix formatting
* add deref in possible_values call to remove From<&&str>
* make clippy happy
* use copied() instad of map(|v|*v)
* Finishing up for merge, hopefully
* changes requested by pksunkara
Before, validating UTF-8 was all-or-nothing and would cause a `panic` if
someone used the right API with non-UTF-8 input.
Now, all arguments are validated for UTF-8, unless opted-out. This
ensures a non-panicing path forward at the cost of people using the
builder API that previously did `value_of_os` need to now set this flag.
Fixes#751
I did some digging to root cause this but gave up and suppressed it,
like others. Warnings like this also come with a cost of code-gen
complexity.
Fixes#2712
Instead they should behave like `default_value`/`default_values`.
In implementingt this, I didn't see any reason to be using a `VecMap`.
In fact, this helped simplify the code / make intent clearer.
With this, we are also able to simplify the derive macro work from #2633.
Fixes#2634
BREAKING CHANGE: `value_name`/`value_names` always overwrite, rather
than append. We expect the impact to be minimal.
Right now
- `default_value="something"` is a raw method
- `default_value` uses native types
This commit splits the meanings
- `default_value="something"` is a raw method
- `default_value_t` uses `T::default()`
- `default_value_t=expr` uses an expression that evaluates to `T`
This is meant to mirror the `value_of` / `value_of_t` API.
At the moment, this is limited to `T: Display` to work with clap's
default system. Something we can look at in the future is a way to
loosen that restriction. One quick win is to specialize when `arg_enum`
is set. The main downside is complicating the processing of attributes
because it then means we need some processed before others.
Since this builds on `clap`s existing default system, this also means
users do not get any performance gains out of using `default_value_t`,
since we still need to parse it but we also need to convert it to a
string.
Fixes#1694
It looks like CI hasn't been running on this and we've introduced some
problems. It looks like we had an off-by-one error in the check for
MSRV for deciding to run ui tests.
It turns out `value_name` appends, so by setting an implicit and
explicit `value_name`, the user gets both and `num_vals=2`.
There is still a question on `value_name` and whether its documentation
or behavior needs updating. If that changes, then this can be
simplified by reverting back.
Fixes#2632
When using `#[clap(flatten)]` inside of a `Subcommand`, we would do a
`from` instead of an `update`.
The challenge is knowing when we are
going into a flattened subcommand vs changing the variant. To resolve
this, I added a `Subcommand:has_subcommand(name)` trait method that we
generate, so we can ask.
I debated putting this fix inside of the Builder API but I figure this
makes it so you "pay for what you use", with the derive API giving it to
you "for free".
A potential next step is to improve the default value name. I tend to
use value_name to hint at how to use an argument, which correlates with
types. We could add a
`ValueName::value_name(fallback: &str) -> &str` with impls for common
types, so we get more of a usage-based result.
Fixes#2608
One of the challenges with #2255 is for the user to discover whats going
wrong. This helps by at least telling people how they got into a bad
state and we can search for the code within the derive.
Before, when doing an `update` involving subcommands, we generated
parsing rules for the `from` case instead, requiring all arguments to be
present.
This switches us to descending into `update` code and adds tests to
verify it works.
This is a part of #2605
When debugging #2586, I noticed we were developing match cases for
variant names that were flattened. At minimum, this is dead code and at
worst this could cause the wrong behavior if a user does an update with
one of those names.
Depends on #2587Fixes#2588
`structopt` originally allowed
```
pub enum Opt {
Daemon(DaemonCommand),
}
pub enum DaemonCommand {
Start,
Stop,
}
```
This was partially broken in #1681 where `$ cmd daemon start` works but `cmd daemon`,
panics. Originally, `structopt` relied on exposing the implementation
details of a derived type by providing a `is_subcommand` option, so we'd
know whether to provide `SubcommandRequiredElseHelp` or not. This was
removed in #1681Fixes#2005
Before, partial command lines would panic at runtime. Now it'll be a
compile error
For example:
```
pub enum Opt {
Daemon(DaemonCommand),
}
pub enum DaemonCommand {
Start,
Stop,
}
```
Gives:
```
error[E0277]: the trait bound `DaemonCommand: clap::Args` is not satisfied
--> clap_derive/tests/subcommands.rs:297:16
|
297 | Daemon(DaemonCommand),
| ^^^^^^^^^^^^^ the trait `clap::Args` is not implemented for `DaemonCommand`
|
= note: required by `augment_args`
```
To nest this, you currently need `enum -> struct -> enum`. A later
change will make it so you can use the `subcommand` attribute within
enums to cover this case.
This is a part of #2005
While having convinience derives can be helpful, deriving traits that
are not used in similar situations (`Clap` and `ArgEnum`) can make
things harder
- From a user, derives are opaque and create uncertainty on how to use
the API if not kept crystal clear (deriving a name gives you the trait
by that name)
- This makes documentation harder to write and read
- You can use types in unintended places, which is made worse for crate
APIs because changing this breaks compatibility.
Fixes#2584
Change default help template:
- The new template introduce new lines before and after
author/about sections.
- Add help template placeholders:
- about-section
- author-section
- Documentation of new placeholders in clap::App::help_template
- Update all unit tests by incorporating new lines
- help / version flag report correct application name when generated
with clap_derive and an Enum.
- add clap_derive unit tests for application name:
file: clap_derive/tests/app_name.rs
tests: app_name_in_[short|long]_[help|version]_from_[struct|enum]()
In the `Clap` derive macro, a function parameter named `arg_matches` is
generated using `quote!` - as a result, this parameter ends up with
call-site hygiene. However, `arg_matches` is written literally within
several `quote_spanned!` blocks, which generate an `arg_matches` token
with the hygiene of whatever span was passed to `quote_spanned!`.
If these two hygienes are different (for example, if the user invokes
the derive macro from a `macro_rules!` macro), then a usage of
`arg_matches` may not resolve to the `arg_matches` parameter definition.
This commit changes the generation of `arg_matches` identifiers to
always use `quote!`, ensuring that they will always be considered the
'same' identifier by Rust.
Updates the derive example handling environment variables to illustrate
the case where it contains a sensitive value which should not be
displayed on the help screen.
Closes https://github.com/clap-rs/clap/issues/2101
Due to macro expansions, a `syn` type may be wrapped in multiple
'layers' of `syn::Type::Group`. However, `clap_derive` currently does
not check for `syn::Type::Group`, which will cause an `Option` (along
with other matched types) to fail to be detected when it results from a
macro expansion.
This commit 'unwraps' outer type groups before checking the
user-provided types against well-known types. Currently, these groups
may not be present due to a rustc bug (rust-lang/rust#43081)
However, once https://github.com/rust-lang/rust/pull/73084 is merged,
these groups will be present in more cases. This commit makes `clap`
compatible with both older and newer versions of rustc.
This PR switches the Arg::Short macro to take a character instead of a string. It removes the hacky code in the Method to_token method and implements the logic for Short when parsing the clap derive arguments.
Fixes#1815.
1689: Remove some mentioning of structopt r=TeXitoi a=CreepySkeleton
A work on #1671. Attention! It doesn't close it just yet
Co-authored-by: CreepySkeleton <creepy-skeleton@yandex.ru>
1681: WIP: Extract subcommands into separate trait r=pksunkara a=CreepySkeleton
Not-yet-working-but-almost-there "multiple traits" approach. More or less done, what's left is to catch some bugs and adapt tests/examples.
For the record: it took so long because of RL stuff (who would have thought?) and because [there was a detailed description of the experience I've had here, but it was deleted because it contained a lot of profanity and emotional notes].
As the only person alive that understands how the derive works (if you won't blow your own horn, nobody will do it for you, yeah), I'd like to made a statement: we Do need the refactoring.
Co-authored-by: CreepySkeleton <creepy-skeleton@yandex.ru>
1678: Refactor clap_generate r=CreepySkeleton a=pksunkara
I have copied the code from [clap_generate]( https://github.com/clap-rs/clap_generate) and refactored the structure a bit.
This new structure will allow people to write their own generators using our `Generator` trait which will contain some helpers (Still working on polishing them).
Co-authored-by: Ole Martin Ruud <barskern@outlook.com>
Co-authored-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>
1664: Import structopt r=pksunkara a=CreepySkeleton
OK, here is about 50% of what's left to import.
`impl StructOpt for Box<impl StructOpt>` is not imported because layouts of `StructOpt` and `Clap` are too different. I'll work it out after the import is done.
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/clap-rs/clap/1664)
<!-- Reviewable:end -->
Co-authored-by: CreepySkeleton <creepy-skeleton@yandex.ru>
Co-authored-by: Cecile Tonglet <cecile.tonglet@cecton.com>
Co-authored-by: David McNeil <mcneil.david2@gmail.com>
