Skip to content

Commit

Permalink
fix(derive): Remove next_help_heading isolation
Browse files Browse the repository at this point in the history
Originally, I saw the ideal as the parent command being isolated from
`#[clap(flatte)]` especially after all of the doc comment
leakage issues.  We scaled that back to just `next_help_heading` because
of the issues with settling on a policy and maintenance to cover
everything.  When doing `next_display_order`, we decided it would mess
things up too much to isolate it.

With clap-rs#1807, we instead have been moving towards setting
`#[command(next_help_heading)]` anywhere, we just need to finish working
out how it should work.
  • Loading branch information
epage committed Sep 16, 2022
1 parent 417f1a9 commit 14c6ce0
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 12 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -229,6 +229,7 @@ Easier to catch changes:
- `#[clap(value_parser)]` and `#[clap(action)]` are now redundant
- *(derive)* `subcommand_required(true).arg_required_else_help(true)` is set instead of `SubcommandRequiredElseHelp` to give more meaningful errors when subcommands are missing and to reduce redundancy (#3280)
- *(derive)* Remove `arg_enum` attribute in favor of `value_enum` to match the new name (we didn't have support in v3 to mark it deprecated) (#4127)
- *(derive)* `next_help_heading` can now leak out of a `#[clap(flatten)]`, like all other command settings
- *(parser)* Assert when the CLI looksup an unknown args when external subcommand support is enabled to help catch bugs (#3703)
- *(assert)* Sometimes `Arg::default_missing_value` didn't require `num_args(0..=N)`, now it does (#4023)
- *(assert)* Leading dashes in `Arg::long` are no longer allowed (#3691)
Expand Down
5 changes: 0 additions & 5 deletions clap_derive/src/derives/args.rs
Expand Up @@ -194,26 +194,21 @@ pub fn gen_augment(
}
Kind::Flatten => {
let ty = &field.ty;
let old_heading_var = format_ident!("__clap_old_heading");
let next_help_heading = item.next_help_heading();
let next_display_order = item.next_display_order();
if override_required {
Some(quote_spanned! { kind.span()=>
let #old_heading_var = #app_var.get_next_help_heading().map(|s| clap::builder::Str::from(s.to_owned()));
let #app_var = #app_var
#next_help_heading
#next_display_order;
let #app_var = <#ty as clap::Args>::augment_args_for_update(#app_var);
let #app_var = #app_var.next_help_heading(clap::builder::Resettable::from(#old_heading_var));
})
} else {
Some(quote_spanned! { kind.span()=>
let #old_heading_var = #app_var.get_next_help_heading().map(|s| clap::builder::Str::from(s.to_owned()));
let #app_var = #app_var
#next_help_heading
#next_display_order;
let #app_var = <#ty as clap::Args>::augment_args(#app_var);
let #app_var = #app_var.next_help_heading(clap::builder::Resettable::from(#old_heading_var));
})
}
}
Expand Down
5 changes: 0 additions & 5 deletions clap_derive/src/derives/subcommand.rs
Expand Up @@ -181,28 +181,23 @@ fn gen_augment(
} else {
quote!()
};
let old_heading_var = format_ident!("__clap_old_heading");
let next_help_heading = item.next_help_heading();
let next_display_order = item.next_display_order();
let subcommand = if override_required {
quote! {
#deprecations
let #old_heading_var = #app_var.get_next_help_heading().map(|s| clap::builder::Str::from(s.to_owned()));
let #app_var = #app_var
#next_help_heading
#next_display_order;
let #app_var = <#ty as clap::Subcommand>::augment_subcommands_for_update(#app_var);
let #app_var = #app_var.next_help_heading(clap::builder::Resettable::from(#old_heading_var));
}
} else {
quote! {
#deprecations
let #old_heading_var = #app_var.get_next_help_heading().map(|s| clap::builder::Str::from(s.to_owned()));
let #app_var = #app_var
#next_help_heading
#next_display_order;
let #app_var = <#ty as clap::Subcommand>::augment_subcommands(#app_var);
let #app_var = #app_var.next_help_heading(clap::builder::Resettable::from(#old_heading_var));
}
};
Some(subcommand)
Expand Down
4 changes: 2 additions & 2 deletions tests/derive/help.rs
Expand Up @@ -131,11 +131,11 @@ fn app_help_heading_flattened() {
.unwrap();
assert_eq!(should_be_in_section_b.get_help_heading(), Some("HEADING B"));

let should_be_in_default_section = cmd
let should_be_in_section_b = cmd
.get_arguments()
.find(|a| a.get_id() == "should_be_in_default_section")
.unwrap();
assert_eq!(should_be_in_default_section.get_help_heading(), None);
assert_eq!(should_be_in_section_b.get_help_heading(), Some("HEADING B"));

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

Expand Down

0 comments on commit 14c6ce0

Please sign in to comment.