Mirrors/clap
2
0
Fork 0
mirror of https://github.com/clap-rs/clap synced 2025-03-05 07:47:40 +00:00
Code Issues Projects Releases Packages Wiki Activity

Merge #1863

Browse source 
1863: A little cleanup r=pksunkara a=CreepySkeleton



Co-authored-by: CreepySkeleton <creepy-skeleton@yandex.ru>
This commit is contained in:
bors[bot] 2020-04-25 19:25:07 +00:00 • committed by GitHub
commit 5f72b0b083
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 116 additions and 173 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

11
src/build/app/mod.rs
View file

@ -10,6 +10,7 @@ use std::env;
use std::ffi::OsString; use std::ffi::OsString;
use std::fmt; use std::fmt;
use std::io::{self, BufRead, Write}; use std::io::{self, BufRead, Write};
use std::ops::Index;
use std::path::Path; use std::path::Path;
use std::process; use std::process;
@ -26,7 +27,7 @@ use crate::parse::{ArgMatcher, ArgMatches, Input, Parser};
use crate::util::{termcolor::ColorChoice, Id, Key}; use crate::util::{termcolor::ColorChoice, Id, Key};
use crate::INTERNAL_ERROR_MSG; use crate::INTERNAL_ERROR_MSG;
// FIXME (@CreepySkeleton): some of this variants are never constructed // FIXME (@CreepySkeleton): some of these variants are never constructed
#[derive(Clone, Debug, PartialEq, Eq)] #[derive(Clone, Debug, PartialEq, Eq)]
#[allow(unused)] #[allow(unused)]
pub(crate) enum Propagation { pub(crate) enum Propagation {
@ -2076,6 +2077,14 @@ impl<'b> App<'b> {
} }
} }
impl<'b> Index<&'_ Id> for App<'b> {
type Output = Arg<'b>;
fn index(&self, key: &'_ Id) -> &Self::Output {
self.find(key).expect(INTERNAL_ERROR_MSG)
}
}
#[cfg(feature = "yaml")] #[cfg(feature = "yaml")]
impl<'a> From<&'a Yaml> for App<'a> { impl<'a> From<&'a Yaml> for App<'a> {
#[allow(clippy::cognitive_complexity)] #[allow(clippy::cognitive_complexity)]

2
src/macros.rs
View file

@ -620,7 +620,7 @@ macro_rules! groups_for_arg {
debug!("groups_for_arg: name={:?}", $grp); debug!("groups_for_arg: name={:?}", $grp);
$app.groups $app.groups
.iter() .iter()
.filter(|grp| grp.args.iter().any(|a| *a == $grp)) .filter(|grp| grp.args.iter().any(|a| a == $grp))
.map(|grp| grp.id.clone()) .map(|grp| grp.id.clone())
}}; }};
} }

10
src/mkeymap.rs
View file

