From dadd95eda54a75b5ef2995e29557b687b23074af Mon Sep 17 00:00:00 2001 From: ldm0 Date: Wed, 27 Jan 2021 17:26:39 +0000 Subject: [PATCH] Change the way parser do self override --- src/build/app/settings.rs | 2 +- src/parse/matches/matched_arg.rs | 50 ++++++++++++++++++++++++++++++++ src/parse/parser.rs | 46 +++++++++++------------------ tests/grouped_values.rs | 45 +++++++++++++++++++++++++++- 4 files changed, 112 insertions(+), 31 deletions(-) diff --git a/src/build/app/settings.rs b/src/build/app/settings.rs index bbea73530167..249d2b57d411 100644 --- a/src/build/app/settings.rs +++ b/src/build/app/settings.rs @@ -225,7 +225,7 @@ pub enum AppSettings { /// [`Arg::allow_hyphen_values`]: ./struct.Arg.html#method.allow_hyphen_values AllowLeadingHyphen, - /// Specifies that all arguments override themselves. This is the equivolent to saying the `foo` + /// Specifies that all arguments override themselves. This is the equivalent to saying the `foo` /// arg using [`Arg::overrides_with("foo")`] for all defined arguments. /// /// [`Arg::overrides_with("foo")`]: ./struct.Arg.html#method.overrides_with diff --git a/src/parse/matches/matched_arg.rs b/src/parse/matches/matched_arg.rs index 49b7cc4bf46e..87d4e043f13b 100644 --- a/src/parse/matches/matched_arg.rs +++ b/src/parse/matches/matched_arg.rs @@ -89,6 +89,8 @@ impl MatchedArg { self.vals.iter().flatten().count() == 0 } + // Will be used later + #[allow(dead_code)] pub(crate) fn remove_vals(&mut self, len: usize) { let mut want_remove = len; let mut remove_group = None; @@ -110,6 +112,13 @@ impl MatchedArg { } } + pub(crate) fn override_vals(&mut self) { + let len = self.vals.len(); + if len > 1 { + self.vals.drain(0..len - 1); + } + } + pub(crate) fn contains_val(&self, val: &str) -> bool { self.vals_flatten() .any(|v| OsString::as_os_str(v) == OsStr::new(val)) @@ -124,6 +133,47 @@ impl MatchedArg { mod tests { use super::*; + #[test] + fn test_vals_override() { + 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.override_vals(); + let vals: Vec<&Vec> = m.vals().collect(); + assert_eq!( + vals, + vec![&vec![ + OsString::from("fff"), + OsString::from("ggg"), + OsString::from("hhh"), + OsString::from("iii"), + ]] + ); + m.override_vals(); + + let vals: Vec<&Vec> = m.vals().collect(); + assert_eq!( + vals, + vec![&vec![ + OsString::from("fff"), + OsString::from("ggg"), + OsString::from("hhh"), + OsString::from("iii"), + ]] + ); + } + #[test] fn test_grouped_vals_push_and_append() { let mut m = MatchedArg::new(); diff --git a/src/parse/parser.rs b/src/parse/parser.rs index 34c50c32b8f6..23447284639a 100644 --- a/src/parse/parser.rs +++ b/src/parse/parser.rs @@ -1346,34 +1346,27 @@ impl<'help, 'app> Parser<'help, 'app> { let mut arg_overrides = Vec::new(); for name in matcher.arg_names() { debug!("Parser::remove_overrides:iter:{:?}", name); - if let Some(arg) = self.app.find(name) { - let mut handle_self_override = |o: &Id| { - if (arg.is_set(ArgSettings::MultipleValues) - || arg.is_set(ArgSettings::MultipleOccurrences)) - || !arg.has_switch() - { - return true; + if let Some(overrider) = self.app.find(name) { + let mut override_self = false; + for overridee in &overrider.overrides { + debug!("Parser::remove_overrides:iter:{:?} => {:?}", name, o); + if *overridee == overrider.id { + override_self = true; + } else { + arg_overrides.push((overrider.id.clone(), overridee)); + arg_overrides.push((overridee.clone(), &overrider.id)); } + } + if (self.is_set(AS::AllArgsOverrideSelf) || override_self) + && !overrider.is_set(ArgSettings::MultipleValues) + && !overrider.is_set(ArgSettings::MultipleOccurrences) + && overrider.has_switch() + { debug!( "Parser::remove_overrides:iter:{:?}:iter:{:?}: self override", name, o ); - self_override.push(o.clone()); - false - }; - for o in &arg.overrides { - debug!("Parser::remove_overrides:iter:{:?} => {:?}", name, o); - if *o == arg.id { - if handle_self_override(o) { - continue; - } - } else { - arg_overrides.push((arg.id.clone(), o)); - arg_overrides.push((o.clone(), &arg.id)); - } - } - if self.is_set(AS::AllArgsOverrideSelf) { - let _ = handle_self_override(&arg.id); + self_override.push(overrider.id.clone()); } } } @@ -1391,13 +1384,8 @@ impl<'help, 'app> Parser<'help, 'app> { for name in &self_override { debug!("Parser::remove_overrides:iter:self:{:?}: resetting", name); if let Some(ma) = matcher.get_mut(name) { - if ma.occurs < 2 { - continue; - } ma.occurs = 1; - - let len = ma.num_vals().saturating_sub(1); - ma.remove_vals(len); + ma.override_vals(); } } diff --git a/tests/grouped_values.rs b/tests/grouped_values.rs index 04c2da0ae081..d6a5ca06a9a7 100644 --- a/tests/grouped_values.rs +++ b/tests/grouped_values.rs @@ -1,6 +1,6 @@ mod utils; -use clap::{App, Arg}; +use clap::{App, AppSettings, Arg}; #[test] fn value_sets_works() { @@ -145,3 +145,46 @@ fn value_sets_multiple_positional_arg_last_multiple() { vec![vec!["val2", "val3", "val4", "val5", "val6"]] ); } + +#[test] +fn issue_1374() { + let app = App::new("MyApp").arg( + Arg::new("input") + .takes_value(true) + .long("input") + .overrides_with("input") + .min_values(0), + ); + let matches = app + .clone() + .get_matches_from(&["MyApp", "--input", "a", "b", "c", "--input", "d"]); + let vs = matches.values_of("input").unwrap(); + assert_eq!(vs.collect::>(), vec!["d"]); + let matches = app + .clone() + .get_matches_from(&["MyApp", "--input", "a", "b", "--input", "c", "d"]); + let vs = matches.values_of("input").unwrap(); + assert_eq!(vs.collect::>(), vec!["c", "d"]); +} + +#[test] +fn issue_2171() { + let schema = App::new("ripgrep#1701 reproducer") + .setting(AppSettings::AllArgsOverrideSelf) + .arg(Arg::new("pretty").short('p').long("pretty")) + .arg(Arg::new("search_zip").short('z').long("search-zip")); + + let test_args = &[ + vec!["reproducer", "-pz", "-p"], + vec!["reproducer", "-pzp"], + vec!["reproducer", "-zpp"], + vec!["reproducer", "-pp", "-z"], + vec!["reproducer", "-p", "-p", "-z"], + vec!["reproducer", "-p", "-pz"], + vec!["reproducer", "-ppz"], + ]; + + for argv in test_args { + let _ = schema.clone().try_get_matches_from(argv).unwrap(); + } +}