diff --git a/src/parse/arg_matcher.rs b/src/parse/arg_matcher.rs index fd43aefb..89d504ed 100644 --- a/src/parse/arg_matcher.rs +++ b/src/parse/arg_matcher.rs @@ -8,6 +8,9 @@ use crate::{ util::Id, }; +// Third party +use indexmap::map::Entry; + #[derive(Debug)] pub(crate) struct ArgMatcher(pub(crate) ArgMatches); @@ -121,7 +124,15 @@ impl ArgMatcher { ma.occurs += 1; } - pub(crate) fn add_val_to(&mut self, arg: &Id, val: OsString, ty: ValueType) { + pub(crate) fn add_val_to(&mut self, arg: &Id, val: OsString, ty: ValueType, append: bool) { + if append { + self.append_val_to(arg, val, ty); + } else { + self.push_val_to(arg, val, ty); + } + } + + pub(crate) fn push_val_to(&mut self, arg: &Id, val: OsString, ty: ValueType) { // We will manually inc occurrences later(for flexibility under // specific circumstances, like only add one occurrence for flag // when we met: `--flag=one,two`). @@ -130,15 +141,8 @@ impl ArgMatcher { ma.push_val(val); } - pub(crate) fn add_vals_to(&mut self, arg: &Id, vals: Vec, ty: ValueType) { + pub(crate) fn new_val_group(&mut self, arg: &Id) { let ma = self.entry(arg).or_default(); - ma.set_ty(ty); - ma.push_vals(vals); - } - - pub(crate) fn add_val_group_to(&mut self, arg: &Id, ty: ValueType) { - let ma = self.entry(arg).or_default(); - ma.set_ty(ty); ma.new_val_group(); } @@ -154,19 +158,24 @@ impl ArgMatcher { ma.push_index(idx); } + pub(crate) fn arg_have_val(&mut self, arg: &Id) -> bool { + matches!(self.entry(arg), Entry::Occupied(_)) + } + 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) { + let current_num = ma.num_vals_current_group() as u64; if let Some(num) = o.num_vals { debug!("ArgMatcher::needs_more_vals: num_vals...{}", num); return if o.is_set(ArgSettings::MultipleValues) { - ((ma.num_vals() as u64) % num) != 0 + (current_num % num) != 0 } else { - num != (ma.num_vals() as u64) + num != current_num }; } else if let Some(num) = o.max_vals { debug!("ArgMatcher::needs_more_vals: max_vals...{}", num); - return (ma.num_vals() as u64) < num; + return current_num < num; } else if o.min_vals.is_some() { debug!("ArgMatcher::needs_more_vals: min_vals...true"); return true; diff --git a/src/parse/matches/matched_arg.rs b/src/parse/matches/matched_arg.rs index ec367b2d..49b7cc4b 100644 --- a/src/parse/matches/matched_arg.rs +++ b/src/parse/matches/matched_arg.rs @@ -77,20 +77,37 @@ impl MatchedArg { self.vals.last_mut().expect(INTERNAL_ERROR_MSG).push(val) } - pub(crate) fn push_vals(&mut self, vals: Vec) { - self.vals.push(vals) - } - pub(crate) fn num_vals(&self) -> usize { self.vals.iter().flatten().count() } + pub(crate) fn num_vals_current_group(&self) -> usize { + self.vals.last().map(|x| x.len()).unwrap_or(0) + } + pub(crate) fn no_val(&self) -> bool { self.vals.iter().flatten().count() == 0 } pub(crate) fn remove_vals(&mut self, len: usize) { - self.vals.drain(0..len); + let mut want_remove = len; + let mut remove_group = None; + let mut remove_val = None; + for (i, g) in self.vals().enumerate() { + if g.len() <= want_remove { + want_remove -= g.len(); + remove_group = Some(i); + } else { + remove_val = Some(want_remove); + break; + } + } + if let Some(remove_group) = remove_group { + self.vals.drain(0..=remove_group); + } + if let Some(remove_val) = remove_val { + self.vals[0].drain(0..remove_val); + } } pub(crate) fn contains_val(&self, val: &str) -> bool { @@ -102,3 +119,191 @@ impl MatchedArg { self.ty = ty; } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_grouped_vals_push_and_append() { + let mut m = MatchedArg::new(); + m.push_val("aaa".into()); + m.new_val_group(); + m.append_val("bbb".into()); + m.append_val("ccc".into()); + m.new_val_group(); + m.append_val("ddd".into()); + m.push_val("eee".into()); + m.new_val_group(); + m.append_val("fff".into()); + m.append_val("ggg".into()); + m.append_val("hhh".into()); + m.append_val("iii".into()); + + let vals: Vec<&Vec> = m.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"), + ] + ] + ) + } + + #[test] + fn test_grouped_vals_removal() { + let m = { + let mut m = MatchedArg::new(); + m.push_val("aaa".into()); + m.new_val_group(); + m.append_val("bbb".into()); + m.append_val("ccc".into()); + m.new_val_group(); + m.append_val("ddd".into()); + m.push_val("eee".into()); + m.new_val_group(); + m.append_val("fff".into()); + m.append_val("ggg".into()); + m.append_val("hhh".into()); + m.append_val("iii".into()); + m + }; + { + let mut m = m.clone(); + m.remove_vals(0); + let vals1 = m.vals().collect::>(); + assert_eq!( + vals1, + 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"), + ] + ] + ); + } + + { + let mut m = m.clone(); + m.remove_vals(1); + let vals0 = m.vals().collect::>(); + assert_eq!( + vals0, + vec![ + &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"), + ] + ] + ); + } + + { + let mut m = m.clone(); + m.remove_vals(2); + let vals1 = m.vals().collect::>(); + assert_eq!( + vals1, + vec![ + &vec![OsString::from("ccc"),], + &vec![OsString::from("ddd")], + &vec![OsString::from("eee")], + &vec![ + OsString::from("fff"), + OsString::from("ggg"), + OsString::from("hhh"), + OsString::from("iii"), + ] + ] + ); + } + + { + let mut m = m.clone(); + m.remove_vals(3); + let vals1 = m.vals().collect::>(); + assert_eq!( + vals1, + vec![ + &vec![OsString::from("ddd")], + &vec![OsString::from("eee")], + &vec![ + OsString::from("fff"), + OsString::from("ggg"), + OsString::from("hhh"), + OsString::from("iii"), + ] + ] + ); + } + + { + let mut m = m.clone(); + m.remove_vals(4); + let vals1 = m.vals().collect::>(); + assert_eq!( + vals1, + vec![ + &vec![OsString::from("eee")], + &vec![ + OsString::from("fff"), + OsString::from("ggg"), + OsString::from("hhh"), + OsString::from("iii"), + ] + ] + ); + } + + { + let mut m = m.clone(); + m.remove_vals(5); + let vals1 = m.vals().collect::>(); + assert_eq!( + vals1, + vec![&vec![ + OsString::from("fff"), + OsString::from("ggg"), + OsString::from("hhh"), + OsString::from("iii"), + ]] + ); + } + + { + let mut m = m.clone(); + m.remove_vals(7); + let vals1 = m.vals().collect::>(); + assert_eq!( + vals1, + vec![&vec![OsString::from("hhh"), OsString::from("iii"),]] + ); + } + + { + let mut m = m.clone(); + m.remove_vals(9); + let vals1: Vec<&Vec> = m.vals().collect(); + assert!(vals1.is_empty()); + } + } +} diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 47c32dc1..0562e41b 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -435,6 +435,7 @@ impl<'help, 'app> Parser<'help, 'app> { &arg_os, matcher, ValueType::CommandLine, + true, ); // get the next value from the iterator continue; @@ -521,7 +522,8 @@ impl<'help, 'app> Parser<'help, 'app> { } self.seen.push(p.id.clone()); - self.add_val_to_arg(p, &arg_os, matcher, ValueType::CommandLine); + let append = self.arg_have_val(matcher, p); + self.add_val_to_arg(p, &arg_os, matcher, ValueType::CommandLine, append); matcher.inc_occurrence_of(&p.id); for grp in self.app.groups_for_arg(&p.id) { @@ -558,7 +560,12 @@ impl<'help, 'app> Parser<'help, 'app> { self.app.color(), )); } - sc_m.add_val_to(&Id::empty_hash(), v.to_os_string(), ValueType::CommandLine); + sc_m.add_val_to( + &Id::empty_hash(), + v.to_os_string(), + ValueType::CommandLine, + false, + ); } matcher.subcommand(SubCommand { @@ -1167,7 +1174,7 @@ impl<'help, 'app> Parser<'help, 'app> { fv, fv.starts_with("=") ); - self.add_val_to_arg(opt, &v, matcher, ValueType::CommandLine); + self.add_val_to_arg(opt, &v, matcher, ValueType::CommandLine, false); } else if require_equals && !empty_vals && !min_vals_zero { debug!("None, but requires equals...Error"); return Err(ClapError::empty_value( @@ -1179,17 +1186,12 @@ impl<'help, 'app> Parser<'help, 'app> { debug!("None and requires equals, but min_vals == 0"); if !opt.default_missing_vals.is_empty() { debug!("Parser::parse_opt: has default_missing_vals"); - let default_missing_vals = opt + let vals = opt .default_missing_vals .iter() - .map(|x| ArgStr::new(x)) + .map(OsString::from) .collect(); - self.add_multiple_vals_to_arg( - opt, - default_missing_vals, - matcher, - ValueType::CommandLine, - ); + self.add_multiple_vals_to_arg(opt, vals, matcher, ValueType::CommandLine, false); }; } else { debug!("None"); @@ -1211,6 +1213,10 @@ impl<'help, 'app> Parser<'help, 'app> { || (multiple && !needs_delimiter) && !has_eq && matcher.needs_more_vals(opt) { debug!("Parser::parse_opt: More arg vals required..."); + matcher.new_val_group(&opt.id); + for group in self.app.groups_for_arg(&opt.id) { + matcher.new_val_group(&group); + } Ok(ParseResult::Opt(opt.id.clone())) } else { debug!("Parser::parse_opt: More arg vals not required..."); @@ -1224,6 +1230,7 @@ impl<'help, 'app> Parser<'help, 'app> { val: &ArgStr, matcher: &mut ArgMatcher, ty: ValueType, + append: bool, ) -> ParseResult { debug!("Parser::add_val_to_arg; arg={}, val={:?}", arg.name, val); debug!( @@ -1246,16 +1253,16 @@ impl<'help, 'app> Parser<'help, 'app> { } else { arg_split.into_iter().collect() }; - let mut parse_result = if vals.is_empty() { - ParseResult::ValuesDone - } else { - self.add_multiple_vals_to_arg(arg, vals, matcher, ty) - }; + let vals = vals.into_iter().map(|x| x.to_os_string()).collect(); + self.add_multiple_vals_to_arg(arg, vals, matcher, ty, append); // If there was a delimiter used or we must use the delimiter to // separate the values, we're not looking for more values. - if val.contains_char(delim) || arg.is_set(ArgSettings::RequireDelimiter) { - parse_result = ParseResult::ValuesDone; - } + let parse_result = + if val.contains_char(delim) || arg.is_set(ArgSettings::RequireDelimiter) { + ParseResult::ValuesDone + } else { + self.need_more_vals(matcher, arg) + }; return parse_result; } } @@ -1264,59 +1271,50 @@ impl<'help, 'app> Parser<'help, 'app> { return ParseResult::ValuesDone; } } - self.add_single_val_to_arg(arg, val, matcher, ty) + self.add_single_val_to_arg(arg, val.to_os_string(), matcher, ty, append); + self.need_more_vals(matcher, arg) } fn add_multiple_vals_to_arg( &self, arg: &Arg<'help>, - vals: Vec, + vals: Vec, matcher: &mut ArgMatcher, ty: ValueType, - ) -> ParseResult { - debug!("Parser::add_multiple_val_to_arg: adding vals...{:?}", vals); - - let num_vals = vals.len(); - let begin_index = self.cur_idx.get() + 1; - self.cur_idx.set(self.cur_idx.get() + num_vals); - for i in begin_index..begin_index + num_vals { - matcher.add_index_to(&arg.id, i, ty); + append: bool, + ) { + // If not appending, create a new val group and then append vals in. + if !append { + matcher.new_val_group(&arg.id); } - let vals: Vec<_> = vals.into_iter().map(|x| x.to_os_string()).collect(); - // Increment or create the group "args" - for val in vals.iter() { - for group in self.app.groups_for_arg(&arg.id) { - matcher.add_val_to(&group, val.clone(), ty); - } - } - matcher.add_vals_to(&arg.id, vals, ty); - if matcher.needs_more_vals(arg) { - ParseResult::Opt(arg.id.clone()) - } else { - ParseResult::ValuesDone + for val in vals { + self.add_single_val_to_arg(arg, val.clone(), matcher, ty, true); } } fn add_single_val_to_arg( &self, arg: &Arg<'help>, - val: &ArgStr, + val: OsString, matcher: &mut ArgMatcher, ty: ValueType, - ) -> ParseResult { + append: bool, + ) { debug!("Parser::add_single_val_to_arg: adding val...{:?}", val); // update the current index because each value is a distinct index to clap self.cur_idx.set(self.cur_idx.get() + 1); - matcher.add_val_to(&arg.id, val.to_os_string(), ty); - matcher.add_index_to(&arg.id, self.cur_idx.get(), ty); - // Increment or create the group "args" for group in self.app.groups_for_arg(&arg.id) { - matcher.add_val_to(&group, val.to_os_string(), ty); + matcher.add_val_to(&group, val.clone(), ty, append); } + matcher.add_val_to(&arg.id, val, ty, append); + matcher.add_index_to(&arg.id, self.cur_idx.get(), ty); + } + + fn need_more_vals(&self, matcher: &mut ArgMatcher, arg: &Arg<'help>) -> ParseResult { if matcher.needs_more_vals(arg) { ParseResult::Opt(arg.id.clone()) } else { @@ -1324,6 +1322,10 @@ impl<'help, 'app> Parser<'help, 'app> { } } + fn arg_have_val(&self, matcher: &mut ArgMatcher, arg: &Arg<'help>) -> bool { + matcher.arg_have_val(&arg.id) + } + fn parse_flag(&self, flag: &Arg<'help>, matcher: &mut ArgMatcher) -> ParseResult { debug!("Parser::parse_flag"); @@ -1437,7 +1439,7 @@ impl<'help, 'app> Parser<'help, 'app> { }; if add { - self.add_val_to_arg(arg, &ArgStr::new(default), matcher, ty); + self.add_val_to_arg(arg, &ArgStr::new(default), matcher, ty, false); return; } } @@ -1454,9 +1456,8 @@ impl<'help, 'app> Parser<'help, 'app> { } else { debug!("Parser::add_value:iter:{}: wasn't used", arg.name); - for val in &arg.default_vals { - self.add_val_to_arg(arg, &ArgStr::new(val), matcher, ty); - } + let vals = arg.default_vals.iter().map(OsString::from).collect(); + self.add_multiple_vals_to_arg(arg, vals, matcher, ty, false); } } else { debug!( @@ -1478,9 +1479,12 @@ impl<'help, 'app> Parser<'help, 'app> { "Parser::add_value:iter:{}: has no user defined vals", arg.name ); - for val in &arg.default_missing_vals { - self.add_val_to_arg(arg, &ArgStr::new(val), matcher, ty); - } + let vals = arg + .default_missing_vals + .iter() + .map(OsString::from) + .collect(); + self.add_multiple_vals_to_arg(arg, vals, matcher, ty, false); } None => { debug!("Parser::add_value:iter:{}: wasn't used", arg.name); @@ -1508,7 +1512,7 @@ impl<'help, 'app> Parser<'help, 'app> { if let Some((_, Some(ref val))) = a.env { let val = &ArgStr::new(val); if a.is_set(ArgSettings::TakesValue) { - self.add_val_to_arg(a, val, matcher, ValueType::EnvVariable); + self.add_val_to_arg(a, val, matcher, ValueType::EnvVariable, false); } else { self.check_for_help_and_version_str(val)?; matcher.add_index_to(&a.id, self.cur_idx.get(), ValueType::EnvVariable);