Skip to content

Commit

Permalink
fix(gen): Ensure subcommands are post-processed
Browse files Browse the repository at this point in the history
`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
  • Loading branch information
epage committed Oct 12, 2021
1 parent 93decf7 commit a61b608
Show file tree
Hide file tree
Showing 13 changed files with 103 additions and 77 deletions.
4 changes: 1 addition & 3 deletions clap_generate/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,7 @@ where
G: Generator,
S: Into<String>,
{
// TODO: All the subcommands need to be built instead of just the top one
app._build();
app._build_bin_names();
app._build_all();

gen.generate(app, buf)
}
58 changes: 40 additions & 18 deletions clap_generate/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,9 +127,8 @@ mod tests {
use clap::Arg;
use pretty_assertions::assert_eq;

fn common() -> App<'static> {
let mut app = App::new("myapp")
.version("3.0")
fn common_app() -> App<'static> {
App::new("myapp")
.subcommand(
App::new("test").subcommand(App::new("config")).arg(
Arg::new("file")
Expand All @@ -141,16 +140,26 @@ mod tests {
),
)
.subcommand(App::new("hello"))
.bin_name("my-app");
.bin_name("my-app")
}

fn built() -> App<'static> {
let mut app = common_app();

app._build();
app._build_bin_names();
app._build_all();
app
}

fn built_with_version() -> App<'static> {
let mut app = common_app().version("3.0");

app._build_all();
app
}

#[test]
fn test_subcommands() {
let app = common();
let app = built_with_version();

assert_eq!(
subcommands(&app),
Expand All @@ -164,7 +173,7 @@ mod tests {

#[test]
fn test_all_subcommands() {
let app = common();
let app = built_with_version();

assert_eq!(
all_subcommands(&app),
Expand All @@ -173,21 +182,22 @@ mod tests {
("hello".to_string(), "my-app hello".to_string()),
("help".to_string(), "my-app help".to_string()),
("config".to_string(), "my-app test config".to_string()),
("help".to_string(), "my-app test help".to_string()),
]
);
}

#[test]
fn test_find_subcommand_with_path() {
let app = common();
let app = built_with_version();
let sc_app = find_subcommand_with_path(&app, "test config".split(' ').collect());

assert_eq!(sc_app.get_name(), "config");
}

#[test]
fn test_flags() {
let app = common();
let app = built_with_version();
let actual_flags = flags(&app);

assert_eq!(actual_flags.len(), 2);
Expand All @@ -196,15 +206,29 @@ mod tests {

let sc_flags = flags(find_subcommand_with_path(&app, vec!["test"]));

assert_eq!(sc_flags.len(), 3);
assert_eq!(sc_flags.len(), 2);
assert_eq!(sc_flags[0].get_long(), Some("file"));
assert_eq!(sc_flags[1].get_long(), Some("help"));
}

#[test]
fn test_flag_subcommand() {
let app = built();
let actual_flags = flags(&app);

assert_eq!(actual_flags.len(), 1);
assert_eq!(actual_flags[0].get_long(), Some("help"));

let sc_flags = flags(find_subcommand_with_path(&app, vec!["test"]));

assert_eq!(sc_flags.len(), 2);
assert_eq!(sc_flags[0].get_long(), Some("file"));
assert_eq!(sc_flags[1].get_long(), Some("help"));
assert_eq!(sc_flags[2].get_long(), Some("version"));
}

#[test]
fn test_shorts() {
let app = common();
let app = built_with_version();
let shorts = shorts_and_visible_aliases(&app);

assert_eq!(shorts.len(), 2);
Expand All @@ -213,16 +237,15 @@ mod tests {

let sc_shorts = shorts_and_visible_aliases(find_subcommand_with_path(&app, vec!["test"]));

assert_eq!(sc_shorts.len(), 4);
assert_eq!(sc_shorts.len(), 3);
assert_eq!(sc_shorts[0], 'p');
assert_eq!(sc_shorts[1], 'f');
assert_eq!(sc_shorts[2], 'h');
assert_eq!(sc_shorts[3], 'V');
}

#[test]
fn test_longs() {
let app = common();
let app = built_with_version();
let longs = longs_and_visible_aliases(&app);

assert_eq!(longs.len(), 2);
Expand All @@ -231,10 +254,9 @@ mod tests {

let sc_longs = longs_and_visible_aliases(find_subcommand_with_path(&app, vec!["test"]));

assert_eq!(sc_longs.len(), 4);
assert_eq!(sc_longs.len(), 3);
assert_eq!(sc_longs[0], "path");
assert_eq!(sc_longs[1], "file");
assert_eq!(sc_longs[2], "help");
assert_eq!(sc_longs[3], "version");
}
}
5 changes: 3 additions & 2 deletions clap_generate/tests/completions/bash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ fn build_app() -> App<'static> {
fn build_app_with_name(s: &'static str) -> App<'static> {
App::new(s)
.version("3.0")
.setting(AppSettings::PropagateVersion)
.about("Tests completions")
.arg(
Arg::new("file")
Expand Down Expand Up @@ -70,7 +71,7 @@ static BASH: &str = r#"_myapp() {
return 0
;;
myapp__help)
opts=" -h -V --help --version "
opts=" -h --help "
if [[ ${cur} == -* || ${COMP_CWORD} -eq 2 ]] ; then
COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
return 0
Expand Down Expand Up @@ -173,7 +174,7 @@ static BASH_SPECIAL_CMDS: &str = r#"_my_app() {
return 0
;;
my_app__help)
opts=" -h -V --help --version "
opts=" -h --help "
if [[ ${cur} == -* || ${COMP_CWORD} -eq 2 ]] ; then
COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
return 0
Expand Down
5 changes: 1 addition & 4 deletions clap_generate/tests/completions/elvish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ fn build_app() -> App<'static> {
fn build_app_with_name(s: &'static str) -> App<'static> {
App::new(s)
.version("3.0")
.setting(AppSettings::PropagateVersion)
.about("Tests completions")
.arg(
Arg::new("file")
Expand Down Expand Up @@ -66,8 +67,6 @@ set edit:completion:arg-completer[my_app] = [@words]{
&'my_app;help'= {
cand -h 'Print help information'
cand --help 'Print help information'
cand -V 'Print version information'
cand --version 'Print version information'
}
]
$completions[$command]
Expand Down Expand Up @@ -145,8 +144,6 @@ set edit:completion:arg-completer[my_app] = [@words]{
&'my_app;help'= {
cand -h 'Print help information'
cand --help 'Print help information'
cand -V 'Print version information'
cand --version 'Print version information'
}
]
$completions[$command]
Expand Down
17 changes: 9 additions & 8 deletions clap_generate/tests/completions/fish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ fn build_app() -> App<'static> {
fn build_app_with_name(s: &'static str) -> App<'static> {
App::new(s)
.version("3.0")
.setting(AppSettings::PropagateVersion)
.about("Tests completions")
.arg(
Arg::new("file")
Expand Down Expand Up @@ -37,7 +38,6 @@ complete -c myapp -n "__fish_seen_subcommand_from test" -l case -d 'the case to
complete -c myapp -n "__fish_seen_subcommand_from test" -s h -l help -d 'Print help information'
complete -c myapp -n "__fish_seen_subcommand_from test" -s V -l version -d 'Print version information'
complete -c myapp -n "__fish_seen_subcommand_from help" -s h -l help -d 'Print help information'
complete -c myapp -n "__fish_seen_subcommand_from help" -s V -l version -d 'Print version information'
"#;

#[test]
Expand Down Expand Up @@ -74,7 +74,6 @@ complete -c my_app -n "__fish_seen_subcommand_from some_cmd" -s V -l version -d
complete -c my_app -n "__fish_seen_subcommand_from some-cmd-with-hypens" -s h -l help -d 'Print help information'
complete -c my_app -n "__fish_seen_subcommand_from some-cmd-with-hypens" -s V -l version -d 'Print version information'
complete -c my_app -n "__fish_seen_subcommand_from help" -s h -l help -d 'Print help information'
complete -c my_app -n "__fish_seen_subcommand_from help" -s V -l version -d 'Print version information'
"#;

#[test]
Expand Down Expand Up @@ -189,12 +188,14 @@ complete -c my_app -n "__fish_use_subcommand" -f -a "help" -d 'Print this messag
complete -c my_app -n "__fish_seen_subcommand_from test" -l case -d 'the case to test' -r
complete -c my_app -n "__fish_seen_subcommand_from test" -s h -l help -d 'Print help information'
complete -c my_app -n "__fish_seen_subcommand_from test" -s V -l version -d 'Print version information'
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd" -s h -l help -d 'Print help information'
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd" -s V -l version -d 'Print version information'
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd" -f -a "sub_cmd" -d 'sub-subcommand'
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd; and not __fish_seen_subcommand_from help" -s h -l help -d 'Print help information'
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd; and not __fish_seen_subcommand_from help" -s V -l version -d 'Print version information'
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd; and not __fish_seen_subcommand_from help" -f -a "sub_cmd" -d 'sub-subcommand'
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and not __fish_seen_subcommand_from sub_cmd; and not __fish_seen_subcommand_from help" -f -a "help" -d 'Print this message or the help of the given subcommand(s)'
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from sub_cmd" -l config -d 'the other case to test' -r
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from sub_cmd" -l help -d 'Print help information'
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from sub_cmd" -l version -d 'Print version information'
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from sub_cmd" -s h -l help -d 'Print help information'
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from sub_cmd" -s V -l version -d 'Print version information'
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from help" -s h -l help -d 'Print help information'
complete -c my_app -n "__fish_seen_subcommand_from some_cmd; and __fish_seen_subcommand_from help" -s V -l version -d 'Print version information'
complete -c my_app -n "__fish_seen_subcommand_from help" -s h -l help -d 'Print help information'
complete -c my_app -n "__fish_seen_subcommand_from help" -s V -l version -d 'Print version information'
"#;
2 changes: 1 addition & 1 deletion clap_generate/tests/completions/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clap::{App, Arg, ValueHint};
use clap::{App, AppSettings, Arg, ValueHint};
use clap_generate::{generate, generators::*};
use std::fmt;

Expand Down
5 changes: 1 addition & 4 deletions clap_generate/tests/completions/powershell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ fn build_app() -> App<'static> {
fn build_app_with_name(s: &'static str) -> App<'static> {
App::new(s)
.version("3.0")
.setting(AppSettings::PropagateVersion)
.about("Tests completions")
.arg(
Arg::new("file")
Expand Down Expand Up @@ -82,8 +83,6 @@ Register-ArgumentCompleter -Native -CommandName 'my_app' -ScriptBlock {
'my_app;help' {
[CompletionResult]::new('-h', 'h', [CompletionResultType]::ParameterName, 'Print help information')
[CompletionResult]::new('--help', 'help', [CompletionResultType]::ParameterName, 'Print help information')
[CompletionResult]::new('-V', 'V', [CompletionResultType]::ParameterName, 'Print version information')
[CompletionResult]::new('--version', 'version', [CompletionResultType]::ParameterName, 'Print version information')
break
}
})
Expand Down Expand Up @@ -174,8 +173,6 @@ Register-ArgumentCompleter -Native -CommandName 'my_app' -ScriptBlock {
'my_app;help' {
[CompletionResult]::new('-h', 'h', [CompletionResultType]::ParameterName, 'Print help information')
[CompletionResult]::new('--help', 'help', [CompletionResultType]::ParameterName, 'Print help information')
[CompletionResult]::new('-V', 'V', [CompletionResultType]::ParameterName, 'Print version information')
[CompletionResult]::new('--version', 'version', [CompletionResultType]::ParameterName, 'Print version information')
break
}
})
Expand Down
23 changes: 15 additions & 8 deletions clap_generate/tests/completions/zsh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ fn build_app() -> App<'static> {
fn build_app_with_name(s: &'static str) -> App<'static> {
App::new(s)
.version("3.0")
.setting(AppSettings::PropagateVersion)
.about("Tests completions")
.arg(
Arg::new("file")
Expand Down Expand Up @@ -73,8 +74,6 @@ _arguments "${_arguments_options[@]}" \
_arguments "${_arguments_options[@]}" \
'-h[Print help information]' \
'--help[Print help information]' \
'-V[Print version information]' \
'--version[Print version information]' \
&& ret=0
;;
esac
Expand Down Expand Up @@ -195,8 +194,6 @@ _arguments "${_arguments_options[@]}" \
_arguments "${_arguments_options[@]}" \
'-h[Print help information]' \
'--help[Print help information]' \
'-V[Print version information]' \
'--version[Print version information]' \
&& ret=0
;;
esac
Expand Down Expand Up @@ -364,8 +361,6 @@ _my_app() {
_arguments "${_arguments_options[@]}" \
'-h[Print help information]' \
'--help[Print help information]' \
'-V[Print version information]' \
'--version[Print version information]' \
":: :_my_app__second_commands" \
"*::: :->second" \
&& ret=0
Expand All @@ -378,8 +373,16 @@ _arguments "${_arguments_options[@]}" \
case $line[1] in
(third)
_arguments "${_arguments_options[@]}" \
'--version[Print version information]' \
'-h[Print help information]' \
'--help[Print help information]' \
&& ret=0
;;
(help)
_arguments "${_arguments_options[@]}" \
'--version[Print version information]' \
'-h[Print help information]' \
'--help[Print help information]' \
&& ret=0
;;
esac
Expand All @@ -390,8 +393,6 @@ esac
_arguments "${_arguments_options[@]}" \
'-h[Print help information]' \
'--help[Print help information]' \
'-V[Print version information]' \
'--version[Print version information]' \
&& ret=0
;;
esac
Expand All @@ -412,10 +413,16 @@ _my_app__help_commands() {
local commands; commands=()
_describe -t commands 'my_app help commands' commands "$@"
}
(( $+functions[_my_app__second__help_commands] )) ||
_my_app__second__help_commands() {
local commands; commands=()
_describe -t commands 'my_app second help commands' commands "$@"
}
(( $+functions[_my_app__second_commands] )) ||
_my_app__second_commands() {
local commands; commands=(
'third:' \
'help:Print this message or the help of the given subcommand(s)' \
)
_describe -t commands 'my_app second commands' commands "$@"
}
Expand Down
9 changes: 2 additions & 7 deletions clap_generate/tests/generate_completions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,8 @@ use std::io;
#[test]
fn generate_completions() {
let mut app = App::new("test_app")
.arg(
Arg::new("config")
.short('c')
.conflicts_with("v")
.global(true),
)
.arg(Arg::new("v").short('v'))
.arg(Arg::new("config").short('c').global(true))
.arg(Arg::new("v").short('v').conflicts_with("config"))
.subcommand(
App::new("test")
.about("Subcommand")
Expand Down

0 comments on commit a61b608

Please sign in to comment.