From 343d47dcbf83786a45c0d0f01b27fd9dd76725de Mon Sep 17 00:00:00 2001 From: Kevin K Date: Thu, 2 Apr 2015 20:04:52 -0400 Subject: [PATCH] fix(positional args): all previous positional args become required when a latter one is required Closes #50 --- src/app.rs | 82 +++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 65 insertions(+), 17 deletions(-) diff --git a/src/app.rs b/src/app.rs index e3142a0e..717ad387 100644 --- a/src/app.rs +++ b/src/app.rs @@ -55,6 +55,7 @@ pub struct App<'a, 'v, 'ab, 'u, 'ar> { needs_short_version: bool, needs_subcmd_help: bool, required: HashSet<&'ar str>, + matched_reqs: HashSet<&'ar str>, arg_list: HashSet<&'ar str>, short_list: HashSet, long_list: HashSet<&'ar str>, @@ -91,6 +92,7 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ needs_subcmd_help: true, needs_short_version: true, required: HashSet::new(), + matched_reqs: HashSet::new(), arg_list: HashSet::new(), short_list: HashSet::new(), long_list: HashSet::new(), @@ -216,6 +218,7 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ if a.takes_value { panic!("Argument \"{}\" has conflicting requirements, both index() and takes_value(true) were supplied", a.name); } + // Create the Positional Arguemnt Builder with each HashSet = None to only allocate those that require it let mut pb = PosBuilder { name: a.name, @@ -238,7 +241,8 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ if let Some(ref r) = a.requires { let mut rhs = HashSet::new(); // without derefing n = &&str - for n in r { rhs.insert(*n); } + for n in r { + rhs.insert(*n); } pb.requires = Some(rhs); } // Check if there is anything in the possible values and add those as well @@ -409,16 +413,31 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ } fn print_usage(&self, more_info: bool) { + let tab = " "; println!("USAGE:"); if let Some(u) = self.usage_str { - println!(" {}",u); + println!("{}{}",tab,u); } else { let flags = !self.flags.is_empty(); let pos = !self.positionals_idx.is_empty(); let opts = !self.opts.is_empty(); let subcmds = !self.subcommands.is_empty(); let mut num_req_pos = 0; - let req_pos = self.positionals_idx.values().filter_map(|ref x| if x.required || self.required.contains(x.name) { + let mut matched_pos_reqs = HashSet::new(); + // If it's required we also need to ensure all previous positionals are required too + let mut found = false; + for p in self.positionals_idx.values().rev() { + if found { + matched_pos_reqs.insert(p.name); + continue; + } + if self.matched_reqs.contains(p.name) { + matched_pos_reqs.insert(p.name); + found = true; + } + } + + let req_pos = self.positionals_idx.values().filter_map(|ref x| if x.required || matched_pos_reqs.contains(x.name) { num_req_pos += 1; if x.multiple { Some(format!("<{}>...", x.name)) @@ -430,7 +449,7 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ }) .fold(String::new(), |acc, ref name| acc + &format!("{} ", name)[..]); let mut num_req_opts = 0; - let req_opts = self.opts.values().filter_map(|x| if x.required || self.required.contains(x.name) { + let req_opts = self.opts.values().filter_map(|x| if x.required || self.matched_reqs.contains(x.name) { num_req_opts += 1; Some(x) }else { @@ -442,7 +461,7 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ format!("{} ",o.short.unwrap()) },o.name)); - print!(" {} {} {} {} {}", self.bin_name.clone().unwrap_or(self.name.clone()), + print!("{}{} {} {} {} {}",tab, self.bin_name.clone().unwrap_or(self.name.clone()), if flags {"[FLAGS]"} else {""}, if opts { if num_req_opts != self.opts.len() && !req_opts.is_empty() { @@ -653,7 +672,7 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ pub fn get_matches(mut self) -> ArgMatches<'ar> { self.verify_positionals(); - for sc in self.subcommands.values() { + for (_,sc) in self.subcommands.iter_mut() { sc.verify_positionals(); } @@ -674,7 +693,7 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ matches } - fn verify_positionals(&self) { + fn verify_positionals(&mut self) { // Because you must wait until all arguments have been supplied, this is the first chance // to make assertions on positional argument indexes // @@ -695,6 +714,19 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ panic!("Found positional argument \"{}\" which accepts multiple values but it's not the last positional argument (i.e. others have a higher index)", p.name); } + + // If it's required we also need to ensure all previous positionals are required too + let mut found = false; + for (_, p) in self.positionals_idx.iter_mut().rev() { + if found { + p.required = true; + self.required.insert(p.name); + continue; + } + if p.required { + found = true; + } + } } fn get_matches_from(&mut self, matches: &mut ArgMatches<'ar>, it: &mut IntoIter) { @@ -738,6 +770,7 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ needs_val_of = None; continue; } + if arg_slice.starts_with("--") && !pos_only { if arg_slice.len() == 2 { pos_only = true; @@ -762,6 +795,9 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ format!("Found positional argument {}, but {} doesn't accept any", arg, self.name), true, true); } + // If we find that an argument requires a positiona, we need to update all the + // previous positionals too. This will denote where to start + // let mut req_pos_from_name = None; if let Some(p) = self.positionals_idx.get(&pos_counter) { if self.blacklist.contains(p.name) { self.report_error(format!("The argument \"{}\" is mutually exclusive with one or more other arguments", arg), @@ -806,13 +842,16 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ self.required.remove(name); } } - if self.required.contains(p.name) { + + // No need to check for existance, returns None if not found + // if self.required.contains(p.name) { self.required.remove(p.name); - } + // } if let Some(ref reqs) = p.requires { // Add all required args which aren't already found in matches to the // final required list for n in reqs { + self.matched_reqs.insert(n); if matches.positionals.contains_key(n) {continue;} if matches.opts.contains_key(n) {continue;} if matches.flags.contains_key(n) {continue;} @@ -955,13 +994,15 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ self.required.remove(name); } } - if self.required.contains(v.name) { + // No need to check for existance, returns None if not found + // if self.required.contains(v.name) { self.required.remove(v.name); - } + // } if let Some(ref reqs) = v.requires { // Add all required args which aren't already found in matches to the // final required list for n in reqs { + self.matched_reqs.insert(n); if matches.opts.contains_key(n) { continue; } if matches.flags.contains_key(n) { continue; } if matches.positionals.contains_key(n) { continue; } @@ -1002,9 +1043,10 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ // If this flag was requierd, remove it // .. even though Flags shouldn't be required - if self.required.contains(v.name) { + // No need to check for existance, returns None if not found + // if self.required.contains(v.name) { self.required.remove(v.name); - } + // } // Add all of this flags "mutually excludes" list to the master list if let Some(ref bl) = v.blacklist { @@ -1017,6 +1059,7 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ // Add all required args which aren't already found in matches to the master list if let Some(ref reqs) = v.requires { for n in reqs { + self.matched_reqs.insert(n); if matches.flags.contains_key(n) { continue; } if matches.opts.contains_key(n) { continue; } if matches.positionals.contains_key(n) { continue; } @@ -1080,13 +1123,15 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ self.required.remove(name); } } - if self.required.contains(v.name) { + // No need to check for existance, returns None if not found + // if self.required.contains(v.name) { self.required.remove(v.name); - } + // } if let Some(ref reqs) = v.requires { // Add all required args which aren't already found in matches to the // final required list for n in reqs { + self.matched_reqs.insert(n); if matches.opts.contains_key(n) { continue; } if matches.flags.contains_key(n) { continue; } if matches.positionals.contains_key(n) { continue; } @@ -1130,9 +1175,11 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ // If this flag was requierd, remove it // .. even though Flags shouldn't be required - if self.required.contains(v.name) { + + // No need to check for existance, returns None if it didn't exist + // if self.required.contains(v.name) { self.required.remove(v.name); - } + // } // Add all of this flags "mutually excludes" list to the master list if let Some(ref bl) = v.blacklist { @@ -1145,6 +1192,7 @@ impl<'a, 'v, 'ab, 'u, 'ar> App<'a, 'v, 'ab, 'u, 'ar>{ // Add all required args which aren't already found in matches to the master list if let Some(ref reqs) = v.requires { for n in reqs { + self.matched_reqs.insert(n); if matches.flags.contains_key(n) { continue; } if matches.opts.contains_key(n) { continue; } if matches.positionals.contains_key(n) { continue; }