From fd8d277207257f11a21dbfdea8174c45fed03120 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 22 Jul 2022 12:22:22 -0500 Subject: [PATCH] fix(derive): Move off of SubcommandRequiredElseHelp This also let us remove the deprecated attribute Fixes #3280 --- CHANGELOG.md | 1 + clap_bench/benches/06_rustup.rs | 5 ----- clap_derive/src/derives/args.rs | 7 +++--- clap_derive/src/derives/into_app.rs | 4 ++-- clap_derive/src/derives/subcommand.rs | 5 +++-- clap_mangen/src/render.rs | 3 +-- src/builder/app_settings.rs | 14 ------------ src/output/usage.rs | 4 +--- src/parser/validator.rs | 5 ----- tests/builder/app_settings.rs | 31 --------------------------- 10 files changed, 11 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57f3f2c2d16b..eaab232c81bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - `ErrorKind::EmptyValue` replaced with `ErrorKind::InvalidValue` - `ErrorKind::UnrecognizedSubcommand` replaced with `ErrorKind::InvalidSubcommand` +- *(derive)* `subcommand_required(true).arg_required_else_help(true)` is set instead of `SubcommandRequiredElseHelp` ### Features diff --git a/clap_bench/benches/06_rustup.rs b/clap_bench/benches/06_rustup.rs index ddc9fb321679..3b2f756bafac 100644 --- a/clap_bench/benches/06_rustup.rs +++ b/clap_bench/benches/06_rustup.rs @@ -27,7 +27,6 @@ fn build_cli() -> Command<'static> { .about("The Rust toolchain installer") .after_help(RUSTUP_HELP) .setting(AppSettings::DeriveDisplayOrder) - // .setting(AppSettings::SubcommandRequiredElseHelp) .arg( Arg::new("verbose") .help("Enable verbose output") @@ -69,7 +68,6 @@ fn build_cli() -> Command<'static> { .about("Modify or query the installed toolchains") .after_help(TOOLCHAIN_HELP) .setting(AppSettings::DeriveDisplayOrder) - // .setting(AppSettings::SubcommandRequiredElseHelp) .subcommand(Command::new("list").about("List installed toolchains")) .subcommand( Command::new("install") @@ -107,7 +105,6 @@ fn build_cli() -> Command<'static> { Command::new("target") .about("Modify a toolchain's supported targets") .setting(AppSettings::DeriveDisplayOrder) - // .setting(AppSettings::SubcommandRequiredElseHelp) .subcommand( Command::new("list") .about("List installed and available targets") @@ -142,7 +139,6 @@ fn build_cli() -> Command<'static> { Command::new("component") .about("Modify a toolchain's installed components") .setting(AppSettings::DeriveDisplayOrder) - // .setting(AppSettings::SubcommandRequiredElseHelp) .subcommand( Command::new("list") .about("List installed and available components") @@ -168,7 +164,6 @@ fn build_cli() -> Command<'static> { .about("Modify directory toolchain overrides") .after_help(OVERRIDE_HELP) .setting(AppSettings::DeriveDisplayOrder) - // .setting(AppSettings::SubcommandRequiredElseHelp) .subcommand(Command::new("list").about("List directory toolchain overrides")) .subcommand( Command::new("set") diff --git a/clap_derive/src/derives/args.rs b/clap_derive/src/derives/args.rs index 4195a0810829..b0b78673342b 100644 --- a/clap_derive/src/derives/args.rs +++ b/clap_derive/src/derives/args.rs @@ -179,10 +179,9 @@ pub fn gen_augment( quote!() } else { quote_spanned! { kind.span()=> - #[allow(deprecated)] - let #app_var = #app_var.setting( - clap::AppSettings::SubcommandRequiredElseHelp - ); + let #app_var = #app_var + .subcommand_required(true) + .arg_required_else_help(true); } }; diff --git a/clap_derive/src/derives/into_app.rs b/clap_derive/src/derives/into_app.rs index 435299e81943..986b0de56667 100644 --- a/clap_derive/src/derives/into_app.rs +++ b/clap_derive/src/derives/into_app.rs @@ -103,9 +103,9 @@ pub fn gen_for_enum(enum_name: &Ident, generics: &Generics, attrs: &[Attribute]) #[deny(clippy::correctness)] impl #impl_generics clap::CommandFactory for #enum_name #ty_generics #where_clause { fn command<'b>() -> clap::Command<'b> { - #[allow(deprecated)] let #app_var = clap::Command::new(#name) - .setting(clap::AppSettings::SubcommandRequiredElseHelp); + .subcommand_required(true) + .arg_required_else_help(true); ::augment_subcommands(#app_var) } diff --git a/clap_derive/src/derives/subcommand.rs b/clap_derive/src/derives/subcommand.rs index 8db400f915fe..3367436a1c02 100644 --- a/clap_derive/src/derives/subcommand.rs +++ b/clap_derive/src/derives/subcommand.rs @@ -256,8 +256,9 @@ fn gen_augment( let #subcommand_var = clap::Command::new(#name); let #subcommand_var = #subcommand_var #initial_app_methods; let #subcommand_var = #arg_block; - #[allow(deprecated)] - let #subcommand_var = #subcommand_var.setting(clap::AppSettings::SubcommandRequiredElseHelp); + let #subcommand_var = #subcommand_var + .subcommand_required(true) + .arg_required_else_help(true); #subcommand_var #final_from_attrs }); }; diff --git a/clap_mangen/src/render.rs b/clap_mangen/src/render.rs index b17f3bb849e3..6530d1d28d9f 100644 --- a/clap_mangen/src/render.rs +++ b/clap_mangen/src/render.rs @@ -188,8 +188,7 @@ pub(crate) fn after_help(roff: &mut Roff, cmd: &clap::Command) { } fn subcommand_markers(cmd: &clap::Command) -> (&'static str, &'static str) { - #[allow(deprecated)] - markers(cmd.is_subcommand_required_set() || cmd.is_set(AppSettings::SubcommandRequiredElseHelp)) + markers(cmd.is_subcommand_required_set() } fn option_markers(opt: &clap::Arg) -> (&'static str, &'static str) { diff --git a/src/builder/app_settings.rs b/src/builder/app_settings.rs index 206c4b064080..9349ed378b15 100644 --- a/src/builder/app_settings.rs +++ b/src/builder/app_settings.rs @@ -116,17 +116,6 @@ pub enum AppSettings { )] SubcommandRequired, - /// Deprecated, replaced with [`Command::subcommand_required`] combined with - /// [`Command::arg_required_else_help`]. - #[cfg_attr( - feature = "deprecated", - deprecated( - since = "3.1.0", - note = "Replaced with `Command::subcommand_required` combined with `Command::arg_required_else_help`" - ) - )] - SubcommandRequiredElseHelp, - /// Deprecated, replaced with [`Command::allow_external_subcommands`] and /// [`Command::is_allow_external_subcommands_set`] #[cfg_attr( @@ -392,7 +381,6 @@ bitflags! { const PROPAGATE_VERSION = 1 << 3; const DISABLE_VERSION_FOR_SC = 1 << 4; const WAIT_ON_ERROR = 1 << 6; - const SC_REQUIRED_ELSE_HELP = 1 << 7; const NO_AUTO_HELP = 1 << 8; const NO_AUTO_VERSION = 1 << 9; const DISABLE_VERSION_FLAG = 1 << 10; @@ -490,8 +478,6 @@ impl_settings! { AppSettings, AppFlags, => Flags::SC_NEGATE_REQS, SubcommandRequired => Flags::SC_REQUIRED, - SubcommandRequiredElseHelp - => Flags::SC_REQUIRED_ELSE_HELP, UseLongFormatForHelpSubcommand => Flags::USE_LONG_FORMAT_FOR_HELP_SC, TrailingVarArg diff --git a/src/output/usage.rs b/src/output/usage.rs index 7adaf58c6eb9..2803ea231c23 100644 --- a/src/output/usage.rs +++ b/src/output/usage.rs @@ -142,9 +142,7 @@ impl<'help, 'cmd> Usage<'help, 'cmd> { usage.push_str(" <"); usage.push_str(placeholder); usage.push('>'); - } else if self.cmd.is_subcommand_required_set() - || self.cmd.is_set(AS::SubcommandRequiredElseHelp) - { + } else if self.cmd.is_subcommand_required_set() { usage.push_str(" <"); usage.push_str(placeholder); usage.push('>'); diff --git a/src/parser/validator.rs b/src/parser/validator.rs index 7d7ef6513e3b..2c65e47a4218 100644 --- a/src/parser/validator.rs +++ b/src/parser/validator.rs @@ -60,7 +60,6 @@ impl<'help, 'cmd> Validator<'help, 'cmd> { return Err(Error::display_help_error(self.cmd, message)); } } - #[allow(deprecated)] if !has_subcmd && self.cmd.is_subcommand_required_set() { let bn = self .cmd @@ -73,10 +72,6 @@ impl<'help, 'cmd> Validator<'help, 'cmd> { .required(&self.required) .create_usage_with_title(&[]), )); - } else if !has_subcmd && self.cmd.is_set(AppSettings::SubcommandRequiredElseHelp) { - debug!("Validator::new::get_matches_with: SubcommandRequiredElseHelp=true"); - let message = self.cmd.write_help_err(false, Stream::Stderr)?; - return Err(Error::display_help_error(self.cmd, message)); } self.validate_conflicts(matcher, &mut conflicts)?; diff --git a/tests/builder/app_settings.rs b/tests/builder/app_settings.rs index fd83961eb2cc..eedc2389e66c 100644 --- a/tests/builder/app_settings.rs +++ b/tests/builder/app_settings.rs @@ -194,37 +194,6 @@ fn arg_required_else_help_error_message() { ); } -#[test] -fn subcommand_required_else_help() { - #![allow(deprecated)] - let result = Command::new("test") - .setting(AppSettings::SubcommandRequiredElseHelp) - .subcommand(Command::new("info")) - .try_get_matches_from(&[""]); - - assert!(result.is_err()); - let err = result.err().unwrap(); - assert_eq!( - err.kind(), - ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand - ); -} - -#[test] -fn subcommand_required_else_help_error_message() { - #![allow(deprecated)] - let cmd = Command::new("test") - .setting(AppSettings::SubcommandRequiredElseHelp) - .version("1.0") - .subcommand(Command::new("info").arg(Arg::new("filename"))); - utils::assert_output( - cmd, - "test", - SUBCOMMAND_REQUIRED_ELSE_HELP, - true, // Unlike normal displaying of help, we should provide a fatal exit code - ); -} - #[cfg(not(feature = "suggestions"))] #[test] fn infer_subcommands_fail_no_args() {