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
2817: Add support for Multicall executables as subcommands with a Multicall setting r=pksunkara a=fishface60
Co-authored-by: Richard Maw <richard.maw@gmail.com>
Who knew people need to ask `help` for how to use `help`?
While auditing `MultpleValues`, I saw commented out code. Looks
its commit (f230cfedc) was part of a large refactor and updating that
part fell through the cracks. Just simply updating it didn't quite get
it to work. The advantage of this approach is it gets us closer to how
clap works on its own.
In clap 2.33.3, `cmd help help` looks like
```
myapp-help
Prints this message or the help of the given subcommand(s)
USAGE:
test-clap help [subcommand]...
ARGS:
<subcommand>... The subcommand whose help message to display
```
But clap3 master looks like:
```
myapp
USAGE:
myapp [SUBCOMMAND]
OPTIONS:
-h, --help Print custom help about text
SUBCOMMANDS:
help Print this message or the help of the given subcommand(s)
subcmd
```
This change improves it to:
```
myapp-help
Print this message or the help of the given subcommand(s)
USAGE:
myapp help [SUBCOMMAND]...
ARGS:
<SUBCOMMAND>... The subcommand whose help message to display
OPTIONS:
-h, --help Print custom help about text
```
We still have global arguments showing up (like `-h`) that will error but its
an improvement! In general, I'd like to find a way to leverage clap's stanard
behavior for implementing this so we don't have to worry about any of these
corner cases in the future.
Note: compared to clap2, I changed `<subcommand>` to `<SUBCOMMAND>` because I
believe the standard convention is for value names to be all caps (e.g.
`clap_derive` has been updated to default to that).
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.
- `App::with_defaults` was not included since that has been deprecated
since 2.14
- `App::args_from_usage` does not have a close enough parallel in the
new API, as far as I could tell
- `ArgMatches::usage` cannot have a thin wrapper around
`App::generate_usage`.
- `App::write_*`: getting lazy, didn't seem like high value functions
- Any `Settings` (some things need to be figured out here)
This is a part of #2617
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.
There were fewer occasions than I expected where the use of
`multiple_values` was superfluous and we could instead use the more
predictable `multiple_occurrences`.
In terms of the remaining `multiple` split work, #1772 will take care of the derive
behavior and #2692 will resolve any remaining issues with values vs
occurrences in positional arguments.
Fixes#2816
This is inline with all of our other help-related functions that return
strings.
This is a part of #2164
BREAKING CHANEG: `App::generate_usage` (added in v3) ->
`App::render_usage`.
PR #1211 originally added `help_heading` with the current priority
(`App::help_heading` wins).
In #1958, the author had proposed to change this
> Note that I made the existing arg's heading a priority over the one in App::help_heading
This was reverted on reviewer request because
> Thanks for the priority explanation. Yes, please make the app's help
> heading a priority. I can see how it would be useful when people might
> want to reuse args.
Re-using args is an important use case but this makes life difficult
for anyone using `help_heading` with `clap_derive` because the user
can only call `App::help_heading` once per struct. Derive users can't get
per-field granularity with `App::help_heading` like the builder API.
As a bonus, I think this will be the least surpising to users. Users
generally expect the more generic specification (App) to be overridden by the
more specific specification (Arg). Honestly, I had thought this PR is
how `help_heading` worked until I dug into the code with #2872.
In the argument re-use cases, a workaround is for the reusable arguments
to **not** set their help_heading and require the caller to set it as
needed.
Fixes#2873
2871: Better positional checks r=epage a=pksunkara
2872: Iterate on help_heading to prepare for derive support r=pksunkara a=epage
2876: Generate/bash: add possible_values to completion when available r=pksunkara a=nstinus
Co-authored-by: Pavan Kumar Sunkara <pavan.sss1991@gmail.com>
Co-authored-by: Ed Page <eopage@gmail.com>
Co-authored-by: Nicolas Stinus <nstinus@latourtrading.com>
Now that `help_heading`'s API is loosened with an `Into<Option>`, we can
more easily allow the existing yaml functionality to work. This still
misses the ability to set the help heading to nothing.
This reverts commit 9031deb806.
This makes `Some()` optional. This change was made with the derive API
in mind where you are unlikely to clear directly but we still need the
ability to clear.
In part, this is just fixing a papercut where someone will try to use
the API in the same way between the two but it fails and they'll have to
consult the docs / rust-analyzer.
The bigger reason is that this is more derive-friendly for dealing with #2803
since we will be able to just ask for the current help heading
before running the app and then restore it back, rather than having to
conditionalize the revert logic.
Problems with this
- It is incompatible with the new signature planned for
`App::help_heading`
- It is missing from `Arg` which is where this is more needed
- All this can do is set a global help heading because you can duplicate
keys (`clap_derive` has a similar problem but it at least has `flatten`)
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
`App::get_matches` lazily post-processes `App`s and `Arg`s so we don't
do it to subcommands that are never run (downside being people have to
exercise their full app to get debug_asserts).
`clap_generate` was only post-processing the top-level `App` and `Arg`s,
ignoring the sub-commands. In #2858, we noticed that `--version` was
being left in the completions instead of being removed during the
`_build` step. We would also have an incorrect `num_vals` and a host of
other problems.
This change adds a `App::_build_all` function for `clap_generate` to use
to eagerly build everything. By having it there, we make sure
everywhere that needs eager building, gets it (like some tests).
In `clap_generate::utils`, we add a unit test to ensure the subcommand's
`--version` was removed.
For some other tests specifying `.version()`, I added
`AppSettings::PropagateVersion` to make it behave more consistently.
The places I didn't were generally where the version was conditionally
set.
For `clap_generate/tests/generate_completions.rs`, I had to adjust the
`conflicts_with` because the subcommand was inheriting the argument with
it defined *but* the subcommand did not have the argument, tripping up a
debug assert.
Fixes#2860
