Skip to content

Commit

Permalink
Merge pull request #3960 from epage/pos
Browse files Browse the repository at this point in the history 
fix(parser): low index multiples work with flags
  • Loading branch information
@epage
epage authored Jul 21, 2022
2 parents 5e02445 + 48f087d commit f48e517
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 71 deletions.
142 changes: 71 additions & 71 deletions src/parser/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,77 +98,6 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
arg_os.to_value_os().as_raw_bytes()
);

// Correct pos_counter.
pos_counter = {
let is_second_to_last = pos_counter + 1 == positional_count;

// The last positional argument, or second to last positional
// argument may be set to .multiple_values(true) or `.multiple_occurrences(true)`
let low_index_mults = is_second_to_last
&& self
.cmd
.get_positionals()
.any(|a| a.is_multiple() && (positional_count != a.index.unwrap_or(0)))
&& self
.cmd
.get_positionals()
.last()
.map_or(false, |p_name| !p_name.is_last_set());

let missing_pos = self.cmd.is_allow_missing_positional_set()
&& is_second_to_last
&& !trailing_values;

debug!(
"Parser::get_matches_with: Positional counter...{}",
pos_counter
);
debug!(
"Parser::get_matches_with: Low index multiples...{:?}",
low_index_mults
);

if low_index_mults || missing_pos {
let skip_current = if let Some(n) = raw_args.peek(&args_cursor) {
if let Some(arg) = self
.cmd
.get_positionals()
.find(|a| a.index == Some(pos_counter))
{
// If next value looks like a new_arg or it's a
// subcommand, skip positional argument under current
// pos_counter(which means current value cannot be a
// positional argument with a value next to it), assume
// current value matches the next arg.
self.is_new_arg(&n, arg)
|| self
.possible_subcommand(n.to_value(), valid_arg_found)
.is_some()
} else {
true
}
} else {
true
};

if skip_current {
debug!("Parser::get_matches_with: Bumping the positional counter...");
pos_counter + 1
} else {
pos_counter
}
} else if trailing_values
&& (self.cmd.is_allow_missing_positional_set() || contains_last)
{
// Came to -- and one positional has .last(true) set, so we go immediately
// to the last (highest index) positional
debug!("Parser::get_matches_with: .last(true) and --, setting last pos");
positional_count
} else {
pos_counter
}
};

// Has the user already passed '--'? Meaning only positional args follow
if !trailing_values {
if self.cmd.is_subcommand_precedence_over_arg_set()
Expand Down Expand Up @@ -376,6 +305,77 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
}
}

// Correct pos_counter.
pos_counter = {
let is_second_to_last = pos_counter + 1 == positional_count;

// The last positional argument, or second to last positional
// argument may be set to .multiple_values(true) or `.multiple_occurrences(true)`
let low_index_mults = is_second_to_last
&& self
.cmd
.get_positionals()
.any(|a| a.is_multiple() && (positional_count != a.index.unwrap_or(0)))
&& self
.cmd
.get_positionals()
.last()
.map_or(false, |p_name| !p_name.is_last_set());

let missing_pos = self.cmd.is_allow_missing_positional_set()
&& is_second_to_last
&& !trailing_values;

debug!(
"Parser::get_matches_with: Positional counter...{}",
pos_counter
);
debug!(
"Parser::get_matches_with: Low index multiples...{:?}",
low_index_mults
);

if low_index_mults || missing_pos {
let skip_current = if let Some(n) = raw_args.peek(&args_cursor) {
if let Some(arg) = self
.cmd
.get_positionals()
.find(|a| a.index == Some(pos_counter))
{
// If next value looks like a new_arg or it's a
// subcommand, skip positional argument under current
// pos_counter(which means current value cannot be a
// positional argument with a value next to it), assume
// current value matches the next arg.
self.is_new_arg(&n, arg)
|| self
.possible_subcommand(n.to_value(), valid_arg_found)
.is_some()
} else {
true
}
} else {
true
};

if skip_current {
debug!("Parser::get_matches_with: Bumping the positional counter...");
pos_counter + 1
} else {
pos_counter
}
} else if trailing_values
&& (self.cmd.is_allow_missing_positional_set() || contains_last)
{
// Came to -- and one positional has .last(true) set, so we go immediately
// to the last (highest index) positional
debug!("Parser::get_matches_with: .last(true) and --, setting last pos");
positional_count
} else {
pos_counter
}
};

if let Some(arg) = self.cmd.get_keymap().get(&pos_counter) {
if arg.is_last_set() && !trailing_values {
let _ = self.resolve_pending(matcher);
Expand Down
35 changes: 35 additions & 0 deletions tests/builder/multiple_values.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1226,6 +1226,41 @@ fn low_index_positional_with_flag() {
assert!(*m.get_one::<bool>("flg").expect("defaulted by clap"));
}

#[test]
fn low_index_positional_with_extra_flags() {
let cmd = Command::new("test")
.arg(Arg::new("yes").long("yes").action(ArgAction::SetTrue))
.arg(Arg::new("one").long("one").action(ArgAction::Set))
.arg(Arg::new("two").long("two").action(ArgAction::Set))
.arg(Arg::new("input").multiple_values(true).required(true))
.arg(Arg::new("output").required(true));
let m = cmd.try_get_matches_from([
"test", "--one", "1", "--two", "2", "3", "4", "5", "6", "7", "8",
]);

assert!(m.is_ok(), "{:?}", m.unwrap_err().kind());
let m = m.unwrap();

assert_eq!(
m.get_many::<String>("input")
.unwrap()
.into_iter()
.map(String::from)
.collect::<Vec<_>>(),
vec![
"3".to_owned(),
"4".to_owned(),
"5".to_owned(),
"6".to_owned(),
"7".to_owned()
],
);
assert_eq!(m.get_one::<String>("output").unwrap(), "8");
assert_eq!(m.get_one::<String>("one").unwrap(), "1");
assert_eq!(m.get_one::<String>("two").unwrap(), "2");
assert!(!*m.get_one::<bool>("yes").unwrap());
}

#[test]
fn multiple_value_terminator_option() {
let m = Command::new("lip")
Expand Down

0 comments on commit f48e517

Please sign in to comment.