diff --git a/src/builder/action.rs b/src/builder/action.rs new file mode 100644 index 00000000000..71da9ccc27e --- /dev/null +++ b/src/builder/action.rs @@ -0,0 +1,8 @@ +/// Behavior of arguments when they are encountered while parsing +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) enum Action { + StoreValue, + Flag, + Help, + Version, +} diff --git a/src/builder/arg.rs b/src/builder/arg.rs index 49cb8a96819..46676adbd94 100644 --- a/src/builder/arg.rs +++ b/src/builder/arg.rs @@ -18,6 +18,7 @@ use yaml_rust::Yaml; // Internal use crate::builder::usage_parser::UsageParser; +use crate::builder::Action; use crate::builder::ArgPredicate; use crate::util::{Id, Key}; use crate::PossibleValue; @@ -63,6 +64,7 @@ pub struct Arg<'help> { pub(crate) name: &'help str, pub(crate) help: Option<&'help str>, pub(crate) long_help: Option<&'help str>, + pub(crate) action: Option, pub(crate) value_parser: Option, pub(crate) blacklist: Vec, pub(crate) settings: ArgFlags, @@ -4528,6 +4530,12 @@ impl<'help> Arg<'help> { self.is_set(ArgSettings::AllowInvalidUtf8) } + /// Behavior when parsing the argument + pub(crate) fn get_action(&self) -> &super::Action { + const DEFAULT: super::Action = super::Action::StoreValue; + self.action.as_ref().unwrap_or(&DEFAULT) + } + /// Configured parser for argument values /// /// # Example diff --git a/src/builder/command.rs b/src/builder/command.rs index b54012bd3dc..0a7ce146bc4 100644 --- a/src/builder/command.rs +++ b/src/builder/command.rs @@ -4115,6 +4115,10 @@ impl<'help> App<'help> { let mut pos_counter = 1; let self_override = self.is_set(AppSettings::AllArgsOverrideSelf); let hide_pv = self.is_set(AppSettings::HidePossibleValues); + let auto_help = + !self.is_set(AppSettings::NoAutoHelp) && !self.is_disable_help_flag_set(); + let auto_version = + !self.is_set(AppSettings::NoAutoVersion) && !self.is_disable_version_flag_set(); for a in self.args.args_mut() { // Fill in the groups for g in &a.groups { @@ -4141,6 +4145,24 @@ impl<'help> App<'help> { a.overrides.push(self_id); } a._build(); + // HACK: Setting up action at this level while auto-help / disable help flag is + // required. Otherwise, most of this won't be needed because when we can break + // compat, actions will reign supreme (default to `Store`) + if a.action.is_none() { + if a.get_id() == "help" && auto_help { + let action = super::Action::Help; + a.action = Some(action); + } else if a.get_id() == "version" && auto_version { + let action = super::Action::Version; + a.action = Some(action); + } else if a.is_takes_value_set() { + let action = super::Action::StoreValue; + a.action = Some(action); + } else { + let action = super::Action::Flag; + a.action = Some(action); + } + } if a.is_positional() && a.index.is_none() { a.index = Some(pos_counter); pos_counter += 1; diff --git a/src/builder/mod.rs b/src/builder/mod.rs index 9b009ea31fe..10e8bdff980 100644 --- a/src/builder/mod.rs +++ b/src/builder/mod.rs @@ -3,6 +3,7 @@ #[macro_use] mod macros; +mod action; mod app_settings; mod arg; mod arg_group; @@ -53,6 +54,7 @@ pub use command::App; #[cfg(feature = "regex")] pub use self::regex::RegexRef; +pub(crate) use action::Action; pub(crate) use arg::display_arg_val; pub(crate) use arg_predicate::ArgPredicate; pub(crate) use value_parser::ValueParserInner; diff --git a/src/parser/arg_matcher.rs b/src/parser/arg_matcher.rs index bb26bc3f261..e162a16f14f 100644 --- a/src/parser/arg_matcher.rs +++ b/src/parser/arg_matcher.rs @@ -11,9 +11,6 @@ use crate::parser::{ArgMatches, MatchedArg, SubCommand, ValueSource}; use crate::util::Id; use crate::INTERNAL_ERROR_MSG; -// Third party -use indexmap::map::Entry; - #[derive(Debug, Default)] pub(crate) struct ArgMatcher(ArgMatches); @@ -141,6 +138,7 @@ impl ArgMatcher { let ma = self.entry(id).or_insert(MatchedArg::new_arg(arg)); debug_assert_eq!(ma.type_id(), Some(arg.get_value_parser().type_id())); ma.set_source(source); + ma.new_val_group(); } pub(crate) fn start_custom_group(&mut self, id: &Id, source: ValueSource) { @@ -151,6 +149,7 @@ impl ArgMatcher { let ma = self.entry(id).or_insert(MatchedArg::new_group()); debug_assert_eq!(ma.type_id(), None); ma.set_source(source); + ma.new_val_group(); } pub(crate) fn start_occurrence_of_arg(&mut self, arg: &Arg) { @@ -160,6 +159,7 @@ impl ArgMatcher { debug_assert_eq!(ma.type_id(), Some(arg.get_value_parser().type_id())); ma.set_source(ValueSource::CommandLine); ma.inc_occurrences(); + ma.new_val_group(); } pub(crate) fn start_occurrence_of_group(&mut self, id: &Id) { @@ -168,6 +168,7 @@ impl ArgMatcher { debug_assert_eq!(ma.type_id(), None); ma.set_source(ValueSource::CommandLine); ma.inc_occurrences(); + ma.new_val_group(); } pub(crate) fn start_occurrence_of_external(&mut self, cmd: &crate::Command) { @@ -184,46 +185,19 @@ impl ArgMatcher { ); ma.set_source(ValueSource::CommandLine); ma.inc_occurrences(); + ma.new_val_group(); } - pub(crate) fn add_val_to(&mut self, arg: &Id, val: AnyValue, raw_val: OsString, append: bool) { - if append { - self.append_val_to(arg, val, raw_val); - } else { - self.push_val_to(arg, val, raw_val); - } - } - - fn push_val_to(&mut self, arg: &Id, val: AnyValue, raw_val: OsString) { - // We will manually inc occurrences later(for flexibility under - // specific circumstances, like only add one occurrence for flag - // when we met: `--flag=one,two`). - let ma = self.get_mut(arg).expect(INTERNAL_ERROR_MSG); - ma.push_val(val, raw_val); - } - - fn append_val_to(&mut self, arg: &Id, val: AnyValue, raw_val: OsString) { + pub(crate) fn add_val_to(&mut self, arg: &Id, val: AnyValue, raw_val: OsString) { let ma = self.get_mut(arg).expect(INTERNAL_ERROR_MSG); ma.append_val(val, raw_val); } - pub(crate) fn new_val_group(&mut self, arg: &Id) { - let ma = self.get_mut(arg).expect(INTERNAL_ERROR_MSG); - ma.new_val_group(); - } - pub(crate) fn add_index_to(&mut self, arg: &Id, idx: usize) { let ma = self.get_mut(arg).expect(INTERNAL_ERROR_MSG); ma.push_index(idx); } - pub(crate) fn has_val_groups(&mut self, arg: &Id) -> bool { - match self.entry(arg) { - Entry::Occupied(e) => e.get().has_val_groups(), - Entry::Vacant(_) => false, - } - } - pub(crate) fn needs_more_vals(&self, o: &Arg) -> bool { debug!("ArgMatcher::needs_more_vals: o={}", o.name); if let Some(ma) = self.get(&o.id) { diff --git a/src/parser/matches/matched_arg.rs b/src/parser/matches/matched_arg.rs index fa9f7664cac..b2c527eaab0 100644 --- a/src/parser/matches/matched_arg.rs +++ b/src/parser/matches/matched_arg.rs @@ -87,11 +87,6 @@ impl MatchedArg { self.indices.push(index) } - #[cfg(test)] - pub(crate) fn raw_vals(&self) -> Iter> { - self.raw_vals.iter() - } - #[cfg(feature = "unstable-grouped")] pub(crate) fn vals(&self) -> Iter> { self.vals.iter() @@ -118,11 +113,6 @@ impl MatchedArg { self.raw_vals_flatten().next() } - pub(crate) fn push_val(&mut self, val: AnyValue, raw_val: OsString) { - self.vals.push(vec![val]); - self.raw_vals.push(vec![raw_val]); - } - pub(crate) fn new_val_group(&mut self) { self.vals.push(vec![]); self.raw_vals.push(vec![]); @@ -151,10 +141,6 @@ impl MatchedArg { self.vals.iter().flatten().count() == 0 } - pub(crate) fn has_val_groups(&self) -> bool { - !self.vals.is_empty() - } - pub(crate) fn check_explicit(&self, predicate: ArgPredicate) -> bool { if self.source == Some(ValueSource::DefaultValue) { return false; @@ -244,38 +230,4 @@ mod tests { m.append_val(AnyValue::new(String::from("ccc")), "ccc".into()); assert_eq!(m.first_raw(), Some(&OsString::from("bbb"))); } - - #[test] - fn test_grouped_vals_push_and_append() { - let mut m = MatchedArg::new_group(); - m.push_val(AnyValue::new(String::from("aaa")), "aaa".into()); - m.new_val_group(); - m.append_val(AnyValue::new(String::from("bbb")), "bbb".into()); - m.append_val(AnyValue::new(String::from("ccc")), "ccc".into()); - m.new_val_group(); - m.append_val(AnyValue::new(String::from("ddd")), "ddd".into()); - m.push_val(AnyValue::new(String::from("eee")), "eee".into()); - m.new_val_group(); - m.append_val(AnyValue::new(String::from("fff")), "fff".into()); - m.append_val(AnyValue::new(String::from("ggg")), "ggg".into()); - m.append_val(AnyValue::new(String::from("hhh")), "hhh".into()); - m.append_val(AnyValue::new(String::from("iii")), "iii".into()); - - let vals: Vec<&Vec> = m.raw_vals().collect(); - assert_eq!( - vals, - vec![ - &vec![OsString::from("aaa")], - &vec![OsString::from("bbb"), OsString::from("ccc"),], - &vec![OsString::from("ddd")], - &vec![OsString::from("eee")], - &vec![ - OsString::from("fff"), - OsString::from("ggg"), - OsString::from("hhh"), - OsString::from("iii"), - ] - ] - ) - } } diff --git a/src/parser/parser.rs b/src/parser/parser.rs index 14e4c223e7f..3e2b22827ba 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -8,6 +8,7 @@ use std::{ use clap_lex::RawOsStr; // Internal +use crate::builder::Action; use crate::builder::AppSettings as AS; use crate::builder::{Arg, Command}; use crate::error::Error as ClapError; @@ -182,8 +183,10 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { && !self.cmd.is_disable_help_subcommand_set() { self.parse_help_subcommand(raw_args.remaining(&mut args_cursor))?; + unreachable!("`parse_help_subcommand` always errors"); + } else { + subcmd_name = Some(sc_name.to_owned()); } - subcmd_name = Some(sc_name.to_owned()); break; } } @@ -253,12 +256,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { Usage::new(self.cmd).create_usage_no_title(&used), )) } - ParseResult::HelpFlag => { - return Err(self.help_err(true, Stream::Stdout)); - } - ParseResult::VersionFlag => { - return Err(self.version_err(true)); - } ParseResult::MaybeHyphenValue => { // Maybe a hyphen value, do nothing. } @@ -336,12 +333,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { Usage::new(self.cmd).create_usage_with_title(&[]), )); } - ParseResult::HelpFlag => { - return Err(self.help_err(false, Stream::Stdout)); - } - ParseResult::VersionFlag => { - return Err(self.version_err(false)); - } ParseResult::MaybeHyphenValue => { // Maybe a hyphen value, do nothing. } @@ -358,7 +349,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { &self.cmd[id], arg_os.to_value_os(), matcher, - true, trailing_values, )?; parse_state = match parse_result { @@ -385,15 +375,15 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { trailing_values = true; } - // Increase occurrence no matter if we are appending, occurrences - // of positional argument equals to number of values rather than - // the number of value groups. - self.start_occurrence_of_arg(matcher, p); - // Creating new value group rather than appending when the arg - // doesn't have any value. This behaviour is right because - // positional arguments are always present continuously. - let append = self.has_val_groups(matcher, p); - self.add_val_to_arg(p, arg_os.to_value_os(), matcher, append, trailing_values)?; + if !p.is_multiple_values_set() || !matcher.contains(&p.id) { + self.start_occurrence_of_arg(matcher, p); + } + let _ignored_result = + self.add_val_to_arg(p, arg_os.to_value_os(), matcher, trailing_values)?; + debug!( + "Parser::get_matches_with: Ignoring state {:?}; positionals do their own thing", + _ignored_result + ); // Only increment the positional counter if it doesn't allow multiples if !p.is_multiple() { @@ -426,7 +416,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { for raw_val in raw_args.remaining(&mut args_cursor) { let val = external_parser.parse_ref(self.cmd, None, raw_val)?; let external_id = &Id::empty_hash(); - sc_m.add_val_to(external_id, val, raw_val.to_os_string(), false); + sc_m.add_val_to(external_id, val, raw_val.to_os_string()); } matcher.subcommand(SubCommand { @@ -436,8 +426,8 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { }); #[cfg(feature = "env")] - self.add_env(matcher, trailing_values)?; - self.add_defaults(matcher, trailing_values)?; + self.add_env(matcher)?; + self.add_defaults(matcher)?; return Validator::new(self.cmd).validate(parse_state, matcher); } else { // Start error processing @@ -456,8 +446,8 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { } #[cfg(feature = "env")] - self.add_env(matcher, trailing_values)?; - self.add_defaults(matcher, trailing_values)?; + self.add_env(matcher)?; + self.add_defaults(matcher)?; Validator::new(self.cmd).validate(parse_state, matcher) } @@ -586,7 +576,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { fn parse_help_subcommand( &self, cmds: impl Iterator, - ) -> ClapResult { + ) -> ClapResult { debug!("Parser::parse_help_subcommand"); let mut cmd = self.cmd.clone(); @@ -695,74 +685,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { Ok(()) } - // Retrieves the names of all args the user has supplied thus far, except required ones - // because those will be listed in self.required - fn check_for_help_and_version_str(&self, arg: &RawOsStr) -> Option { - debug!("Parser::check_for_help_and_version_str"); - debug!( - "Parser::check_for_help_and_version_str: Checking if --{:?} is help or version...", - arg - ); - - if let Some(help) = self.cmd.find(&Id::help_hash()) { - if let Some(h) = help.long { - if arg == h && !self.is_set(AS::NoAutoHelp) && !self.cmd.is_disable_help_flag_set() - { - debug!("Help"); - return Some(ParseResult::HelpFlag); - } - } - } - - if let Some(version) = self.cmd.find(&Id::version_hash()) { - if let Some(v) = version.long { - if arg == v - && !self.is_set(AS::NoAutoVersion) - && !self.cmd.is_disable_version_flag_set() - { - debug!("Version"); - return Some(ParseResult::VersionFlag); - } - } - } - - debug!("Neither"); - None - } - - fn check_for_help_and_version_char(&self, arg: char) -> Option { - debug!("Parser::check_for_help_and_version_char"); - debug!( - "Parser::check_for_help_and_version_char: Checking if -{} is help or version...", - arg - ); - - if let Some(help) = self.cmd.find(&Id::help_hash()) { - if let Some(h) = help.short { - if arg == h && !self.is_set(AS::NoAutoHelp) && !self.cmd.is_disable_help_flag_set() - { - debug!("Help"); - return Some(ParseResult::HelpFlag); - } - } - } - - if let Some(version) = self.cmd.find(&Id::version_hash()) { - if let Some(v) = version.short { - if arg == v - && !self.is_set(AS::NoAutoVersion) - && !self.cmd.is_disable_version_flag_set() - { - debug!("Version"); - return Some(ParseResult::VersionFlag); - } - } - } - - debug!("Neither"); - None - } - fn parse_long_arg( &mut self, matcher: &mut ArgMatcher, @@ -804,30 +726,37 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { "Parser::parse_long_arg: Found valid opt or flag '{}'", opt.to_string() ); - Some(opt) + Some((long_arg, opt)) } else if self.cmd.is_infer_long_args_set() { - self.cmd.get_arguments().find(|a| { - a.long.map_or(false, |long| long.starts_with(long_arg)) - || a.aliases - .iter() - .any(|(alias, _)| alias.starts_with(long_arg)) + self.cmd.get_arguments().find_map(|a| { + if let Some(long) = a.long { + if long.starts_with(long_arg) { + return Some((long, a)); + } + } + a.aliases + .iter() + .find_map(|(alias, _)| alias.starts_with(long_arg).then(|| (*alias, a))) }) } else { None }; - if let Some(opt) = opt { + if let Some((long_arg, opt)) = opt { *valid_arg_found = true; if opt.is_takes_value_set() { debug!( - "Parser::parse_long_arg: Found an opt with value '{:?}'", - &long_value + "Parser::parse_long_arg({:?}): Found an opt with value '{:?}'", + long_arg, &long_value ); let has_eq = long_value.is_some(); - self.parse_opt(long_value, opt, matcher, trailing_values, has_eq) + self.parse_opt_value(long_value, opt, matcher, trailing_values, has_eq) } else if let Some(rest) = long_value { let required = self.cmd.required_graph(); - debug!("Parser::parse_long_arg: Got invalid literal `{:?}`", rest); + debug!( + "Parser::parse_long_arg({:?}): Got invalid literal `{:?}`", + long_arg, rest + ); let used: Vec = matcher .arg_names() .filter(|&n| { @@ -843,13 +772,9 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { used, arg: opt.to_string(), }) - } else if let Some(parse_result) = - self.check_for_help_and_version_str(RawOsStr::from_str(long_arg)) - { - Ok(parse_result) } else { - debug!("Parser::parse_long_arg: Presence validated"); - Ok(self.parse_flag(opt, matcher)) + debug!("Parser::parse_long_arg({:?}): Presence validated", long_arg); + self.parse_flag(Identifier::Long(long_arg), opt, matcher) } } else if let Some(sc_name) = self.possible_long_flag_subcommand(long_arg) { Ok(ParseResult::FlagSubCommand(sc_name.to_string())) @@ -946,10 +871,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { ); *valid_arg_found = true; if !opt.is_takes_value_set() { - if let Some(parse_result) = self.check_for_help_and_version_char(c) { - return Ok(parse_result); - } - ret = self.parse_flag(opt, matcher); + ret = self.parse_flag(Identifier::Short(c), opt, matcher)?; continue; } @@ -975,7 +897,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { } else { (val, false) }; - match self.parse_opt(val, opt, matcher, trailing_values, has_eq)? { + match self.parse_opt_value(val, opt, matcher, trailing_values, has_eq)? { ParseResult::AttachedValueNotConsumed => continue, x => return Ok(x), } @@ -1003,7 +925,7 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { Ok(ret) } - fn parse_opt( + fn parse_opt_value( &self, attached_value: Option<&RawOsStr>, opt: &Arg<'help>, @@ -1012,12 +934,12 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { has_eq: bool, ) -> ClapResult { debug!( - "Parser::parse_opt; opt={}, val={:?}, has_eq={:?}", + "Parser::parse_opt_value; opt={}, val={:?}, has_eq={:?}", opt.name, attached_value, has_eq ); - debug!("Parser::parse_opt; opt.settings={:?}", opt.settings); + debug!("Parser::parse_opt_value; opt.settings={:?}", opt.settings); - debug!("Parser::parse_opt; Checking for val..."); + debug!("Parser::parse_opt_value; Checking for val..."); // require_equals is set, but no '=' is provided, try throwing error. if opt.is_require_equals_set() && !has_eq { if opt.min_vals == Some(0) { @@ -1025,13 +947,14 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { self.start_occurrence_of_arg(matcher, opt); // We assume this case is valid: require equals, but min_vals == 0. if !opt.default_missing_vals.is_empty() { - debug!("Parser::parse_opt: has default_missing_vals"); - self.add_multiple_vals_to_arg( - opt, - opt.default_missing_vals.iter().map(OsString::from), - matcher, - false, - )?; + debug!("Parser::parse_opt_value: has default_missing_vals"); + for v in opt.default_missing_vals.iter() { + let _ignored_result = + self.add_val_to_arg(opt, &RawOsStr::new(v), matcher, trailing_values)?; + if _ignored_result != ParseResult::ValuesDone { + debug!("Parser::parse_opt_value: Ignoring state {:?}; no values accepted after default_missing_values", _ignored_result); + } + } }; if attached_value.is_some() { Ok(ParseResult::AttachedValueNotConsumed) @@ -1046,15 +969,15 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { } } else if let Some(v) = attached_value { self.start_occurrence_of_arg(matcher, opt); - self.add_val_to_arg(opt, v, matcher, false, trailing_values)?; - Ok(ParseResult::ValuesDone) + let mut val_result = self.add_val_to_arg(opt, v, matcher, trailing_values)?; + if val_result != ParseResult::ValuesDone { + debug!("Parser::parse_opt_value: Overriding state {:?}; no values accepted after attached", val_result); + val_result = ParseResult::ValuesDone; + } + Ok(val_result) } else { - debug!("Parser::parse_opt: More arg vals required..."); + debug!("Parser::parse_opt_value: More arg vals required..."); self.start_occurrence_of_arg(matcher, opt); - matcher.new_val_group(&opt.id); - for group in self.cmd.groups_for_arg(&opt.id) { - matcher.new_val_group(&group); - } Ok(ParseResult::Opt(opt.id.clone())) } } @@ -1064,7 +987,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { arg: &Arg<'help>, val: &RawOsStr, matcher: &mut ArgMatcher, - append: bool, trailing_values: bool, ) -> ClapResult { debug!("Parser::add_val_to_arg; arg={}, val={:?}", arg.name, val); @@ -1073,59 +995,35 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { trailing_values, self.cmd.is_dont_delimit_trailing_values_set() ); - if !(trailing_values && self.cmd.is_dont_delimit_trailing_values_set()) { - if let Some(delim) = arg.val_delim { - let terminator = arg.terminator.map(OsStr::new); - let vals = val - .split(delim) - .map(|x| x.to_os_str().into_owned()) - .take_while(|val| Some(val.as_os_str()) != terminator); - self.add_multiple_vals_to_arg(arg, vals, matcher, append)?; - // If there was a delimiter used or we must use the delimiter to - // separate the values or no more vals is needed, we're not - // looking for more values. - return if val.contains(delim) - || arg.is_require_value_delimiter_set() - || !matcher.needs_more_vals(arg) - { - Ok(ParseResult::ValuesDone) - } else { - Ok(ParseResult::Opt(arg.id.clone())) - }; - } + + let mut delim = arg.val_delim; + if trailing_values && self.cmd.is_dont_delimit_trailing_values_set() { + delim = None; } - if let Some(t) = arg.terminator { - if t == val { - return Ok(ParseResult::ValuesDone); + match delim { + Some(delim) if val.contains(delim) => { + let vals = val.split(delim).map(|x| x.to_os_str().into_owned()); + for raw_val in vals { + if Some(raw_val.as_os_str()) == arg.terminator.map(OsStr::new) { + return Ok(ParseResult::ValuesDone); + } + self.add_single_val_to_arg(arg, raw_val, matcher)?; + } + // Delimited values are always considered the final value + Ok(ParseResult::ValuesDone) } - } - self.add_single_val_to_arg(arg, val.to_os_str().into_owned(), matcher, append)?; - if matcher.needs_more_vals(arg) { - Ok(ParseResult::Opt(arg.id.clone())) - } else { - Ok(ParseResult::ValuesDone) - } - } - - fn add_multiple_vals_to_arg( - &self, - arg: &Arg<'help>, - raw_vals: impl Iterator, - matcher: &mut ArgMatcher, - append: bool, - ) -> ClapResult<()> { - // If not appending, create a new val group and then append vals in. - if !append { - matcher.new_val_group(&arg.id); - for group in self.cmd.groups_for_arg(&arg.id) { - matcher.new_val_group(&group); + _ if Some(val) == arg.terminator.map(RawOsStr::from_str) => Ok(ParseResult::ValuesDone), + _ => { + self.add_single_val_to_arg(arg, val.to_os_str().into_owned(), matcher)?; + if arg.is_require_value_delimiter_set() { + Ok(ParseResult::ValuesDone) + } else if matcher.needs_more_vals(arg) { + Ok(ParseResult::Opt(arg.id.clone())) + } else { + Ok(ParseResult::ValuesDone) + } } } - for raw_val in raw_vals { - self.add_single_val_to_arg(arg, raw_val, matcher, true)?; - } - - Ok(()) } fn add_single_val_to_arg( @@ -1133,7 +1031,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { arg: &Arg<'help>, raw_val: OsString, matcher: &mut ArgMatcher, - append: bool, ) -> ClapResult<()> { debug!("Parser::add_single_val_to_arg: adding val...{:?}", raw_val); @@ -1148,26 +1045,47 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { // Increment or create the group "args" for group in self.cmd.groups_for_arg(&arg.id) { - matcher.add_val_to(&group, val.clone(), raw_val.clone(), append); + matcher.add_val_to(&group, val.clone(), raw_val.clone()); } - matcher.add_val_to(&arg.id, val, raw_val, append); + matcher.add_val_to(&arg.id, val, raw_val); matcher.add_index_to(&arg.id, self.cur_idx.get()); Ok(()) } - fn has_val_groups(&self, matcher: &mut ArgMatcher, arg: &Arg<'help>) -> bool { - matcher.has_val_groups(&arg.id) - } - - fn parse_flag(&self, flag: &Arg<'help>, matcher: &mut ArgMatcher) -> ParseResult { + fn parse_flag( + &self, + ident: Identifier, + arg: &Arg<'help>, + matcher: &mut ArgMatcher, + ) -> ClapResult { debug!("Parser::parse_flag"); + match arg.get_action() { + Action::StoreValue => unreachable!("{:?} is not a flag", arg.get_id()), + Action::Flag => { + self.start_occurrence_of_arg(matcher, arg); + matcher.add_index_to(&arg.id, self.cur_idx.get()); - self.start_occurrence_of_arg(matcher, flag); - matcher.add_index_to(&flag.id, self.cur_idx.get()); - - ParseResult::ValuesDone + Ok(ParseResult::ValuesDone) + } + Action::Help => { + let use_long = match ident { + Identifier::Long(_) => true, + Identifier::Short(_) => false, + }; + debug!("Help: use_long={}", use_long); + Err(self.help_err(use_long, Stream::Stdout)) + } + Action::Version => { + let use_long = match ident { + Identifier::Long(_) => true, + Identifier::Short(_) => false, + }; + debug!("Version: use_long={}", use_long); + Err(self.version_err(use_long)) + } + } } fn remove_overrides(&self, arg: &Arg<'help>, matcher: &mut ArgMatcher) { @@ -1193,17 +1111,15 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { } #[cfg(feature = "env")] - fn add_env(&mut self, matcher: &mut ArgMatcher, trailing_values: bool) -> ClapResult<()> { + fn add_env(&mut self, matcher: &mut ArgMatcher) -> ClapResult<()> { debug!("Parser::add_env"); use crate::util::str_to_bool; + let trailing_values = false; // defaults are independent of the commandline for arg in self.cmd.get_arguments() { // Use env only if the arg was absent among command line args, // early return if this is not the case. - if matcher - .get(&arg.id) - .map_or(false, |arg| arg.get_occurrences() != 0) - { + if matcher.contains(&arg.id) { debug!("Parser::add_env: Skipping existing arg `{}`", arg); continue; } @@ -1218,17 +1134,15 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { val, trailing_values ); self.start_custom_arg(matcher, arg, ValueSource::EnvVariable); - self.add_val_to_arg(arg, &val, matcher, false, trailing_values)?; + let _ignored_result = + self.add_val_to_arg(arg, &val, matcher, trailing_values)?; + if _ignored_result != ParseResult::ValuesDone { + debug!("Parser::add_env: Ignoring state {:?}; env variables are outside of the parse loop", _ignored_result); + } } else { - // Early return on `HelpFlag` or `VersionFlag`. - match self.check_for_help_and_version_str(&val) { - Some(ParseResult::HelpFlag) => { - return Err(self.help_err(true, Stream::Stdout)); - } - Some(ParseResult::VersionFlag) => { - return Err(self.version_err(true)); - } - _ => { + match arg.get_action() { + Action::StoreValue => unreachable!("{:?} is not a flag", arg.get_id()), + Action::Flag => { debug!("Parser::add_env: Found a flag with value `{:?}`", val); let predicate = str_to_bool(val.to_str_lossy()); debug!("Parser::add_env: Found boolean literal `{:?}`", predicate); @@ -1237,6 +1151,13 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { matcher.add_index_to(&arg.id, self.cur_idx.get()); } } + // Early return on `Help` or `Version`. + Action::Help => { + return Err(self.help_err(true, Stream::Stdout)); + } + Action::Version => { + return Err(self.version_err(true)); + } } } } @@ -1245,31 +1166,69 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { Ok(()) } - fn add_defaults(&mut self, matcher: &mut ArgMatcher, trailing_values: bool) -> ClapResult<()> { + fn add_defaults(&mut self, matcher: &mut ArgMatcher) -> ClapResult<()> { debug!("Parser::add_defaults"); for o in self.cmd.get_opts() { debug!("Parser::add_defaults:iter:{}:", o.name); - self.add_default_value(o, matcher, trailing_values)?; + self.add_default_value(o, matcher)?; } for p in self.cmd.get_positionals() { debug!("Parser::add_defaults:iter:{}:", p.name); - self.add_default_value(p, matcher, trailing_values)?; + self.add_default_value(p, matcher)?; } Ok(()) } - fn add_default_value( - &self, - arg: &Arg<'help>, - matcher: &mut ArgMatcher, - trailing_values: bool, - ) -> ClapResult<()> { + fn add_default_value(&self, arg: &Arg<'help>, matcher: &mut ArgMatcher) -> ClapResult<()> { + let trailing_values = false; // defaults are independent of the commandline + + if !arg.default_missing_vals.is_empty() { + debug!( + "Parser::add_default_value:iter:{}: has default missing vals", + arg.name + ); + match matcher.get(&arg.id) { + Some(ma) if ma.all_val_groups_empty() => { + debug!( + "Parser::add_default_value:iter:{}: has no user defined vals", + arg.name + ); + // The flag occurred, we just want to add the val groups + self.start_custom_arg(matcher, arg, ValueSource::CommandLine); + for v in arg.default_missing_vals.iter() { + let _ignored_result = + self.add_val_to_arg(arg, &RawOsStr::new(v), matcher, trailing_values)?; + if _ignored_result != ParseResult::ValuesDone { + debug!("Parser::add_default_value: Ignoring state {:?}; defaults are outside of the parse loop", _ignored_result); + } + } + } + None => { + debug!("Parser::add_default_value:iter:{}: wasn't used", arg.name); + // do nothing + } + _ => { + debug!( + "Parser::add_default_value:iter:{}: has user defined vals", + arg.name + ); + // do nothing + } + } + } else { + debug!( + "Parser::add_default_value:iter:{}: doesn't have default missing vals", + arg.name + ); + // do nothing + } + if !arg.default_vals_ifs.is_empty() { debug!("Parser::add_default_value: has conditional defaults"); - if matcher.get(&arg.id).is_none() { + if !matcher.contains(&arg.id) { for (id, val, default) in arg.default_vals_ifs.iter() { let add = if let Some(a) = matcher.get(id) { match val { @@ -1285,13 +1244,15 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { if add { if let Some(default) = default { self.start_custom_arg(matcher, arg, ValueSource::DefaultValue); - self.add_val_to_arg( + let _ignored_result = self.add_val_to_arg( arg, &RawOsStr::new(default), matcher, - false, trailing_values, )?; + if _ignored_result != ParseResult::ValuesDone { + debug!("Parser::add_default_value: Ignoring state {:?}; defaults are outside of the parse loop", _ignored_result); + } } return Ok(()); } @@ -1301,83 +1262,29 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { debug!("Parser::add_default_value: doesn't have conditional defaults"); } - fn process_default_vals(arg: &Arg<'_>, default_vals: &[&OsStr]) -> Vec { - if let Some(delim) = arg.val_delim { - let mut vals = vec![]; - for val in default_vals { - let val = RawOsStr::new(val); - for val in val.split(delim) { - vals.push(val.to_os_str().into_owned()); - } - } - vals - } else { - default_vals.iter().map(OsString::from).collect() - } - } - if !arg.default_vals.is_empty() { debug!( "Parser::add_default_value:iter:{}: has default vals", arg.name ); - if matcher.get(&arg.id).is_some() { + if matcher.contains(&arg.id) { debug!("Parser::add_default_value:iter:{}: was used", arg.name); // do nothing } else { debug!("Parser::add_default_value:iter:{}: wasn't used", arg.name); self.start_custom_arg(matcher, arg, ValueSource::DefaultValue); - self.add_multiple_vals_to_arg( - arg, - process_default_vals(arg, &arg.default_vals).into_iter(), - matcher, - false, - )?; - } - } else { - debug!( - "Parser::add_default_value:iter:{}: doesn't have default vals", - arg.name - ); - - // do nothing - } - - if !arg.default_missing_vals.is_empty() { - debug!( - "Parser::add_default_value:iter:{}: has default missing vals", - arg.name - ); - match matcher.get(&arg.id) { - Some(ma) if ma.all_val_groups_empty() => { - debug!( - "Parser::add_default_value:iter:{}: has no user defined vals", - arg.name - ); - self.start_custom_arg(matcher, arg, ValueSource::DefaultValue); - self.add_multiple_vals_to_arg( - arg, - process_default_vals(arg, &arg.default_missing_vals).into_iter(), - matcher, - false, - )?; - } - None => { - debug!("Parser::add_default_value:iter:{}: wasn't used", arg.name); - // do nothing - } - _ => { - debug!( - "Parser::add_default_value:iter:{}: has user defined vals", - arg.name - ); - // do nothing + for v in arg.default_vals.iter() { + let _ignored_result = + self.add_val_to_arg(arg, &RawOsStr::new(v), matcher, trailing_values)?; + if _ignored_result != ParseResult::ValuesDone { + debug!("Parser::add_default_value: Ignoring state {:?}; defaults are outside of the parse loop", _ignored_result); + } } } } else { debug!( - "Parser::add_default_value:iter:{}: doesn't have default missing vals", + "Parser::add_default_value:iter:{}: doesn't have default vals", arg.name ); @@ -1493,6 +1400,7 @@ pub(crate) enum ParseState { /// Recoverable Parsing results. #[derive(Debug, PartialEq, Clone)] +#[must_use] enum ParseResult { FlagSubCommand(String), Opt(Id), @@ -1518,8 +1426,10 @@ enum ParseResult { }, /// No argument found e.g. parser is given `-` when parsing a flag. NoArg, - /// This is a Help flag. - HelpFlag, - /// This is a version flag. - VersionFlag, +} + +#[derive(Copy, Clone, Debug, PartialEq, Eq)] +enum Identifier<'f> { + Short(char), + Long(&'f str), } diff --git a/tests/builder/default_missing_vals.rs b/tests/builder/default_missing_vals.rs index 0700df6712f..bacbe1ff8e6 100644 --- a/tests/builder/default_missing_vals.rs +++ b/tests/builder/default_missing_vals.rs @@ -1,4 +1,4 @@ -use clap::{arg, Arg, ArgMatches, Command}; +use clap::{arg, Arg, Command}; #[test] fn opt_missing() { @@ -20,6 +20,10 @@ fn opt_missing() { "auto" ); assert_eq!(m.occurrences_of("color"), 0); + assert_eq!( + m.value_source("color").unwrap(), + clap::ValueSource::DefaultValue + ); } #[test] @@ -42,6 +46,10 @@ fn opt_present_with_missing_value() { "always" ); assert_eq!(m.occurrences_of("color"), 1); + assert_eq!( + m.value_source("color").unwrap(), + clap::ValueSource::CommandLine + ); } #[test] @@ -64,6 +72,10 @@ fn opt_present_with_value() { "never" ); assert_eq!(m.occurrences_of("color"), 1); + assert_eq!( + m.value_source("color").unwrap(), + clap::ValueSource::CommandLine + ); } #[test] @@ -85,6 +97,10 @@ fn opt_present_with_empty_value() { "" ); assert_eq!(m.occurrences_of("color"), 1); + assert_eq!( + m.value_source("color").unwrap(), + clap::ValueSource::CommandLine + ); } //## `default_value`/`default_missing_value` non-interaction checks @@ -134,44 +150,96 @@ fn default_missing_value_flag_value() { Arg::new("flag") .long("flag") .takes_value(true) + .default_value("false") .default_missing_value("true"), ); - fn flag_value(m: ArgMatches) -> bool { - match m.get_one::("flag").map(|v| v.as_str()) { - None => false, - Some(x) => x.parse().expect("non boolean value"), - } - } + let m = cmd.clone().try_get_matches_from(&["test"]).unwrap(); + assert!(m.is_present("flag")); + assert_eq!( + m.get_one::("flag").map(|v| v.as_str()), + Some("false") + ); + assert_eq!(m.occurrences_of("flag"), 0); + assert_eq!( + m.value_source("flag").unwrap(), + clap::ValueSource::DefaultValue + ); + let m = cmd + .clone() + .try_get_matches_from(&["test", "--flag"]) + .unwrap(); + assert!(m.is_present("flag")); assert_eq!( - flag_value(cmd.clone().try_get_matches_from(&["test"]).unwrap()), - false + m.get_one::("flag").map(|v| v.as_str()), + Some("true") ); + assert_eq!(m.occurrences_of("flag"), 1); assert_eq!( - flag_value( - cmd.clone() - .try_get_matches_from(&["test", "--flag"]) - .unwrap() - ), - true + m.value_source("flag").unwrap(), + clap::ValueSource::CommandLine ); + + let m = cmd + .clone() + .try_get_matches_from(&["test", "--flag=true"]) + .unwrap(); + assert!(m.is_present("flag")); + assert_eq!( + m.get_one::("flag").map(|v| v.as_str()), + Some("true") + ); + assert_eq!(m.occurrences_of("flag"), 1); assert_eq!( - flag_value( - cmd.clone() - .try_get_matches_from(&["test", "--flag=true"]) - .unwrap() - ), - true + m.value_source("flag").unwrap(), + clap::ValueSource::CommandLine ); + + let m = cmd.try_get_matches_from(&["test", "--flag=false"]).unwrap(); + assert!(m.is_present("flag")); + assert_eq!( + m.get_one::("flag").map(|v| v.as_str()), + Some("false") + ); + assert_eq!(m.occurrences_of("flag"), 1); + assert_eq!( + m.value_source("flag").unwrap(), + clap::ValueSource::CommandLine + ); +} + +#[test] +fn delimited_missing_value() { + let cmd = Command::new("test").arg( + Arg::new("flag") + .long("flag") + .default_value("one,two") + .default_missing_value("three,four") + .min_values(0) + .value_delimiter(',') + .require_equals(true), + ); + + let m = cmd.clone().try_get_matches_from(["test"]).unwrap(); + assert_eq!( + m.get_many::("flag") + .unwrap() + .map(|s| s.as_str()) + .collect::>(), + vec!["one", "two"] + ); + assert_eq!(m.occurrences_of("flag"), 0); + + let m = cmd.try_get_matches_from(["test", "--flag"]).unwrap(); assert_eq!( - flag_value( - cmd.clone() - .try_get_matches_from(&["test", "--flag=false"]) - .unwrap() - ), - false + m.get_many::("flag") + .unwrap() + .map(|s| s.as_str()) + .collect::>(), + vec!["three", "four"] ); + assert_eq!(m.occurrences_of("flag"), 1); } #[cfg(debug_assertions)] diff --git a/tests/builder/default_vals.rs b/tests/builder/default_vals.rs index 678484737d0..501ffcd386a 100644 --- a/tests/builder/default_vals.rs +++ b/tests/builder/default_vals.rs @@ -831,3 +831,49 @@ fn missing_with_value_delimiter() { ["value1", "value2", "value3", "value4", "value5"] ); } + +#[test] +fn default_independent_of_trailing() { + let cmd = Command::new("test") + .dont_delimit_trailing_values(true) + .arg(Arg::new("pos").required(true)) + .arg( + Arg::new("flag") + .long("flag") + .default_value("one,two") + .value_delimiter(','), + ); + + // Baseline behavior + let m = cmd + .clone() + .try_get_matches_from(vec!["program", "here"]) + .unwrap(); + assert_eq!( + m.get_one::("pos").map(|v| v.as_str()).unwrap(), + "here" + ); + assert_eq!( + m.get_many::("flag") + .unwrap() + .map(|v| v.as_str()) + .collect::>(), + ["one", "two"] + ); + + // Trailing-values behavior should match the baseline + let m = cmd + .try_get_matches_from(vec!["program", "--", "here"]) + .unwrap(); + assert_eq!( + m.get_one::("pos").map(|v| v.as_str()).unwrap(), + "here" + ); + assert_eq!( + m.get_many::("flag") + .unwrap() + .map(|v| v.as_str()) + .collect::>(), + ["one", "two"] + ); +} diff --git a/tests/builder/multiple_values.rs b/tests/builder/multiple_values.rs index 2029a4df356..c775e359dd6 100644 --- a/tests/builder/multiple_values.rs +++ b/tests/builder/multiple_values.rs @@ -385,7 +385,7 @@ fn positional() { let m = m.unwrap(); assert!(m.is_present("pos")); - assert_eq!(m.occurrences_of("pos"), 3); + assert_eq!(m.occurrences_of("pos"), 1); assert_eq!( m.get_many::("pos") .unwrap() @@ -409,7 +409,7 @@ fn positional_exact_exact() { let m = m.unwrap(); assert!(m.is_present("pos")); - assert_eq!(m.occurrences_of("pos"), 3); + assert_eq!(m.occurrences_of("pos"), 1); assert_eq!( m.get_many::("pos") .unwrap() @@ -457,7 +457,7 @@ fn positional_min_exact() { let m = m.unwrap(); assert!(m.is_present("pos")); - assert_eq!(m.occurrences_of("pos"), 3); + assert_eq!(m.occurrences_of("pos"), 1); assert_eq!( m.get_many::("pos") .unwrap() @@ -487,7 +487,7 @@ fn positional_min_more() { let m = m.unwrap(); assert!(m.is_present("pos")); - assert_eq!(m.occurrences_of("pos"), 4); + assert_eq!(m.occurrences_of("pos"), 1); assert_eq!( m.get_many::("pos") .unwrap() @@ -507,7 +507,7 @@ fn positional_max_exact() { let m = m.unwrap(); assert!(m.is_present("pos")); - assert_eq!(m.occurrences_of("pos"), 3); + assert_eq!(m.occurrences_of("pos"), 1); assert_eq!( m.get_many::("pos") .unwrap() @@ -527,7 +527,7 @@ fn positional_max_less() { let m = m.unwrap(); assert!(m.is_present("pos")); - assert_eq!(m.occurrences_of("pos"), 2); + assert_eq!(m.occurrences_of("pos"), 1); assert_eq!( m.get_many::("pos") .unwrap() @@ -1139,7 +1139,7 @@ fn low_index_positional() { let m = m.unwrap(); assert!(m.is_present("files")); - assert_eq!(m.occurrences_of("files"), 3); + assert_eq!(m.occurrences_of("files"), 1); assert!(m.is_present("target")); assert_eq!(m.occurrences_of("target"), 1); assert_eq!( @@ -1176,7 +1176,7 @@ fn low_index_positional_in_subcmd() { let sm = m.subcommand_matches("test").unwrap(); assert!(sm.is_present("files")); - assert_eq!(sm.occurrences_of("files"), 3); + assert_eq!(sm.occurrences_of("files"), 1); assert!(sm.is_present("target")); assert_eq!(sm.occurrences_of("target"), 1); assert_eq!( @@ -1212,7 +1212,7 @@ fn low_index_positional_with_option() { let m = m.unwrap(); assert!(m.is_present("files")); - assert_eq!(m.occurrences_of("files"), 3); + assert_eq!(m.occurrences_of("files"), 1); assert!(m.is_present("target")); assert_eq!(m.occurrences_of("target"), 1); assert_eq!( @@ -1250,7 +1250,7 @@ fn low_index_positional_with_flag() { let m = m.unwrap(); assert!(m.is_present("files")); - assert_eq!(m.occurrences_of("files"), 3); + assert_eq!(m.occurrences_of("files"), 1); assert!(m.is_present("target")); assert_eq!(m.occurrences_of("target"), 1); assert_eq!( @@ -1498,6 +1498,7 @@ fn values_per_occurrence_named() { .collect::>(), ["val1", "val2"] ); + assert_eq!(m.occurrences_of("pos"), 1); let m = a.try_get_matches_from_mut(vec![ "myprog", "--pos", "val1", "val2", "--pos", "val3", "val4", @@ -1513,6 +1514,7 @@ fn values_per_occurrence_named() { .collect::>(), ["val1", "val2", "val3", "val4"] ); + assert_eq!(m.occurrences_of("pos"), 2); } #[test] @@ -1535,6 +1537,7 @@ fn values_per_occurrence_positional() { .collect::>(), ["val1", "val2"] ); + assert_eq!(m.occurrences_of("pos"), 1); let m = a.try_get_matches_from_mut(vec!["myprog", "val1", "val2", "val3", "val4"]); let m = match m { @@ -1548,4 +1551,36 @@ fn values_per_occurrence_positional() { .collect::>(), ["val1", "val2", "val3", "val4"] ); + //assert_eq!(m.occurrences_of("pos"), 2); // Fails, we don't recognize this as two occurrences +} + +// Theoretically we could support this but we aren't tracking occurrence boundaries for positionals +#[test] +#[should_panic = "When using a positional argument with .multiple_values(true) that is *not the last* positional argument, the last positional argument (i.e. the one with the highest index) *must* have .required(true) or .last(true) set."] +fn positional_parser_advances() { + let m = Command::new("multiple_values") + .arg(Arg::new("pos1").number_of_values(2)) + .arg(Arg::new("pos2").number_of_values(2)) + .try_get_matches_from(vec!["myprog", "val1", "val2", "val3", "val4"]); + + assert!(m.is_ok(), "{}", m.unwrap_err()); + let m = m.unwrap(); + + assert_eq!(m.occurrences_of("pos1"), 1); + assert_eq!( + m.get_many::("pos1") + .unwrap() + .map(|v| v.as_str()) + .collect::>(), + ["val1", "val2"] + ); + + assert_eq!(m.occurrences_of("pos2"), 1); + assert_eq!( + m.get_many::("pos2") + .unwrap() + .map(|v| v.as_str()) + .collect::>(), + ["val3", "val4"] + ); }