@ -1,6 +1,8 @@
use crate::build::Arg; use crate::build::Arg;
use crate::util::Id; use crate::util::Id;
use crate::INTERNAL_ERROR_MSG;
use std::ffi::{OsStr, OsString}; use std::ffi::{OsStr, OsString};
use std::ops::Index;
#[derive(PartialEq, Debug, Clone)] #[derive(PartialEq, Debug, Clone)]
pub(crate) struct Key { pub(crate) struct Key {
@ -123,6 +125,14 @@ impl<'b> MKeyMap<'b> {
} }
} }
impl<'b> Index<&'_ KeyType> for MKeyMap<'b> {
type Output = Arg<'b>;
fn index(&self, key: &'_ KeyType) -> &Self::Output {
self.get(key).expect(INTERNAL_ERROR_MSG)
}
}
fn _get_keys(arg: &Arg) -> Vec<KeyType> { fn _get_keys(arg: &Arg) -> Vec<KeyType> {
if let Some(index) = arg.index { if let Some(index) = arg.index {
return vec![KeyType::Position(index)]; return vec![KeyType::Position(index)];

4
src/output/usage.rs
View file

@ -185,7 +185,7 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> {
.filter(|pos| !pos.is_set(ArgSettings::Last)) .filter(|pos| !pos.is_set(ArgSettings::Last))
{ {
debug!("Usage::get_args_tag:iter:{}", pos.name); debug!("Usage::get_args_tag:iter:{}", pos.name);
for grp_s in groups_for_arg!(self.p.app, pos.id) { for grp_s in groups_for_arg!(self.p.app, &pos.id) {
debug!("Usage::get_args_tag:iter:{:?}:iter:{:?}", pos.name, grp_s); debug!("Usage::get_args_tag:iter:{:?}:iter:{:?}", pos.name, grp_s);
// if it's part of a required group we don't want to count it // if it's part of a required group we don't want to count it
if self if self
@ -281,7 +281,7 @@ impl<'b, 'c, 'z> Usage<'b, 'c, 'z> {
continue; continue;
} }
} }
for grp_s in groups_for_arg!(self.p.app, f.id) { for grp_s in groups_for_arg!(self.p.app, &f.id) {
debug!("Usage::needs_flags_tag:iter:iter: grp_s={:?}", grp_s); debug!("Usage::needs_flags_tag:iter:iter: grp_s={:?}", grp_s);
if self if self
.p .p

4
src/parse/features/suggestions.rs
View file

@ -17,10 +17,10 @@ where
let mut candidates: Vec<(f64, String)> = possible_values let mut candidates: Vec<(f64, String)> = possible_values
.into_iter() .into_iter()
.map(|pv| (strsim::jaro_winkler(v, pv.as_ref()), pv.as_ref().to_owned())) .map(|pv| (strsim::jaro_winkler(v, pv.as_ref()), pv.as_ref().to_owned()))
.filter(|(confidence, _pv)| *confidence > 0.8) .filter(|(confidence, _)| *confidence > 0.8)
.collect(); .collect();
candidates.sort_by(|a, b| a.0.partial_cmp(&b.0).unwrap_or(Ordering::Equal)); candidates.sort_by(|a, b| a.0.partial_cmp(&b.0).unwrap_or(Ordering::Equal));
candidates.into_iter().map(|(_confidence, pv)| pv).collect() candidates.into_iter().map(|(_, pv)| pv).collect()
} }
#[cfg(not(feature = "suggestions"))] #[cfg(not(feature = "suggestions"))]

193
src/parse/parser.rs
View file

@ -2,7 +2,6 @@
use std::cell::Cell; use std::cell::Cell;
use std::ffi::{OsStr, OsString}; use std::ffi::{OsStr, OsString};
use std::io::Write; use std::io::Write;
use std::mem;
// Internal // Internal
use crate::build::app::Propagation; use crate::build::app::Propagation;
@ -52,12 +51,8 @@ where
impl Input { impl Input {
pub(crate) fn next(&mut self, new: Option<&[&str]>) -> Option<(&OsStr, Option<&OsStr>)> { pub(crate) fn next(&mut self, new: Option<&[&str]>) -> Option<(&OsStr, Option<&OsStr>)> {
if new.is_some() { if let Some(new) = new {
let mut new_items: Vec<OsString> = new let mut new_items: Vec<OsString> = new.iter().map(OsString::from).collect();
.expect(INTERNAL_ERROR_MSG)
.iter()
.map(OsString::from)
.collect();
for i in self.cursor..self.items.len() { for i in self.cursor..self.items.len() {
new_items.push(self.items[i].clone()); new_items.push(self.items[i].clone());
@ -108,9 +103,8 @@ where
.args .args
.iter() .iter()
.filter(|a| a.settings.is_set(ArgSettings::Required)) .filter(|a| a.settings.is_set(ArgSettings::Required))
.map(|a| a.id.clone())
{ {
reqs.insert(a); reqs.insert(a.id.clone());
} }
Parser { Parser {
@ -131,21 +125,20 @@ where
// positional arguments to verify there are no gaps (i.e. supplying an index of 1 and 3 // positional arguments to verify there are no gaps (i.e. supplying an index of 1 and 3
// but no 2) // but no 2)
let highest_idx = *self let highest_idx = self
.app .app
.args .args
.keys .keys
.iter() .iter()
.map(|x| &x.key)
.filter_map(|x| { .filter_map(|x| {
if let KeyType::Position(n) = x { if let KeyType::Position(n) = x.key {
Some(n) Some(n)
} else { } else {
None None
} }
}) })
.max() .max()
.unwrap_or(&0); .unwrap_or(0);
//_highest_idx(&self.positionals); //_highest_idx(&self.positionals);
@ -155,13 +148,7 @@ where
.keys .keys
.iter() .iter()
.map(|x| &x.key) .map(|x| &x.key)
.filter(|x| { .filter(|x| x.is_position())
if let KeyType::Position(_) = x {
true
} else {
false
}
})
.count(); .count();
assert!( assert!(
@ -185,17 +172,8 @@ where
// We can't pass the closure (it.next()) to the macro directly because each call to // We can't pass the closure (it.next()) to the macro directly because each call to
// find() (iterator, not macro) gets called repeatedly. // find() (iterator, not macro) gets called repeatedly.
let last = self let last = &self.app.args[&KeyType::Position(highest_idx)];
.app let second_to_last = &self.app.args[&KeyType::Position(highest_idx - 1)];
.args
.get(&KeyType::Position(highest_idx))
.expect(INTERNAL_ERROR_MSG);
let second_to_last = self
.app
.args
.get(&KeyType::Position(highest_idx - 1))
.expect(INTERNAL_ERROR_MSG);
// Either the final positional is required // Either the final positional is required
// Or the second to last has a terminator or .last(true) set // Or the second to last has a terminator or .last(true) set
@ -220,13 +198,9 @@ where
); );
// Next we check how many have both Multiple and not a specific number of values set // Next we check how many have both Multiple and not a specific number of values set
let count = positionals!(self.app).fold(0, |acc, p| { let count = positionals!(self.app)
if p.settings.is_set(ArgSettings::MultipleValues) && p.num_vals.is_none() { .filter(|p| p.settings.is_set(ArgSettings::MultipleValues) && p.num_vals.is_none())
acc + 1 .count();
} else {
acc
}
});
let ok = count <= 1 let ok = count <= 1
|| (last.is_set(ArgSettings::Last) || (last.is_set(ArgSettings::Last)
&& last.is_set(ArgSettings::MultipleValues) && last.is_set(ArgSettings::MultipleValues)
@ -301,11 +275,10 @@ where
} }
} }
assert!( assert!(
positionals!(self.app).fold(0, |acc, p| if p.is_set(ArgSettings::Last) { positionals!(self.app)
acc + 1 .filter(|p| p.is_set(ArgSettings::Last))
} else { .count()
acc < 2,
}) < 2,
"Only one positional argument may have last(true) set. Found two." "Only one positional argument may have last(true) set. Found two."
); );
if positionals!(self.app) if positionals!(self.app)
@ -328,10 +301,10 @@ where
debug!("Parser::_build"); debug!("Parser::_build");
//I wonder whether this part is even needed if we insert all Args using make_entries //I wonder whether this part is even needed if we insert all Args using make_entries
let mut key: Vec<(KeyType, usize)> = Vec::new(); let mut key = Vec::new();
let mut counter = 0; let mut counter = 0;
for (i, a) in self.app.args.args.iter_mut().enumerate() { for (i, a) in self.app.args.args.iter_mut().enumerate() {
if a.index == None && a.short == None && a.long == None { if a.index == None && a.is_positional() {
counter += 1; counter += 1;
a.index = Some(counter); a.index = Some(counter);
key.push((KeyType::Position(counter), i)); key.push((KeyType::Position(counter), i));
@ -343,12 +316,8 @@ where
let idx = self.required.insert(a.id.clone()); let idx = self.required.insert(a.id.clone());
// If the arg is required, add all it's requirements to master required list // If the arg is required, add all it's requirements to master required list
if let Some(ref areqs) = a.requires { if let Some(ref areqs) = a.requires {
for name in areqs for (_, name) in areqs.iter().filter(|(val, _)| val.is_none()) {
.iter() self.required.insert_child(idx, name.clone());
.filter(|(val, _)| val.is_none())
.map(|(_, name)| name.clone())
{
self.required.insert_child(idx, name);
} }
} }
} }
@ -369,15 +338,10 @@ where
.args .args
.keys .keys
.iter() .iter()
.map(|x| &x.key) .filter(|x| x.key.is_position())
.filter(|x| x.is_position())
.count()) .count())
}) && positionals!(self.app).last().map_or(false, |p_name| { }) && positionals!(self.app).last().map_or(false, |p_name| {
!self !self.app[&p_name.id].is_set(ArgSettings::Last)
.app
.find(&p_name.id)
.expect(INTERNAL_ERROR_MSG)
.is_set(ArgSettings::Last)
}) { }) {
self.app.settings.set(AS::LowIndexMultiplePositional); self.app.settings.set(AS::LowIndexMultiplePositional);
} }
@ -445,7 +409,7 @@ where
self.unset(AS::ValidNegNumFound); self.unset(AS::ValidNegNumFound);
// Is this a new argument, or values from a previous option? // Is this a new argument, or values from a previous option?
let starts_new_arg = self.is_new_arg(&arg_os, needs_val_of.clone()); let starts_new_arg = self.is_new_arg(&arg_os, &needs_val_of);
if !self.is_set(AS::TrailingValues) && arg_os == "--" && starts_new_arg { if !self.is_set(AS::TrailingValues) && arg_os == "--" && starts_new_arg {
debug!("Parser::get_matches_with: setting TrailingVals=true"); debug!("Parser::get_matches_with: setting TrailingVals=true");
@ -467,9 +431,7 @@ where
sc_name sc_name
); );
if sc_name.is_some() { if let Some(sc_name) = sc_name {
let sc_name = sc_name.expect(INTERNAL_ERROR_MSG);
if sc_name == "help" && !self.is_set(AS::NoAutoHelp) { if sc_name == "help" && !self.is_set(AS::NoAutoHelp) {
self.parse_help_subcommand(it.remaining())?; self.parse_help_subcommand(it.remaining())?;
} }
@ -530,11 +492,11 @@ where
_ => (), _ => (),
} }
} }
} else if let ParseResult::Opt(name) = needs_val_of { } else if let ParseResult::Opt(id) = needs_val_of {
// Check to see if parsing a value from a previous arg // Check to see if parsing a value from a previous arg
let arg = self.app.find(&name).expect(INTERNAL_ERROR_MSG);
// get the option so we can check the settings // get the option so we can check the settings
needs_val_of = self.add_val_to_arg(arg, &arg_os, matcher)?; needs_val_of = self.add_val_to_arg(&self.app[&id], &arg_os, matcher)?;
// get the next value from the iterator // get the next value from the iterator
continue; continue;
} }
@ -564,14 +526,7 @@ where
.args .args
.keys .keys
.iter() .iter()
.map(|x| &x.key) .filter(|x| x.key.is_position())
.filter(|x| {
if let KeyType::Position(_) = x {
true
} else {
false
}
})
.count(); .count();
let is_second_to_last = positional_count > 1 && (pos_counter == (positional_count - 1)); let is_second_to_last = positional_count > 1 && (pos_counter == (positional_count - 1));
@ -608,7 +563,7 @@ where
let n = ArgStr::new(n); let n = ArgStr::new(n);
let sc_match = { self.possible_subcommand(&n).is_some() }; let sc_match = { self.possible_subcommand(&n).is_some() };
if self.is_new_arg(&n, needs_val_of.clone()) if self.is_new_arg(&n, &needs_val_of)
|| sc_match || sc_match
|| !suggestions::did_you_mean(&n.to_string_lossy(), sc_names!(self.app)) || !suggestions::did_you_mean(&n.to_string_lossy(), sc_names!(self.app))
.is_empty() .is_empty()
@ -631,14 +586,7 @@ where
.args .args
.keys .keys
.iter() .iter()
.map(|x| &x.key) .filter(|x| x.key.is_position())
.filter(|x| {
if let KeyType::Position(_) = x {
true
} else {
false
}
})
.count(); .count();
} }
@ -647,7 +595,7 @@ where
.args .args
.args .args
.iter() .iter()
.filter(|a| a.short.is_none() && a.long.is_none()) .filter(|a| a.is_positional())
.find(|p| p.index == Some(pos_counter as u64)) .find(|p| p.index == Some(pos_counter as u64))
{ {
if p.is_set(ArgSettings::Last) && !self.is_set(AS::TrailingValues) { if p.is_set(ArgSettings::Last) && !self.is_set(AS::TrailingValues) {
@ -667,18 +615,17 @@ where
.args .args
.keys .keys
.iter() .iter()
.map(|x| &x.key) .filter(|x| x.key.is_position())
.filter(|x| x.is_position())
.count()) .count())
{ {
self.app.settings.set(AS::TrailingValues); self.app.settings.set(AS::TrailingValues);
} }
self.seen.push(p.id.clone()); self.seen.push(p.id.clone());
let _ = self.add_val_to_arg(p, &arg_os, matcher)?; self.add_val_to_arg(p, &arg_os, matcher)?;
matcher.inc_occurrence_of(&p.id); matcher.inc_occurrence_of(&p.id);
for grp in groups_for_arg!(self.app, p.id) { for grp in groups_for_arg!(self.app, &p.id) {
matcher.inc_occurrence_of(&grp); matcher.inc_occurrence_of(&grp);
} }
@ -764,13 +711,11 @@ where
} }
if let Some(ref pos_sc_name) = subcmd_name { if let Some(ref pos_sc_name) = subcmd_name {
let sc_name = { let sc_name = find_subcmd!(self.app, *pos_sc_name)
find_subcmd!(self.app, *pos_sc_name) .expect(INTERNAL_ERROR_MSG)
.expect(INTERNAL_ERROR_MSG) .name
.name .clone();
.clone() self.parse_subcommand(&sc_name, matcher, it)?;
};
self.parse_subcommand(&*sc_name, matcher, it)?;
} else if self.is_set(AS::SubcommandRequired) { } else if self.is_set(AS::SubcommandRequired) {
let bn = self.app.bin_name.as_ref().unwrap_or(&self.app.name); let bn = self.app.bin_name.as_ref().unwrap_or(&self.app.name);
return Err(ClapError::missing_subcommand( return Err(ClapError::missing_subcommand(
@ -829,7 +774,7 @@ where
fn maybe_inc_pos_counter(&self, pos_counter: &mut usize, id: &Id) { fn maybe_inc_pos_counter(&self, pos_counter: &mut usize, id: &Id) {
debug!("Parser::maybe_inc_pos_counter: arg = {:?}", id); debug!("Parser::maybe_inc_pos_counter: arg = {:?}", id);
let arg = self.app.find(id).expect(INTERNAL_ERROR_MSG); let arg = &self.app[id];
debug!("Parser::maybe_inc_pos_counter: is it positional?"); debug!("Parser::maybe_inc_pos_counter: is it positional?");
// will be incremented by other means. // will be incremented by other means.
@ -839,7 +784,7 @@ where
} }
debug!("No"); debug!("No");
for group in groups_for_arg!(self.app, arg.id.clone()) { for group in groups_for_arg!(self.app, &arg.id) {
debug!("Parser::maybe_inc_pos_counter: group={:?}", group); debug!("Parser::maybe_inc_pos_counter: group={:?}", group);
let group = self.app.groups.iter().find(|g| g.id == group); let group = self.app.groups.iter().find(|g| g.id == group);
@ -847,8 +792,7 @@ where
if let Some(group) = group { if let Some(group) = group {
for arg in group.args.iter() { for arg in group.args.iter() {
debug!("Parser::maybe_inc_pos_counter: arg={:?}", arg); debug!("Parser::maybe_inc_pos_counter: arg={:?}", arg);
let arg = self.app.find(arg).expect(INTERNAL_ERROR_MSG); if self.app[arg].is_positional() {
if arg.is_positional() {
debug!("Parser::maybe_inc_pos_counter: Incrementing counter!"); debug!("Parser::maybe_inc_pos_counter: Incrementing counter!");
*pos_counter += 1; *pos_counter += 1;
} }
@ -868,11 +812,7 @@ where
return None; return None;
} }
if !self.is_set(AS::InferSubcommands) { if self.is_set(AS::InferSubcommands) {
if let Some(sc) = find_subcmd!(self.app, arg_os) {
return Some(&sc.name);
}
} else {
let v = sc_names!(self.app) let v = sc_names!(self.app)
.filter(|s| starts(s, &*arg_os)) .filter(|s| starts(s, &*arg_os))
.collect::<Vec<_>>(); .collect::<Vec<_>>();
@ -886,6 +826,8 @@ where
return Some(sc); return Some(sc);
} }
} }
} else if let Some(sc) = find_subcmd!(self.app, arg_os) {
return Some(&sc.name);
} }
None None
@ -898,7 +840,7 @@ where
let mut bin_name = self.app.bin_name.as_ref().unwrap_or(&self.app.name).clone(); let mut bin_name = self.app.bin_name.as_ref().unwrap_or(&self.app.name).clone();
let mut sc = { let mut sc = {
// @TODO @perf: cloning all these Apps ins't great, but since it's just displaying the // @TODO @perf: cloning all these Apps isn't great, but since it's just displaying the
// help message there are bigger fish to fry // help message there are bigger fish to fry
let mut sc = self.app.clone(); let mut sc = self.app.clone();
@ -919,7 +861,7 @@ where
if i == cmds.len() - 1 { if i == cmds.len() - 1 {
break; break;
} }
} else if let Some(mut c) = find_subcmd_cloned!(sc, &*cmd.to_string_lossy()) { } else if let Some(mut c) = find_subcmd_cloned!(sc, &cmd.to_string_lossy()) {
c._build(); c._build();
sc = c; sc = c;
@ -934,7 +876,7 @@ where
)?); )?);
} }
bin_name = format!("{} {}", bin_name, &*sc.name); bin_name = format!("{} {}", bin_name, &sc.name);
} }
sc sc
@ -961,7 +903,7 @@ where
Err(parser.help_err(false)) Err(parser.help_err(false))
} }
fn is_new_arg(&mut self, arg_os: &ArgStr<'_>, needs_val_of: ParseResult) -> bool { fn is_new_arg(&mut self, arg_os: &ArgStr<'_>, needs_val_of: &ParseResult) -> bool {
debug!("Parser::is_new_arg: {:?}:{:?}", arg_os, needs_val_of); debug!("Parser::is_new_arg: {:?}:{:?}", arg_os, needs_val_of);
let app_wide_settings = if self.is_set(AS::AllowLeadingHyphen) { let app_wide_settings = if self.is_set(AS::AllowLeadingHyphen) {
@ -980,14 +922,10 @@ where
}; };
let arg_allows_tac = match needs_val_of { let arg_allows_tac = match needs_val_of {
ParseResult::Opt(name) => { ParseResult::Opt(name) | ParseResult::Pos(name) => {
let o = self.app.find(&name).expect(INTERNAL_ERROR_MSG); app_wide_settings || self.app[name].is_set(ArgSettings::AllowHyphenValues)
o.is_set(ArgSettings::AllowHyphenValues) || app_wide_settings
}
ParseResult::Pos(name) => {
let p = self.app.find(&name).expect(INTERNAL_ERROR_MSG);
p.is_set(ArgSettings::AllowHyphenValues) || app_wide_settings
} }
ParseResult::ValuesDone(..) => return true, ParseResult::ValuesDone(..) => return true,
_ => false, _ => false,
}; };
@ -1042,7 +980,8 @@ where
mid_string.push_str(" "); mid_string.push_str(" ");
if let Some(id) = find_subcmd!(self.app, sc_name).map(|x| x.id.clone()) { if let Some(x) = find_subcmd!(self.app, sc_name) {
let id = x.id.clone();
self.app._propagate(Propagation::To(id)); self.app._propagate(Propagation::To(id));
} }
@ -1151,7 +1090,7 @@ where
}; };
self.app.long_about.is_some() self.app.long_about.is_some()
|| self.app.args.args.iter().any(|f| should_long(&f)) || self.app.args.args.iter().any(should_long)
|| self.app.subcommands.iter().any(|s| s.long_about.is_some()) || self.app.subcommands.iter().any(|s| s.long_about.is_some())
} }
@ -1339,7 +1278,7 @@ where
matcher.inc_occurrence_of(&opt.id); matcher.inc_occurrence_of(&opt.id);
// Increment or create the group "args" // Increment or create the group "args"
for grp in groups_for_arg!(self.app, opt.id) { for grp in groups_for_arg!(self.app, &opt.id) {
matcher.inc_occurrence_of(&grp); matcher.inc_occurrence_of(&grp);
} }
@ -1414,7 +1353,7 @@ where
matcher.add_index_to(&arg.id, self.cur_idx.get()); matcher.add_index_to(&arg.id, self.cur_idx.get());
// Increment or create the group "args" // Increment or create the group "args"
for grp in groups_for_arg!(self.app, arg.id) { for grp in groups_for_arg!(self.app, &arg.id) {
matcher.add_val_to(&grp, v.to_os_string()); matcher.add_val_to(&grp, v.to_os_string());
} }
@ -1430,7 +1369,7 @@ where
matcher.inc_occurrence_of(&flag.id); matcher.inc_occurrence_of(&flag.id);
matcher.add_index_to(&flag.id, self.cur_idx.get()); matcher.add_index_to(&flag.id, self.cur_idx.get());
// Increment or create the group "args" // Increment or create the group "args"
for grp in groups_for_arg!(self.app, flag.id) { for grp in groups_for_arg!(self.app, &flag.id) {
matcher.inc_occurrence_of(&grp); matcher.inc_occurrence_of(&grp);
} }
@ -1495,11 +1434,9 @@ where
continue; continue;
} }
ma.occurs = 1; ma.occurs = 1;
if !ma.vals.is_empty() {
// This avoids a clone let len = ma.vals.len().saturating_sub(1);
let mut v = vec![ma.vals.pop().expect(INTERNAL_ERROR_MSG)]; ma.vals.drain(0..len);
mem::swap(&mut v, &mut ma.vals);
}
} }
} }
@ -1647,7 +1584,7 @@ where
.args .args
.get(&KeyType::Long(OsString::from(name.0.clone()))) .get(&KeyType::Long(OsString::from(name.0.clone())))
{ {
for g in groups_for_arg!(self.app, opt.id) { for g in groups_for_arg!(self.app, &opt.id) {
matcher.inc_occurrence_of(&g); matcher.inc_occurrence_of(&g);
} }
matcher.insert(&opt.id); matcher.insert(&opt.id);
@ -1657,11 +1594,9 @@ where
let used: Vec<Id> = matcher let used: Vec<Id> = matcher
.arg_names() .arg_names()
.filter(|n| { .filter(|n| {
if let Some(a) = self.app.find(n) { self.app.find(n).map_or(true, |a| {
!(self.required.contains(&a.id) || a.is_set(ArgSettings::Hidden)) !(self.required.contains(&a.id) || a.is_set(ArgSettings::Hidden))
} else { })
true
}
}) })
.cloned() .cloned()
.collect(); .collect();

65
src/parse/validator.rs
View file

@ -40,7 +40,7 @@ impl<'b, 'c, 'z> Validator<'b, 'c, 'z> {
debug!("Validator::validate: needs_val_of={:?}", a); debug!("Validator::validate: needs_val_of={:?}", a);
self.validate_required(matcher)?; self.validate_required(matcher)?;
let o = self.p.app.find(&a).expect(INTERNAL_ERROR_MSG); let o = &self.p.app[&a];
reqs_validated = true; reqs_validated = true;
let should_err = if let Some(v) = matcher.0.args.get(&o.id) { let should_err = if let Some(v) = matcher.0.args.get(&o.id) {
v.vals.is_empty() && !(o.min_vals.is_some() && o.min_vals.unwrap() == 0) v.vals.is_empty() && !(o.min_vals.is_some() && o.min_vals.unwrap() == 0)
@ -100,7 +100,7 @@ impl<'b, 'c, 'z> Validator<'b, 'c, 'z> {
debug!("Validator::validate_arg_values: possible_vals={:?}", p_vals); debug!("Validator::validate_arg_values: possible_vals={:?}", p_vals);
let val_str = val.to_string_lossy(); let val_str = val.to_string_lossy();
let ok = if arg.is_set(ArgSettings::IgnoreCase) { let ok = if arg.is_set(ArgSettings::IgnoreCase) {
p_vals.iter().any(|pv| pv.eq_ignore_ascii_case(&*val_str)) p_vals.iter().any(|pv| pv.eq_ignore_ascii_case(&val_str))
} else { } else {
p_vals.contains(&&*val_str) p_vals.contains(&&*val_str)
}; };
@ -108,11 +108,9 @@ impl<'b, 'c, 'z> Validator<'b, 'c, 'z> {
let used: Vec<Id> = matcher let used: Vec<Id> = matcher
.arg_names() .arg_names()
.filter(|&n| { .filter(|&n| {
if let Some(a) = self.p.app.find(n) { self.p.app.find(n).map_or(true, |a| {
!(self.p.required.contains(&a.id) || a.is_set(ArgSettings::Hidden)) !(a.is_set(ArgSettings::Hidden) || self.p.required.contains(&a.id))
} else { })
true
}
}) })
.cloned() .cloned()
.collect(); .collect();
@ -189,10 +187,10 @@ impl<'b, 'c, 'z> Validator<'b, 'c, 'z> {
let c_with = matcher let c_with = matcher
.arg_names() .arg_names()
.find(|x| x != &first && args_in_group.contains(x)) .find(|x| x != &first && args_in_group.contains(x))
.map(|x| self.p.app.find(x).expect(INTERNAL_ERROR_MSG).to_string()); .map(|x| self.p.app[x].to_string());
debug!("Validator::build_conflict_err: c_with={:?}:group", c_with); debug!("Validator::build_conflict_err: c_with={:?}:group", c_with);
return Err(Error::argument_conflict( return Err(Error::argument_conflict(
self.p.app.find(first).expect(INTERNAL_ERROR_MSG), &self.p.app[first],
c_with, c_with,
&*usg, &*usg,
self.p.app.color(), self.p.app.color(),
@ -204,11 +202,9 @@ impl<'b, 'c, 'z> Validator<'b, 'c, 'z> {
fn validate_conflicts(&mut self, matcher: &mut ArgMatcher) -> ClapResult<()> { fn validate_conflicts(&mut self, matcher: &mut ArgMatcher) -> ClapResult<()> {
debug!("Validator::validate_conflicts"); debug!("Validator::validate_conflicts");
let validate_result = self.validate_exclusive(matcher); self.validate_exclusive(matcher)?;
if validate_result.is_err() {
return validate_result;
}
self.gather_conflicts(matcher); self.gather_conflicts(matcher);
for name in self.c.iter() { for name in self.c.iter() {
debug!("Validator::validate_conflicts:iter:{:?}", name); debug!("Validator::validate_conflicts:iter:{:?}", name);
let mut should_err = false; let mut should_err = false;
@ -229,11 +225,10 @@ impl<'b, 'c, 'z> Validator<'b, 'c, 'z> {
.count() .count()
> 1; > 1;
let conf_with_arg = if let Some(ref c) = g.conflicts { let conf_with_arg = g
c.iter().any(|x| matcher.contains(x)) .conflicts
} else { .as_ref()
false .map_or(false, |c| c.iter().any(|x| matcher.contains(x)));
};
let arg_conf_with_gr = matcher let arg_conf_with_gr = matcher
.arg_names() .arg_names()
@ -300,12 +295,11 @@ impl<'b, 'c, 'z> Validator<'b, 'c, 'z> {
} }
} }
// Now we need to know which groups this arg was a memeber of, to add all other // Now we need to know which groups this arg was a member of, to add all other
// args in that group to the conflicts, as well as any args those args conflict // args in that group to the conflicts, as well as any args those args conflict
// with // with
// // FIXME: probably no need to clone the id here. Review the macro for grp in groups_for_arg!(self.p.app, name) {
for grp in groups_for_arg!(self.p.app, name.clone()) {
if let Some(g) = self if let Some(g) = self
.p .p
.app .app
@ -477,15 +471,13 @@ impl<'b, 'c, 'z> Validator<'b, 'c, 'z> {
) -> ClapResult<()> { ) -> ClapResult<()> {
debug!("Validator::validate_arg_requires:{:?}", a.name); debug!("Validator::validate_arg_requires:{:?}", a.name);
if let Some(ref a_reqs) = a.requires { if let Some(ref a_reqs) = a.requires {
for (val, name) in a_reqs.iter().filter(|(val, _)| val.is_some()) { for (val, name) in a_reqs.iter() {
let missing_req = if let Some(val) = val {
|v| v == val.expect(INTERNAL_ERROR_MSG) && !matcher.contains(&name); let missing_req = |v| v == val && !matcher.contains(&name);
if ma.vals.iter().any(missing_req) { if ma.vals.iter().any(missing_req) {
return self.missing_required_error(matcher, Some(&a.id)); return self.missing_required_error(matcher, Some(&a.id));
} }
} } else if !matcher.contains(&name) {
for (_, name) in a_reqs.iter().filter(|(val, _)| val.is_none()) {
if !matcher.contains(&name) {
return self.missing_required_error(matcher, Some(&name)); return self.missing_required_error(matcher, Some(&name));
} }
} }
@ -593,8 +585,7 @@ impl<'b, 'c, 'z> Validator<'b, 'c, 'z> {
($how:ident, $_self:expr, $a:ident, $m:ident) => {{ ($how:ident, $_self:expr, $a:ident, $m:ident) => {{
$a.r_unless $a.r_unless
.as_ref() .as_ref()
.map(|ru| !ru.iter().$how(|n| $m.contains(n))) .map_or(false, |ru| !ru.iter().$how(|n| $m.contains(n)))
.unwrap_or(false)
}}; }};
} }
if a.is_set(ArgSettings::RequiredUnlessAll) { if a.is_set(ArgSettings::RequiredUnlessAll) {
@ -629,12 +620,10 @@ impl<'b, 'c, 'z> Validator<'b, 'c, 'z> {
let used: Vec<Id> = matcher let used: Vec<Id> = matcher
.arg_names() .arg_names()
.filter(|&n| { .filter(|n| {
if let Some(a) = self.p.app.find(n) { self.p.app.find(n).map_or(true, |a| {
!(self.p.required.contains(&a.id) || a.is_set(ArgSettings::Hidden)) !(a.is_set(ArgSettings::Hidden) || self.p.required.contains(&a.id))
} else { })
true
}
}) })
.cloned() .cloned()
.chain(incl.cloned()) .chain(incl.cloned())