Merge pull request #3425 from epage/help

fix(assert): Provide next steps for '-h' conflicts
clap-rs · Feb 9, 2022 · 1b5f9e6 · 1b5f9e6
2 parents 341b128 + 6c60b79
commit 1b5f9e6
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 6 deletions.
diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs
@@ -2917,7 +2917,7 @@ impl<'help> App<'help> {
 
     #[allow(clippy::blocks_in_if_conditions)]
     pub(crate) fn _check_help_and_version(&mut self) {
-        debug!("App::_check_help_and_version");
+        debug!("App::_check_help_and_version: {}", self.name);
 
         if self.is_set(AppSettings::DisableHelpFlag)
             || self.args.args().any(|x| {
@@ -2940,18 +2940,39 @@ impl<'help> App<'help> {
                 self.args.remove(index);
             }
         } else {
-            let other_arg_has_short = self.args.args().any(|x| x.short == Some('h'));
             let help = self
                 .args
-                .args_mut()
+                .args()
                 .find(|x| x.id == Id::help_hash())
                 .expect(INTERNAL_ERROR_MSG);
+            assert_ne!(help.provider, ArgProvider::User);
 
-            if !(help.short.is_some()
-                || other_arg_has_short
+            if help.short.is_some() {
+                if help.short == Some('h') {
+                    if let Some(other_arg) = self
+                        .args
+                        .args()
+                        .find(|x| x.id != Id::help_hash() && x.short == Some('h'))
+                    {
+                        panic!(
+                            "`help`s `-h` conflicts with `{}`.
+
+To change `help`s short, call `app.arg(Arg::new(\"help\")...)`.",
+                            other_arg.name
+                        );
+                    }
+                }
+            } else if !(self.args.args().any(|x| x.short == Some('h'))
                 || self.subcommands.iter().any(|sc| sc.short_flag == Some('h')))
             {
+                let help = self
+                    .args
+                    .args_mut()
+                    .find(|x| x.id == Id::help_hash())
+                    .expect(INTERNAL_ERROR_MSG);
                 help.short = Some('h');
+            } else {
+                debug!("App::_check_help_and_version: Removing `-h` from help");
             }
         }
 

diff --git a/tests/builder/help.rs b/tests/builder/help.rs
@@ -1557,7 +1557,7 @@ fn arg_short_conflict_with_help() {
 
 #[cfg(debug_assertions)]
 #[test]
-#[should_panic = "Short option names must be unique for each argument, but '-h' is in use by both 'home' and 'help'"]
+#[should_panic = "`help`s `-h` conflicts with `home`."]
 fn arg_short_conflict_with_help_mut_arg() {
     let _ = App::new("conflict")
         .arg(Arg::new("home").short('h'))
@@ -2761,3 +2761,20 @@ ARGS:
 
     app.debug_assert();
 }
+
+#[test]
+fn help_without_short() {
+    let mut app = clap::App::new("test")
+        .arg(arg!(-h --hex <NUM>))
+        .arg(arg!(--help));
+
+    app._build_all();
+    let help = app
+        .get_arguments()
+        .find(|a| a.get_name() == "help")
+        .unwrap();
+    assert_eq!(help.get_short(), None);
+
+    let m = app.try_get_matches_from(["test", "-h", "0x100"]).unwrap();
+    assert_eq!(m.value_of("hex"), Some("0x100"));
+}