From f2a219e77d6769dd9089307f2df31d70bbc5060d Mon Sep 17 00:00:00 2001 From: Ed Page Date: Tue, 31 May 2022 12:19:00 -0500 Subject: [PATCH] refactor(parser)!: Switch flag values to Actions This changes how occurrences and values are grouped for multiple values. Today, it appears as a bug. If we move forward with #3772, then this can make sense. --- src/parser/arg_matcher.rs | 49 ++++++++++++++++++++------------- src/parser/parser.rs | 35 ++++++++++++----------- tests/builder/grouped_values.rs | 4 +-- 3 files changed, 51 insertions(+), 37 deletions(-) diff --git a/src/parser/arg_matcher.rs b/src/parser/arg_matcher.rs index 2e818ad4152..eab4999189f 100644 --- a/src/parser/arg_matcher.rs +++ b/src/parser/arg_matcher.rs @@ -210,26 +210,35 @@ impl ArgMatcher { } 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(); - if let Some(num) = o.num_vals { - debug!("ArgMatcher::needs_more_vals: num_vals...{}", num); - return if o.is_multiple_occurrences_set() { - (current_num % num) != 0 - } else { - num != current_num - }; - } else if let Some(num) = o.max_vals { - debug!("ArgMatcher::needs_more_vals: max_vals...{}", num); - return current_num < num; - } else if o.min_vals.is_some() { - debug!("ArgMatcher::needs_more_vals: min_vals...true"); - return true; + let num_resolved = self.get(&o.id).map(|ma| ma.num_vals()).unwrap_or(0); + let num_pending = self + .pending + .as_ref() + .and_then(|p| (p.id == o.id).then(|| p.raw_vals.len())) + .unwrap_or(0); + let current_num = num_resolved + num_pending; + debug!( + "ArgMatcher::needs_more_vals: o={}, resolved={}, pending={}", + o.name, num_resolved, num_pending + ); + if current_num == 0 { + true + } else if let Some(num) = o.num_vals { + debug!("ArgMatcher::needs_more_vals: num_vals...{}", num); + if o.is_multiple_occurrences_set() { + (current_num % num) != 0 + } else { + num != current_num } - return o.is_multiple_values_set(); + } else if let Some(num) = o.max_vals { + debug!("ArgMatcher::needs_more_vals: max_vals...{}", num); + current_num < num + } else if o.min_vals.is_some() { + debug!("ArgMatcher::needs_more_vals: min_vals...true"); + true + } else { + o.is_multiple_values_set() } - true } pub(crate) fn pending_arg_id(&self) -> Option<&Id> { @@ -247,7 +256,9 @@ impl ArgMatcher { raw_vals: Default::default(), }); debug_assert_eq!(pending.id, *id, "{}", INTERNAL_ERROR_MSG); - debug_assert_eq!(pending.ident, ident, "{}", INTERNAL_ERROR_MSG); + if ident.is_some() { + debug_assert_eq!(pending.ident, ident, "{}", INTERNAL_ERROR_MSG); + } &mut pending.raw_vals } diff --git a/src/parser/parser.rs b/src/parser/parser.rs index 09174843e29..cc6d0e4085f 100644 --- a/src/parser/parser.rs +++ b/src/parser/parser.rs @@ -345,15 +345,14 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { // Assume this is a value of a previous arg. // get the option so we can check the settings - let mut arg_values = Vec::new(); + let arg_values = matcher.pending_values_mut(&id, None); let arg = &self.cmd[id]; let parse_result = self.push_arg_values( arg, arg_os.to_value_os(), trailing_values, - &mut arg_values, + arg_values, ); - self.store_arg_values(arg, arg_values, matcher)?; let parse_result = parse_result.unwrap_or_else(|| { if matcher.needs_more_vals(arg) { ParseResult::Opt(arg.id.clone()) @@ -720,10 +719,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { return Ok(ParseResult::MaybeHyphenValue); } - // Update the current index - self.cur_idx.set(self.cur_idx.get() + 1); - debug!("Parser::parse_long_arg: cur_idx:={}", self.cur_idx.get()); - debug!("Parser::parse_long_arg: Does it contain '='..."); let long_arg = match long_arg { Ok(long_arg) => long_arg, @@ -870,14 +865,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { }; debug!("Parser::parse_short_arg:iter:{}", c); - // update each index because `-abcd` is four indices to clap - self.cur_idx.set(self.cur_idx.get() + 1); - debug!( - "Parser::parse_short_arg:iter:{}: cur_idx:={}", - c, - self.cur_idx.get() - ); - // Check for matching short options, and return the name if there is no trailing // concatenated value: -oval // Option: -o @@ -925,6 +912,11 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { return if let Some(sc_name) = self.cmd.find_short_subcmd(c) { debug!("Parser::parse_short_arg:iter:{}: subcommand={}", c, sc_name); + // Make sure indices get updated before reading `self.cur_idx` + self.resolve_pending(matcher)?; + self.cur_idx.set(self.cur_idx.get() + 1); + debug!("Parser::parse_short_arg: cur_idx:={}", self.cur_idx.get()); + let name = sc_name.to_string(); // Get the index of the previously saved flag subcommand in the group of flags (if exists). // If it is a new flag subcommand, then the formentioned index should be the current one @@ -1028,7 +1020,8 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { Ok(parse_result) } else { debug!("Parser::parse_opt_value: More arg vals required..."); - self.start_occurrence_of_arg(matcher, arg); + self.resolve_pending(matcher)?; + matcher.pending_values_mut(&arg.id, Some(ident)); Ok(ParseResult::Opt(arg.id.clone())) } } @@ -1147,6 +1140,11 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { match arg.get_action() { Action::StoreValue => { if source == ValueSource::CommandLine { + if matches!(ident, Some(Identifier::Short) | Some(Identifier::Long)) { + // Record flag's index + self.cur_idx.set(self.cur_idx.get() + 1); + debug!("Parser::react: cur_idx:={}", self.cur_idx.get()); + } self.start_occurrence_of_arg(matcher, arg); } else { self.start_custom_arg(matcher, arg, source); @@ -1162,6 +1160,11 @@ impl<'help, 'cmd> Parser<'help, 'cmd> { Action::Flag => { debug_assert_eq!(raw_vals, Vec::::new()); if source == ValueSource::CommandLine { + if matches!(ident, Some(Identifier::Short) | Some(Identifier::Long)) { + // Record flag's index + self.cur_idx.set(self.cur_idx.get() + 1); + debug!("Parser::react: cur_idx:={}", self.cur_idx.get()); + } self.start_occurrence_of_arg(matcher, arg); } else { self.start_custom_arg(matcher, arg, source); diff --git a/tests/builder/grouped_values.rs b/tests/builder/grouped_values.rs index 1a2c617053e..fbb61d04960 100644 --- a/tests/builder/grouped_values.rs +++ b/tests/builder/grouped_values.rs @@ -190,8 +190,8 @@ fn grouped_interleaved_positional_values() { .try_get_matches_from(["foo", "1", "2", "-f", "a", "3", "-f", "b", "4"]) .unwrap(); let pos: Vec<_> = m.grouped_values_of("pos").unwrap().collect(); - assert_eq!(pos, vec![vec!["1", "2", "3", "4"]]); - assert_eq!(m.occurrences_of("pos"), 1); + assert_eq!(pos, vec![vec!["1", "2"], vec!["3"], vec!["4"]]); + assert_eq!(m.occurrences_of("pos"), 3); let flag: Vec<_> = m.grouped_values_of("flag").unwrap().collect(); assert_eq!(flag, vec![vec!["a"], vec!["b"]]); assert_eq!(m.occurrences_of("flag"), 2);