Skip to content

Commit

Permalink
fix(derive): Don't leak help headings
Browse files Browse the repository at this point in the history
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
  • Loading branch information
epage committed Oct 16, 2021
1 parent 6eacd8a commit 65e4bf2
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 8 deletions.
9 changes: 7 additions & 2 deletions clap_derive/src/derives/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,18 @@ pub fn gen_augment(
| Kind::ExternalSubcommand => None,
Kind::Flatten => {
let ty = &field.ty;
let old_heading_var = format_ident!("old_heading");
if override_required {
Some(quote_spanned! { kind.span()=>
let #old_heading_var = #app_var.get_help_heading();
let #app_var = <#ty as clap::Args>::augment_args_for_update(#app_var);
let #app_var = #app_var.help_heading(#old_heading_var);
})
} else {
Some(quote_spanned! { kind.span()=>
let #old_heading_var = #app_var.get_help_heading();
let #app_var = <#ty as clap::Args>::augment_args(#app_var);
let #app_var = #app_var.help_heading(#old_heading_var);
})
}
}
Expand Down Expand Up @@ -358,10 +363,10 @@ pub fn gen_augment(
let initial_app_methods = parent_attribute.initial_top_level_methods();
let final_app_methods = parent_attribute.final_top_level_methods();
quote! {{
let #app_var = #app_var#initial_app_methods;
let #app_var = #app_var #initial_app_methods;
#( #args )*
#subcmd
#app_var#final_app_methods
#app_var #final_app_methods
}}
}

Expand Down
15 changes: 10 additions & 5 deletions clap_derive/src/derives/subcommand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,18 @@ fn gen_augment(
Kind::Flatten => match variant.fields {
Unnamed(FieldsUnnamed { ref unnamed, .. }) if unnamed.len() == 1 => {
let ty = &unnamed[0];
let old_heading_var = format_ident!("old_heading");
let subcommand = if override_required {
quote! {
let #old_heading_var = #app_var.get_help_heading();
let #app_var = <#ty as clap::Subcommand>::augment_subcommands_for_update(#app_var);
let #app_var = #app_var.help_heading(#old_heading_var);
}
} else {
quote! {
let #old_heading_var = #app_var.get_help_heading();
let #app_var = <#ty as clap::Subcommand>::augment_subcommands(#app_var);
let #app_var = #app_var.help_heading(#old_heading_var);
}
};
Some(subcommand)
Expand Down Expand Up @@ -221,10 +226,10 @@ fn gen_augment(
let subcommand = quote! {
let #app_var = #app_var.subcommand({
let #subcommand_var = clap::App::new(#name);
let #subcommand_var = #subcommand_var#initial_app_methods;
let #subcommand_var = #subcommand_var #initial_app_methods;
let #subcommand_var = #arg_block;
let #subcommand_var = #subcommand_var.setting(::clap::AppSettings::SubcommandRequiredElseHelp);
#subcommand_var#final_from_attrs
#subcommand_var #final_from_attrs
});
};
Some(subcommand)
Expand Down Expand Up @@ -264,9 +269,9 @@ fn gen_augment(
let subcommand = quote! {
let #app_var = #app_var.subcommand({
let #subcommand_var = clap::App::new(#name);
let #subcommand_var = #subcommand_var#initial_app_methods;
let #subcommand_var = #subcommand_var #initial_app_methods;
let #subcommand_var = #arg_block;
#subcommand_var#final_from_attrs
#subcommand_var #final_from_attrs
});
};
Some(subcommand)
Expand All @@ -278,7 +283,7 @@ fn gen_augment(
let initial_app_methods = parent_attribute.initial_top_level_methods();
let final_app_methods = parent_attribute.final_top_level_methods();
quote! {
let #app_var = #app_var#initial_app_methods;
let #app_var = #app_var #initial_app_methods;
#( #subcommands )*;
#app_var #final_app_methods
}
Expand Down
64 changes: 63 additions & 1 deletion clap_derive/tests/help.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clap::{Args, IntoApp, Parser};
use clap::{Args, IntoApp, Parser, Subcommand};

#[test]
fn arg_help_heading_applied() {
Expand Down Expand Up @@ -67,6 +67,12 @@ fn app_help_heading_flattened() {

#[clap(flatten)]
options_b: OptionsB,

#[clap(subcommand)]
sub_a: SubA,

#[clap(long)]
should_be_in_default_section: Option<u32>,
}

#[derive(Debug, Clone, Args)]
Expand All @@ -83,6 +89,31 @@ fn app_help_heading_flattened() {
should_be_in_section_b: Option<u32>,
}

#[derive(Debug, Clone, Subcommand)]
#[clap(help_heading = "SUB A")]
enum SubA {
#[clap(flatten)]
SubB(SubB),
#[clap(subcommand)]
SubC(SubC),
SubAOne,
SubATwo {
should_be_in_sub_a: Option<u32>,
},
}

#[derive(Debug, Clone, Subcommand)]
#[clap(help_heading = "SUB B")]
enum SubB {
SubBOne { should_be_in_sub_b: Option<u32> },
}

#[derive(Debug, Clone, Subcommand)]
#[clap(help_heading = "SUB C")]
enum SubC {
SubCOne { should_be_in_sub_c: Option<u32> },
}

let app = CliOptions::into_app();

let should_be_in_section_a = app
Expand All @@ -96,4 +127,35 @@ fn app_help_heading_flattened() {
.find(|a| a.get_name() == "should-be-in-section-b")
.unwrap();
assert_eq!(should_be_in_section_b.get_help_heading(), Some("HEADING B"));

let should_be_in_default_section = app
.get_arguments()
.find(|a| a.get_name() == "should-be-in-default-section")
.unwrap();
assert_eq!(should_be_in_default_section.get_help_heading(), None);

let sub_a_two = app.find_subcommand("sub-a-two").unwrap();

let should_be_in_sub_a = sub_a_two
.get_arguments()
.find(|a| a.get_name() == "should-be-in-sub-a")
.unwrap();
assert_eq!(should_be_in_sub_a.get_help_heading(), Some("SUB A"));

let sub_b_one = app.find_subcommand("sub-b-one").unwrap();

let should_be_in_sub_b = sub_b_one
.get_arguments()
.find(|a| a.get_name() == "should-be-in-sub-b")
.unwrap();
assert_eq!(should_be_in_sub_b.get_help_heading(), Some("SUB B"));

let sub_c = app.find_subcommand("sub-c").unwrap();
let sub_c_one = sub_c.find_subcommand("sub-c-one").unwrap();

let should_be_in_sub_c = sub_c_one
.get_arguments()
.find(|a| a.get_name() == "should-be-in-sub-c")
.unwrap();
assert_eq!(should_be_in_sub_c.get_help_heading(), Some("SUB C"));
}

0 comments on commit 65e4bf2

Please sign in to comment.