Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(parser): Only add ArgGroup to ArgMatches for command-line #4375

Merged
merged 7 commits into from Oct 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 0 additions & 17 deletions src/parser/arg_matcher.rs
Expand Up @@ -152,23 +152,6 @@ impl ArgMatcher {
ma.new_val_group();
}

pub(crate) fn start_occurrence_of_arg(&mut self, arg: &Arg) {
let id = arg.get_id().clone();
debug!("ArgMatcher::start_occurrence_of_arg: id={:?}", id);
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(ValueSource::CommandLine);
ma.new_val_group();
}

pub(crate) fn start_occurrence_of_group(&mut self, id: Id) {
debug!("ArgMatcher::start_occurrence_of_group: id={:?}", id);
let ma = self.entry(id).or_insert(MatchedArg::new_group());
debug_assert_eq!(ma.type_id(), None);
ma.set_source(ValueSource::CommandLine);
ma.new_val_group();
}

pub(crate) fn start_occurrence_of_external(&mut self, cmd: &crate::Command) {
let id = Id::from_static_ref(Id::EXTERNAL);
debug!("ArgMatcher::start_occurrence_of_external: id={:?}", id,);
Expand Down
2 changes: 1 addition & 1 deletion src/parser/matches/matched_arg.rs
Expand Up @@ -130,7 +130,7 @@ impl MatchedArg {
}

pub(crate) fn check_explicit(&self, predicate: &ArgPredicate) -> bool {
if self.source == Some(ValueSource::DefaultValue) {
if self.source.map(|s| !s.is_explicit()).unwrap_or(false) {
return false;
}

Expand Down
6 changes: 6 additions & 0 deletions src/parser/matches/value_source.rs
Expand Up @@ -9,3 +9,9 @@ pub enum ValueSource {
/// Value was passed in on the command-line
CommandLine,
}

impl ValueSource {
pub(crate) fn is_explicit(self) -> bool {
self != Self::DefaultValue
}
}
34 changes: 10 additions & 24 deletions src/parser/parser.rs
Expand Up @@ -1078,15 +1078,6 @@ impl<'cmd> Parser<'cmd> {
matcher.add_index_to(arg.get_id(), self.cur_idx.get());
}

// Increment or create the group "args"
for group in self.cmd.groups_for_arg(arg.get_id()) {
matcher.add_val_to(
&group,
AnyValue::new(arg.get_id().clone()),
OsString::from(arg.get_id().as_str()),
);
}

Ok(())
}

Expand Down Expand Up @@ -1499,20 +1490,15 @@ impl<'cmd> Parser<'cmd> {
self.remove_overrides(arg, matcher);
}
matcher.start_custom_arg(arg, source);
for group in self.cmd.groups_for_arg(arg.get_id()) {
matcher.start_custom_group(group, source);
}
}

/// Increase occurrence of specific argument and the grouped arg it's in.
fn start_occurrence_of_arg(&self, matcher: &mut ArgMatcher, arg: &Arg) {
// With each new occurrence, remove overrides from prior occurrences
self.remove_overrides(arg, matcher);

matcher.start_occurrence_of_arg(arg);
// Increment or create the group "args"
for group in self.cmd.groups_for_arg(arg.get_id()) {
matcher.start_occurrence_of_group(group);
if source.is_explicit() {
for group in self.cmd.groups_for_arg(arg.get_id()) {
matcher.start_custom_group(group.clone(), source);
matcher.add_val_to(
&group,
AnyValue::new(arg.get_id().clone()),
OsString::from(arg.get_id().as_str()),
);
}
}
}
}
Expand Down Expand Up @@ -1549,7 +1535,7 @@ impl<'cmd> Parser<'cmd> {
// Add the arg to the matches to build a proper usage string
if let Some((name, _)) = did_you_mean.as_ref() {
if let Some(arg) = self.cmd.get_keymap().get(&name.as_ref()) {
self.start_occurrence_of_arg(matcher, arg);
self.start_custom_arg(matcher, arg, ValueSource::CommandLine);
}
}

Expand Down
9 changes: 6 additions & 3 deletions tests/builder/groups.rs
Expand Up @@ -73,9 +73,10 @@ fn group_single_value() {
#[test]
fn group_empty() {
let res = Command::new("group")
.arg(arg!(-f --flag "some flag"))
.arg(arg!(-c --color [color] "some option"))
.arg(arg!(-n --hostname <name> "another option"))
.group(ArgGroup::new("grp").args(["hostname", "color"]))
.group(ArgGroup::new("grp").args(["hostname", "color", "flag"]))
.try_get_matches_from(vec![""]);
assert!(res.is_ok(), "{}", res.unwrap_err());

Expand All @@ -87,12 +88,13 @@ fn group_empty() {
#[test]
fn group_required_flags_empty() {
let result = Command::new("group")
.arg(arg!(-f --flag "some flag"))
.arg(arg!(-c --color "some option"))
.arg(arg!(-n --hostname <name> "another option"))
.group(
ArgGroup::new("grp")
.required(true)
.args(["hostname", "color"]),
.args(["hostname", "color", "flag"]),
)
.try_get_matches_from(vec![""]);
assert!(result.is_err());
Expand All @@ -103,9 +105,10 @@ fn group_required_flags_empty() {
#[test]
fn group_multi_value_single_arg() {
let res = Command::new("group")
.arg(arg!(-f --flag "some flag"))
.arg(arg!(-c --color <color> "some option").num_args(1..))
.arg(arg!(-n --hostname <name> "another option"))
.group(ArgGroup::new("grp").args(["hostname", "color"]))
.group(ArgGroup::new("grp").args(["hostname", "color", "flag"]))
.try_get_matches_from(vec!["", "-c", "blue", "red", "green"]);
assert!(res.is_ok(), "{:?}", res.unwrap_err().kind());

Expand Down