Mirrors/clap
2
0
Fork 0
mirror of https://github.com/clap-rs/clap synced 2024-11-10 06:44:16 +00:00
Code Issues Projects Releases Packages Wiki Activity

perf: refactors the POSIX override handling to lazy handling

Browse source 
This commit primarily changes to a lazy handling of POSIX overrides by
relying on github.com/bluss/ordermap instead of the old HashMap impl.
The ordermap allows us to keep track of which arguments arrived first,
and therefore determine which ones should be removed when an override
conflict is found.

This has the added benefit of we no longer have to do the bookkeeping to
keep track and override args as they come in, we can do it once at the
end.

Finally, ordermap allows fast Vec like iteration of the keys, which we
end up doing several times. Benching is still TBD once the v3 prep is
done, but this change should have a meaningful impact.
This commit is contained in:
Kevin K 2018-01-25 15:04:41 -05:00
parent e890b647f3
commit 7673dfc085
No known key found for this signature in database
GPG key ID: 17218E4B3692F01A
8 changed files with 129 additions and 160 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
Cargo.toml
View file

@ -23,6 +23,7 @@ appveyor = { repository = "kbknapp/clap-rs" }
bitflags = "1.0"
unicode-width = "0.1.4"
textwrap = "0.9.0"
ordermap = "0.3.5"
strsim = { version = "0.7.0", optional = true }
ansi_term = { version = "0.10.0", optional = true }
yaml-rust = { version = "0.3.5", optional = true }

87
src/app/parser.rs
View file

