diff --git a/src/app/help.rs b/src/app/help.rs index f8ee21ee31b..4f3f77b2d52 100644 --- a/src/app/help.rs +++ b/src/app/help.rs @@ -514,24 +514,26 @@ impl<'w> Help<'w> { } if let Some(ref aliases) = a.aliases { debugln!("Help::spec_vals: Found aliases...{:?}", aliases); - spec_vals.push(format!( - " [aliases: {}]", - if self.color { - aliases - .iter() - .filter(|&als| als.1) // visible - .map(|&als| format!("{}", self.cizer.good(als.0))) // name - .collect::>() - .join(", ") - } else { - aliases - .iter() - .filter(|&als| als.1) - .map(|&als| als.0) - .collect::>() - .join(", ") - } - )); + let als = if self.color { + aliases + .iter() + .filter(|&als| als.1) // visible + .map(|&als| format!("{}", self.cizer.good(als.0))) // name + .collect::>() + .join(", ") + } else { + aliases + .iter() + .filter(|&als| als.1) + .map(|&als| als.0) + .collect::>() + .join(", ") + }; + if !als.is_empty() { + spec_vals.push(format!( + " [aliases: {}]", als + )); + } } if !self.hide_pv && !a.is_set(ArgSettings::HidePossibleValues) { if let Some(ref pv) = a.possible_vals { diff --git a/src/app/mod.rs b/src/app/mod.rs index fd103f0f97a..622a021951a 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -1769,13 +1769,6 @@ impl<'a, 'b> App<'a, 'b> { self.settings.set(AppSettings::DontCollapseArgsInUsage); self.settings.set(AppSettings::ContainsLast); } - if let Some(l) = a.long { - if l == "version" { - self.settings.unset(AppSettings::NeedsLongVersion); - } else if l == "help" { - self.settings.unset(AppSettings::NeedsLongHelp); - } - } a._build(); } @@ -1803,16 +1796,8 @@ impl<'a, 'b> App<'a, 'b> { // @TODO @v3-alpha @perf: should only propagate globals to subcmd we find, or for help pub fn _propagate(&mut self) { - debugln!("App::propagate:{}", self.name); + debugln!("App::_propagate:{}", self.name); for sc in &mut self.subcommands { - // We have to create a new scope in order to tell rustc the borrow of `sc` is - // done and to recursively call this method - debugln!( - "App::_propagate:{}: settings={:#?}, g_settings={:#?}", - sc.name, - sc.settings, - sc.g_settings - ); // We have to create a new scope in order to tell rustc the borrow of `sc` is // done and to recursively call this method { @@ -1836,16 +1821,17 @@ impl<'a, 'b> App<'a, 'b> { sc.args.push(a.clone()); } } + // @TODO @deadcode @perf @v3-alpha: Currently we're not propagating // sc._create_help_and_version(); // sc._propagate(); } } pub(crate) fn _create_help_and_version(&mut self) { - debugln!("App::create_help_and_version;"); + debugln!("App::_create_help_and_version;"); // name is "hclap_help" because flags are sorted by name if !self.contains_long("help") { - debugln!("App::create_help_and_version: Building --help"); + debugln!("App::_create_help_and_version: Building --help"); if self.help_short.is_none() && !self.contains_short('h') { self.help_short = Some('h'); } @@ -1857,9 +1843,11 @@ impl<'a, 'b> App<'a, 'b> { ..Default::default() }; self.args.push(arg); + } else { + self.settings.unset(AppSettings::NeedsLongHelp); } if !self.is_set(AppSettings::DisableVersion) && !self.contains_long("version") { - debugln!("App::create_help_and_version: Building --version"); + debugln!("App::_create_help_and_version: Building --version"); if self.version_short.is_none() && !self.contains_short('V') { self.version_short = Some('V'); } @@ -1872,15 +1860,19 @@ impl<'a, 'b> App<'a, 'b> { ..Default::default() }; self.args.push(arg); + } else { + self.settings.unset(AppSettings::NeedsLongVersion); } if self.has_subcommands() && !self.is_set(AppSettings::DisableHelpSubcommand) && subcommands!(self).any(|s| s.name == "help") { - debugln!("App::create_help_and_version: Building help"); + debugln!("App::_create_help_and_version: Building help"); self.subcommands.push( App::new("help") .about("Prints this message or the help of the given subcommand(s)"), ); + } else { + self.settings.unset(AppSettings::NeedsSubcommandHelp); } } @@ -1919,14 +1911,14 @@ impl<'a, 'b> App<'a, 'b> { debugln!("App::_arg_debug_asserts:{}", a.name); // No naming conflicts assert!( - arg_names!(self).filter(|name| name == &a.name).count() <= 1, + arg_names!(self).filter(|name| name == &a.name).count() < 2, format!("Non-unique argument name: {} is already in use", a.name) ); // Long conflicts if let Some(l) = a.long { assert!( - args!(self).filter(|a| a.long == Some(l)).count() <= 1, + args!(self).filter(|arg| arg.long == Some(l)).count() < 2, "Argument long must be unique\n\n\t--{} is already in use", l ); @@ -1935,7 +1927,7 @@ impl<'a, 'b> App<'a, 'b> { // Short conflicts if let Some(s) = a.short { assert!( - args!(self).filter(|a| a.short == Some(s)).count() <= 1, + args!(self).filter(|arg| arg.short == Some(s)).count() < 2, "Argument short must be unique\n\n\t-{} is already in use", s ); @@ -1944,26 +1936,14 @@ impl<'a, 'b> App<'a, 'b> { if let Some(idx) = a.index { // No index conflicts assert!( - positionals!(self) - .filter(|p| p.index == Some(idx as u64)) - .count() <= 1, + positionals!(self).filter(|p| p.index == Some(idx as u64)).count() < 2, "Argument '{}' has the same index as another positional \ argument\n\n\tUse Arg::multiple(true) to allow one positional argument \ to take multiple values", a.name ); } - assert!( - !(a.is_set(ArgSettings::Required) && a.is_set(ArgSettings::Global)), - "Global arguments cannot be required.\n\n\t'{}' is marked as \ - global and required", - a.name - ); if a.is_set(ArgSettings::Last) { - assert!( - !positionals!(self).any(|p| p.is_set(ArgSettings::Last)), - "Only one positional argument may have last(true) set. Found two." - ); assert!(a.long.is_none(), "Flags or Options may not have last(true) set. {} has both a long and last(true) set.", a.name); @@ -1971,6 +1951,13 @@ impl<'a, 'b> App<'a, 'b> { "Flags or Options may not have last(true) set. {} has both a short and last(true) set.", a.name); } + assert!( + !(a.is_set(ArgSettings::Required) && a.is_set(ArgSettings::Global)), + "Global arguments cannot be required.\n\n\t'{}' is marked as \ + global and required", + a.name + ); + true } diff --git a/src/app/parser.rs b/src/app/parser.rs index 107197b933c..9ece331bf77 100644 --- a/src/app/parser.rs +++ b/src/app/parser.rs @@ -135,10 +135,14 @@ where // * ArgSettings::Last // * The last arg is Required let mut it = self.positionals.values().rev(); - let last = - find!(self.app, it.next().expect(INTERNAL_ERROR_MSG)).expect(INTERNAL_ERROR_MSG); - let second_to_last = - find!(self.app, it.next().expect(INTERNAL_ERROR_MSG)).expect(INTERNAL_ERROR_MSG); + + // We can't pass the closure (it.next()) to the macro directly because each call to + // find() (iterator, not macro) gets called repeatedly. + let last_name = it.next().expect(INTERNAL_ERROR_MSG); + let second_to_last_name = it.next().expect(INTERNAL_ERROR_MSG); + let last = find!(self.app, last_name).expect(INTERNAL_ERROR_MSG); + let second_to_last = find!(self.app, second_to_last_name).expect(INTERNAL_ERROR_MSG); + // Either the final positional is required // Or the second to last has a terminator or .last(true) set let ok = last.is_set(ArgSettings::Required) @@ -239,6 +243,12 @@ where } } } + assert!( + positionals!(self.app) + .filter(|p| p.is_set(ArgSettings::Last)) + .count() < 2, + "Only one positional argument may have last(true) set. Found two." + ); if positionals!(self.app) .any(|p| p.is_set(ArgSettings::Last) && p.is_set(ArgSettings::Required)) && self.has_subcommands() && !self.is_set(AS::SubcommandsNegateReqs) @@ -891,11 +901,14 @@ where "Parser::check_for_help_and_version_str: Checking if --{} is help or version...", arg.to_str().unwrap() ); - if arg == "help" && self.is_set(AS::NeedsLongHelp) { + + // Needs to use app.settings.is_set instead of just is_set() because is_set() checks + // both global and local settings, we only want to check local + if arg == "help" && self.app.settings.is_set(AS::NeedsLongHelp) { sdebugln!("Help"); return Err(self.help_err(true)); } - if arg == "version" && self.is_set(AS::NeedsLongVersion) { + if arg == "version" && self.app.settings.is_set(AS::NeedsLongVersion) { sdebugln!("Version"); return Err(self.version_err(true)); } @@ -910,14 +923,16 @@ where "Parser::check_for_help_and_version_char: Checking if -{} is help or version...", arg ); + // Needs to use app.settings.is_set instead of just is_set() because is_set() checks + // both global and local settings, we only want to check local if let Some(h) = self.app.help_short { - if arg == h && self.is_set(AS::NeedsLongHelp) { + if arg == h && self.app.settings.is_set(AS::NeedsLongHelp) { sdebugln!("Help"); return Err(self.help_err(false)); } } if let Some(v) = self.app.version_short { - if arg == v && self.is_set(AS::NeedsLongVersion) { + if arg == v && self.app.settings.is_set(AS::NeedsLongVersion) { sdebugln!("Version"); return Err(self.version_err(false)); } diff --git a/src/macros.rs b/src/macros.rs index 65d8563dff1..6d0f746d72e 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -857,13 +857,15 @@ macro_rules! args { $app.args.$how() }; ($app:expr) => { - opts!($app, iter) + args!($app, iter) } } macro_rules! flags { ($app:expr, $how:ident) => { - $app.args.$how().filter(|a| !a.settings.is_set(::args::settings::ArgSettings::TakesValue) && (a.short.is_some() || a.long.is_some())) + $app.args.$how() + .filter(|a| !a.settings.is_set(::args::settings::ArgSettings::TakesValue)) + .filter(|a| a.short.is_some() || a.long.is_some()) }; ($app:expr) => { flags!($app, iter) @@ -878,7 +880,9 @@ macro_rules! flags_mut { macro_rules! opts { ($app:expr, $how:ident) => { - $app.args.$how().filter(|a| a.settings.is_set(::args::settings::ArgSettings::TakesValue) && (a.short.is_some() || a.long.is_some())) + $app.args.$how() + .filter(|a| a.settings.is_set(::args::settings::ArgSettings::TakesValue)) + .filter(|a| a.short.is_some() || a.long.is_some()) }; ($app:expr) => { opts!($app, iter)