fix(derive): Move off of SubcommandRequiredElseHelp

This also let us remove the deprecated attribute Fixes clap-rs#3280
epage · Jul 22, 2022 · 8b064cf · 8b064cf
1 parent 50019ca
commit 8b064cf
Show file tree

Hide file tree

Showing 11 changed files with 22 additions and 88 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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);
                 <Self as clap::Subcommand>::augment_subcommands(#app_var)
             }
 

diff --git 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
@@ -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
@@ -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] <SUBCOMMAND>
 
-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
@@ -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
@@ -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
@@ -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
@@ -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 <SUBCOMMAND>
-
-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() {