@ -47,7 +47,7 @@ where
pub app: &'c mut App<'a, 'b>,
pub required: Vec<&'a str>,
pub r_ifs: Vec<(&'a str, &'b str, &'a str)>,
pub overrides: Vec<(&'b str, &'a str)>,
pub overriden: Vec<&'a str>,
cache: Option<&'a str>,
num_opts: usize,
num_flags: usize,
@ -97,7 +97,7 @@ where
app: app,
required: reqs,
r_ifs: Vec::new(),
overrides: Vec::new(),
overriden: Vec::new(),
cache: None,
num_opts: 0,
num_flags: 0,
@ -390,14 +390,14 @@ where
}
if starts_new_arg {
{
let any_arg = find!(self.app, &self.cache.unwrap_or(""));
matcher.process_arg_overrides(
any_arg,
&mut self.overrides,
&mut self.required,
);
}
// {
// let any_arg = find!(self.app, &self.cache.unwrap_or(""));
// matcher.process_arg_overrides(
// any_arg,
// &mut self.overrides,
// &mut self.required,
// );
// }
if arg_os.starts_with(b"--") {
needs_val_of = self.parse_long_arg(matcher, &arg_os)?;
@ -527,14 +527,14 @@ where
self.app.settings.set(AS::TrailingValues);
}
if self.cache.map_or(true, |name| name != p.name) {
{
let any_arg = find!(self.app, &self.cache.unwrap_or(""));
matcher.process_arg_overrides(
any_arg,
&mut self.overrides,
&mut self.required,
);
}
// {
// let any_arg = find!(self.app, &self.cache.unwrap_or(""));
// matcher.process_arg_overrides(
// any_arg,
// &mut self.overrides,
// &mut self.required,
// );
// }
self.cache = Some(p.name);
}
let _ = self.add_val_to_arg(p, &arg_os, matcher)?;
@ -647,10 +647,10 @@ where
}
// In case the last arg was new, we need to process it's overrides
{
let any_arg = find!(self.app, &self.cache.unwrap_or(""));
matcher.process_arg_overrides(any_arg, &mut self.overrides, &mut self.required);
}
// {
// let any_arg = find!(self.app, &self.cache.unwrap_or(""));
// matcher.process_arg_overrides(any_arg, &mut self.overrides, &mut self.required);
// }
self.remove_overrides(matcher);
@ -804,31 +804,32 @@ where
}
fn remove_overrides(&mut self, matcher: &mut ArgMatcher) {
debugln!("Parser::remove_overrides:{:?};", self.overrides);
for &(overr, name) in &self.overrides {
debugln!("Parser::remove_overrides:iter:({},{});", overr, name);
if matcher.is_present(overr) {
debugln!(
"Parser::remove_overrides:iter:({},{}): removing {};",
overr,
name,
name
);
matcher.remove(name);
for i in (0..self.required.len()).rev() {
debugln!(
"Parser::remove_overrides:iter:({},{}): removing required {};",
overr,
name,
name
);
if self.required[i] == name {
self.required.swap_remove(i);
break;
debugln!("Parser::remove_overrides;");
let mut to_rem: Vec<&str> = Vec::new();
let mut seen: Vec<&str> = Vec::new();
for name in matcher.arg_names() {
debugln!("Parser::remove_overrides:iter:{};", name);
if let Some(arg) = find!(self.app, name) {
if let Some(ref overrides) = arg.overrides {
debugln!("Parser::remove_overrides:iter:{}:{:?};", name, overrides);
for o in overrides {
if matcher.is_present(o) && !seen.contains(o) {
debugln!("Parser::remove_overrides:iter:{}:iter:{}: self;", name, o);
to_rem.push(arg.name);
} else {
debugln!("Parser::remove_overrides:iter:{}:iter:{}: other;", name, o);
to_rem.push(o);
}
}
}
seen.push(arg.name);
}
}
for name in &to_rem {
debugln!("Parser::remove_overrides:iter:{}: removing;", name);
matcher.remove(name);
self.overriden.push(name);
}
}
fn parse_subcommand<I, T>(

1
src/app/usage.rs
View file

@ -31,7 +31,6 @@ impl<'a, 'b, 'c, 'z> Usage<'a, 'b, 'c, 'z> {
pub fn create_error_usage(&self, matcher: &ArgMatcher<'a>, extra: Option<&str>) -> String {
let mut args: Vec<_> = matcher
.arg_names()
.iter()
.filter(|ref n| {
if let Some(a) = find!(self.0.app, **n) {
!a.is_set(ArgSettings::Required) && !a.is_set(ArgSettings::Hidden)

136
src/app/validator.rs
View file

@ -146,7 +146,7 @@ impl<'a, 'b, 'c, 'z> Validator<'a, 'b, 'c, 'z> {
Ok(())
}
fn build_err(&self, name: &str, matcher: &ArgMatcher<'a>) -> ClapResult<()> {
fn build_conflict_err(&self, name: &str, matcher: &ArgMatcher<'a>) -> ClapResult<()> {
debugln!("build_err!: name={}", name);
let mut c_with = find_from!(self.0.app, &name, blacklist, &matcher);
c_with = c_with.or(find!(self.0.app, &name)
@ -172,78 +172,24 @@ impl<'a, 'b, 'c, 'z> Validator<'a, 'b, 'c, 'z> {
fn validate_blacklist(&self, matcher: &mut ArgMatcher<'a>) -> ClapResult<()> {
debugln!("Validator::validate_blacklist;");
let mut conflicts: Vec<&str> = vec![];
for (&name, _) in matcher.iter() {
for name in &self.gather_conflicts(matcher) {
debugln!("Validator::validate_blacklist:iter:{};", name);
if let Some(grps) = self.0.groups_for_arg(name) {
for grp in &grps {
if let Some(g) = self.0.app.groups.iter().find(|g| &g.name == grp) {
if !g.multiple {
for arg in &g.args {
if arg == &name {
continue;
}
conflicts.push(arg);
}
}
if let Some(ref gc) = g.conflicts {
conflicts.extend(&*gc);
}
}
}
}
if let Some(arg) = find!(self.0.app, &name) {
if let Some(ref bl) = arg.blacklist {
for conf in bl {
if matcher.get(conf).is_some() {
conflicts.push(conf);
}
}
}
} else {
debugln!("Validator::validate_blacklist:iter:{}:group;", name);
let args = self.0.arg_names_in_group(name);
for arg in &args {
debugln!(
"Validator::validate_blacklist:iter:{}:group:iter:{};",
name,
arg
);
if let Some(ref bl) = find!(self.0.app, arg).unwrap().blacklist {
for conf in bl {
if matcher.get(conf).is_some() {
conflicts.push(conf);
}
}
}
}
}
}
for name in &conflicts {
debugln!(
"Validator::validate_blacklist:iter:{}: Checking blacklisted arg",
name
);
let mut should_err = false;
if groups!(self.0.app).any(|g| &g.name == name) {
debugln!(
"Validator::validate_blacklist:iter:{}: groups contains it...",
name
);
debugln!("Validator::validate_blacklist:iter:{}:group;", name);
for n in self.0.arg_names_in_group(name) {
debugln!(
"Validator::validate_blacklist:iter:{}:iter:{}: looking in group...",
"Validator::validate_blacklist:iter:{}:group:iter:{};",
name,
n
);
if matcher.contains(n) {
debugln!(
"Validator::validate_blacklist:iter:{}:iter:{}: matcher contains it...",
"Validator::validate_blacklist:iter:{}:group:iter:{}: found;",
name,
n
);
return self.build_err(n, matcher);
return self.build_conflict_err(n, matcher);
}
}
} else if let Some(ma) = matcher.get(name) {
@ -254,12 +200,66 @@ impl<'a, 'b, 'c, 'z> Validator<'a, 'b, 'c, 'z> {
should_err = ma.occurs > 0;
}
if should_err {
return self.build_err(*name, matcher);
return self.build_conflict_err(*name, matcher);
}
}
Ok(())
}
fn gather_conflicts(&self, matcher: &mut ArgMatcher<'a>) -> Vec<&'a str> {
debugln!("Validator::gather_conflicts;");
let mut conflicts = vec![];
for name in matcher.arg_names() {
debugln!("Validator::gather_conflicts:iter:{};", name);
if let Some(arg) = find!(self.0.app, name) {
if let Some(ref bl) = arg.blacklist {
for conf in bl {
if matcher.get(conf).is_some() {
conflicts.push(*conf);
}
}
}
if let Some(grps) = self.0.groups_for_arg(name) {
for grp in &grps {
if let Some(g) = find!(self.0.app, grp, groups) {
if !g.multiple {
for g_arg in &g.args {
if &g_arg == &name {
continue;
}
conflicts.push(g_arg);
}
}
if let Some(ref gc) = g.conflicts {
conflicts.extend(&*gc);
}
}
}
}
} else {
debugln!("Validator::gather_conflicts:iter:{}:group;", name);
let args = self.0.arg_names_in_group(name);
for arg in &args {
debugln!(
"Validator::gather_conflicts:iter:{}:group:iter:{};",
name,
arg
);
if let Some(ref bl) =
find!(self.0.app, arg).expect(INTERNAL_ERROR_MSG).blacklist
{
for conf in bl {
if matcher.get(conf).is_some() {
conflicts.push(conf);
}
}
}
}
}
}
conflicts
}
fn validate_matched_args(&self, matcher: &mut ArgMatcher<'a>) -> ClapResult<()> {
debugln!("Validator::validate_matched_args;");
for (name, ma) in matcher.iter() {
@ -438,6 +438,13 @@ impl<'a, 'b, 'c, 'z> Validator<'a, 'b, 'c, 'z> {
Ok(())
}
fn is_missing_required_ok(&self, a: &Arg<'a, 'b>, matcher: &ArgMatcher<'a>) -> bool {
debugln!("Validator::is_missing_required_ok: a={}", a.name);
self.validate_arg_conflicts(a, matcher).unwrap_or(false)
|| self.validate_required_unless(a, matcher).unwrap_or(false)
|| self.0.overriden.contains(&a.name)
}
fn validate_arg_conflicts(&self, a: &Arg<'a, 'b>, matcher: &ArgMatcher<'a>) -> Option<bool> {
debugln!("Validator::validate_arg_conflicts: a={:?};", a.name);
a.blacklist.as_ref().map(|bl| {
@ -506,11 +513,4 @@ impl<'a, 'b, 'c, 'z> Validator<'a, 'b, 'c, 'z> {
self.0.app.color(),
))
}
#[inline]
fn is_missing_required_ok(&self, a: &Arg<'a, 'b>, matcher: &ArgMatcher<'a>) -> bool {
debugln!("Validator::is_missing_required_ok: a={}", a.name);
self.validate_arg_conflicts(a, matcher).unwrap_or(false)
|| self.validate_required_unless(a, matcher).unwrap_or(false)
}
}

53
src/args/arg_matcher.rs
View file

@ -1,10 +1,11 @@
// Std
use std::collections::hash_map::{Entry, Iter};
use std::collections::HashMap;
use std::ffi::OsStr;
use std::ops::Deref;
use std::collections::HashMap;
use std::mem;
// Third Party
use ordermap;
// Internal
use args::{Arg, ArgMatches, MatchedArg, SubCommand};
use args::settings::ArgSettings;
@ -20,44 +21,6 @@ impl<'a> Default for ArgMatcher<'a> {
impl<'a> ArgMatcher<'a> {
pub fn new() -> Self { ArgMatcher::default() }
pub fn process_arg_overrides<'b>(
&mut self,
a: Option<&Arg<'a, 'b>>,
overrides: &mut Vec<(&'b str, &'a str)>,
required: &mut Vec<&'a str>,
) {
debugln!(
"ArgMatcher::process_arg_overrides:{:?};",
a.map_or(None, |a| Some(a.name))
);
if let Some(aa) = a {
if let Some(ref a_overrides) = aa.overrides {
for overr in a_overrides {
debugln!("ArgMatcher::process_arg_overrides:iter:{};", overr);
if self.is_present(overr) {
debugln!(
"ArgMatcher::process_arg_overrides:iter:{}: removing from matches;",
overr
);
self.remove(overr);
for i in (0..required.len()).rev() {
if &required[i] == overr {
debugln!(
"ArgMatcher::process_arg_overrides:iter:{}: removing required;",
overr
);
required.swap_remove(i);
break;
}
}
} else {
overrides.push((overr, aa.name));
}
}
}
}
}
pub fn is_present(&self, name: &str) -> bool { self.0.is_present(name) }
pub fn propagate_globals(&mut self, global_arg_vec: &[&'a str]) {
@ -126,15 +89,17 @@ impl<'a> ArgMatcher<'a> {
pub fn usage(&mut self, usage: String) { self.0.usage = Some(usage); }
pub fn arg_names(&'a self) -> Vec<&'a str> { self.0.args.keys().map(Deref::deref).collect() }
pub fn arg_names(&'a self) -> ordermap::Keys<&'a str, MatchedArg> { self.0.args.keys() }
pub fn entry(&mut self, arg: &'a str) -> Entry<&'a str, MatchedArg> { self.0.args.entry(arg) }
pub fn entry(&mut self, arg: &'a str) -> ordermap::Entry<&'a str, MatchedArg> {
self.0.args.entry(arg)
}
pub fn subcommand(&mut self, sc: SubCommand<'a>) { self.0.subcommand = Some(Box::new(sc)); }
pub fn subcommand_name(&self) -> Option<&str> { self.0.subcommand_name() }
pub fn iter(&self) -> Iter<&str, MatchedArg> { self.0.args.iter() }
pub fn iter(&self) -> ordermap::Iter<&str, MatchedArg> { self.0.args.iter() }
pub fn inc_occurrence_of(&mut self, arg: &'a str) {
debugln!("ArgMatcher::inc_occurrence_of: arg={}", arg);

8
src/args/arg_matches.rs
View file

@ -1,10 +1,12 @@
// Std
use std::borrow::Cow;
use std::collections::HashMap;
use std::ffi::{OsStr, OsString};
use std::iter::Map;
use std::slice::Iter;
// Third Party
use ordermap::OrderMap;
// Internal
use INVALID_UTF8;
use args::MatchedArg;
@ -60,7 +62,7 @@ use args::SubCommand;
#[derive(Debug, Clone)]
pub struct ArgMatches<'a> {
#[doc(hidden)]
pub args: HashMap<&'a str, MatchedArg>,
pub args: OrderMap<&'a str, MatchedArg>,
#[doc(hidden)]
pub subcommand: Option<Box<SubCommand<'a>>>,
#[doc(hidden)]
@ -70,7 +72,7 @@ pub struct ArgMatches<'a> {
impl<'a> Default for ArgMatches<'a> {
fn default() -> Self {
ArgMatches {
args: HashMap::new(),
args: OrderMap::new(),
subcommand: None,
usage: None,
}

1
src/lib.rs
View file

@ -534,6 +534,7 @@ extern crate ansi_term;
extern crate atty;
#[macro_use]
extern crate bitflags;
extern crate ordermap;
#[cfg(feature = "suggestions")]
extern crate strsim;
#[cfg(feature = "wrap_help")]

2
src/macros.rs
View file

@ -955,7 +955,7 @@ macro_rules! find_from {
($app:expr, $arg_name:expr, $from:ident, $matcher:expr) => {{
let mut ret = None;
for k in $matcher.arg_names() {
if let Some(a) = find!($app, &k) {
if let Some(a) = find!($app, k) {
if let Some(ref v) = a.$from {
if v.contains($arg_name) {
ret = Some(a.to_string());