Skip to content

Commit

Permalink
refactor(parser)!: Switch flag values to Actions
Browse files Browse the repository at this point in the history 
This changes how occurrences and values are grouped for multiple values.
Today, it appears as a bug.  If we move forward with clap-rs#3772, then this
can make sense.
  • Loading branch information
@epage
epage committed May 31, 2022
1 parent bba83cb commit f2a219e
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 37 deletions.
49 changes: 30 additions & 19 deletions src/parser/arg_matcher.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand All @@ -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
}

Expand Down
35 changes: 19 additions & 16 deletions src/parser/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()))
}
}
Expand Down Expand Up @@ -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);
Expand All @@ -1162,6 +1160,11 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
Action::Flag => {
debug_assert_eq!(raw_vals, Vec::<OsString>::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);
Expand Down
4 changes: 2 additions & 2 deletions tests/builder/grouped_values.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit f2a219e

Please sign in to comment.