Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gen): Ensure subcommands are post-processed #2863

Merged
merged 1 commit into from
Oct 12, 2021
Merged

Conversation

epage
Copy link
Member

@epage epage commented Oct 12, 2021

App::get_matches lazily post-processes Apps and Args 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 Args,
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

Copy link
Member

@pksunkara pksunkara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor stuff remaining.

clap_generate/src/lib.rs Outdated Show resolved Hide resolved
clap_generate/tests/completions/mod.rs Show resolved Hide resolved
clap_generate_fig/tests/completions/mod.rs Show resolved Hide resolved
`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 clap-rs#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 clap-rs#2860
Copy link
Member

@pksunkara pksunkara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+

@bors
Copy link
Contributor

bors bot commented Oct 12, 2021

Build succeeded:

@bors bors bot merged commit d833d66 into clap-rs:master Oct 12, 2021
@epage epage deleted the version branch October 13, 2021 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shell completions include --version on subcommands despite no version set
2 participants