From 8b064cfee9b5bcedc3394a9fe864f7a07acd8f7e 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 | 4 +-- examples/derive_ref/interop_tests.md | 11 +++++-- src/builder/app_settings.rs | 14 -------- src/output/usage.rs | 5 +-- src/parser/validator.rs | 7 +--- tests/builder/app_settings.rs | 47 +-------------------------- 11 files changed, 22 insertions(+), 88 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57f3f2c2d16..eaab232c81b 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 ddc9fb32167..3b2f756bafa 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 ea5c5539d9f..6f3a4bd64fe 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 435299e8194..986b0de5666 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 8db400f915f..3367436a1c0 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 b17f3bb849e..1655af38a2a 100644 --- a/clap_mangen/src/render.rs +++ b/clap_mangen/src/render.rs @@ -1,4 +1,3 @@ -use clap::AppSettings; use roff::{bold, italic, roman, Inline, Roff}; pub(crate) fn subcommand_heading(cmd: &clap::Command) -> String { @@ -188,8 +187,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/examples/derive_ref/interop_tests.md b/examples/derive_ref/interop_tests.md index 746fe1878d7..a455c535df5 100644 --- a/examples/derive_ref/interop_tests.md +++ b/examples/derive_ref/interop_tests.md @@ -101,12 +101,19 @@ For more information try --help ```console $ interop_hand_subcommand ? failed -error: 'interop_hand_subcommand[EXE]' requires a subcommand but one was not provided +clap USAGE: interop_hand_subcommand[EXE] [OPTIONS] -For more information try --help +OPTIONS: + -h, --help Print help information + -t, --top-level + +SUBCOMMANDS: + add + help Print this message or the help of the given subcommand(s) + remove ``` diff --git a/src/builder/app_settings.rs b/src/builder/app_settings.rs index 206c4b06408..9349ed378b1 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 7adaf58c6eb..5cdbff08f3b 100644 --- a/src/output/usage.rs +++ b/src/output/usage.rs @@ -1,7 +1,6 @@ use indexmap::IndexSet; // Internal -use crate::builder::AppSettings as AS; use crate::builder::{Arg, ArgPredicate, Command}; use crate::parser::ArgMatcher; use crate::util::ChildGraph; @@ -142,9 +141,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 7d7ef6513e3..069152b47d6 100644 --- a/src/parser/validator.rs +++ b/src/parser/validator.rs @@ -1,5 +1,5 @@ // Internal -use crate::builder::{AppSettings, Arg, ArgPredicate, Command, PossibleValue}; +use crate::builder::{Arg, ArgPredicate, Command, PossibleValue}; use crate::error::{Error, Result as ClapResult}; use crate::output::fmt::Stream; use crate::output::Usage; @@ -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 fd83961eb2c..f2cf4d22038 100644 --- a/tests/builder/app_settings.rs +++ b/tests/builder/app_settings.rs @@ -2,7 +2,7 @@ use std::ffi::OsString; use super::utils; -use clap::{arg, error::ErrorKind, AppSettings, Arg, ArgAction, Command}; +use clap::{arg, error::ErrorKind, Arg, ArgAction, Command}; static ALLOW_EXT_SC: &str = "clap-test v1.4.8 @@ -67,20 +67,6 @@ OPTIONS: -V, --version Print version information "; -static SUBCOMMAND_REQUIRED_ELSE_HELP: &str = "test 1.0 - -USAGE: - test - -OPTIONS: - -h, --help Print help information - -V, --version Print version information - -SUBCOMMANDS: - help Print this message or the help of the given subcommand(s) - info -"; - #[test] fn sub_command_negate_required() { Command::new("sub_command_negate") @@ -194,37 +180,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() {