From 0aabcd70c169130b6034174e903eb2536cc60149 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 8 Feb 2022 18:51:44 -0600 Subject: [PATCH 1/3] test: Ensure we can avoid a short on help --- tests/builder/help.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/builder/help.rs b/tests/builder/help.rs index b5c18631..c194b0f7 100644 --- a/tests/builder/help.rs +++ b/tests/builder/help.rs @@ -2761,3 +2761,20 @@ ARGS: app.debug_assert(); } + +#[test] +fn help_without_short() { + let mut app = clap::App::new("test") + .arg(arg!(-h --hex )) + .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")); +} From df1d50c367715d812075bfb458bacfdf7be0f6c2 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Sat, 5 Feb 2022 09:00:41 -0600 Subject: [PATCH 2/3] fix: Easier to debug help flags --- src/build/app/mod.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index d6174a71..317f9514 100644 --- 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| { @@ -2947,11 +2947,13 @@ impl<'help> App<'help> { .find(|x| x.id == Id::help_hash()) .expect(INTERNAL_ERROR_MSG); - if !(help.short.is_some() - || other_arg_has_short + if help.short.is_some() { + } else if !(other_arg_has_short || self.subcommands.iter().any(|sc| sc.short_flag == Some('h'))) { help.short = Some('h'); + } else { + debug!("App::_check_help_and_version: Removing `-h` from help"); } } From 6c60b79d217fc0829094971eaaa5b9ce34d2dc08 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 8 Feb 2022 18:44:46 -0600 Subject: [PATCH 3/3] fix(assert): Provide next steps for '-h' conflicts For now, we are going to provide a better debug assert in this case. Resolving #3405 is the better long term route. Fixes #3403 --- src/build/app/mod.rs | 25 ++++++++++++++++++++++--- tests/builder/help.rs | 2 +- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/build/app/mod.rs b/src/build/app/mod.rs index 317f9514..ccc733e7 100644 --- a/src/build/app/mod.rs +++ b/src/build/app/mod.rs @@ -2940,17 +2940,36 @@ 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() { - } else if !(other_arg_has_short + 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 index c194b0f7..e4d3bb9b 100644 --- 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'))