Get Fig compiling and testing #2858
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
2 tasks
epage
added a commit
to epage/clap
that referenced
this pull request
Oct 12, 2021
`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
|
bors r+ |
|
I see you writing that comment again. Not sure why. Bors does read the comment if I use it while approving. |
I was experimenting because bors seemed to be stuck |
|
Build succeeded: |
epage
added a commit
to epage/clap
that referenced
this pull request
Oct 12, 2021
`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
epage
added a commit
to epage/clap
that referenced
this pull request
Oct 12, 2021
`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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Because we forgot to add it to our default workspace members, it started to drift already.