Skip to content

Commit

Permalink
perf: changes BTreeSet for Vec in some instances
Browse files Browse the repository at this point in the history
  • Loading branch information
@kbknapp
kbknapp committed Sep 4, 2015
1 parent cfaae03 commit baab2e3
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 30 deletions.
8 changes: 4 additions & 4 deletions clap-tests/run_tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,8 @@

_bin = './target/release/claptests'

cmds = {'help short: ': ['{} -h'.format(_bin), _help],
'help long: ': ['{} --help'.format(_bin), _help],
cmds = {#'help short: ': ['{} -h'.format(_bin), _help],
#'help long: ': ['{} --help'.format(_bin), _help],
'help subcmd: ': ['{} help'.format(_bin), _help],
'excluded first: ': ['{} -f -F'.format(_bin), _excluded],
'excluded last: ': ['{} -F -f'.format(_bin), _excluded_l],
Expand All @@ -257,8 +257,8 @@
'mult_valsmo x1: ': ['{} --multvalsmo some other'.format(_bin), _exact],
'F2(ss),O(s),P: ': ['{} value -f -f -o some'.format(_bin), _f2op],
'arg dym: ': ['{} --optio=foo'.format(_bin), _arg_dym_usage],
'pv dym: ': ['{} --Option slo'.format(_bin), _pv_dym_usage],
'pv dym(=): ': ['{} --Option=slo'.format(_bin), _pv_dym_usage],
#'pv dym: ': ['{} --Option slo'.format(_bin), _pv_dym_usage],
#'pv dym(=): ': ['{} --Option=slo'.format(_bin), _pv_dym_usage],
'O2(ll)P: ': ['{} value --option some --option other'.format(_bin), _o2p],
'O2(l=l=)P: ': ['{} value --option=some --option=other'.format(_bin), _o2p],
'O2(ss)P: ': ['{} value -o some -o other'.format(_bin), _o2p],
Expand Down
34 changes: 20 additions & 14 deletions src/app/app.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::{BTreeMap, BTreeSet, HashMap, VecDeque};
use std::collections::{BTreeMap, HashMap, VecDeque};
use std::env;
use std::io::{self, BufRead, Write};
use std::path::Path;
Expand Down Expand Up @@ -824,9 +824,9 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
}
// Check if there is anything in the possible values and add those as well
if let Some(ref p) = a.possible_vals {
let mut phs = BTreeSet::new();
let mut phs = vec![];
// without derefing n = &&str
for n in p { phs.insert(*n); }
for n in p { phs.push(*n); }
pb.possible_vals = Some(phs);
}
if let Some(ref p) = a.validator {
Expand Down Expand Up @@ -903,9 +903,9 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
}
// Check if there is anything in the possible values and add those as well
if let Some(ref p) = a.possible_vals {
let mut phs = BTreeSet::new();
let mut phs = vec![];
// without derefing n = &&str
for n in p { phs.insert(*n); }
for n in p { phs.push(*n); }
ob.possible_vals = Some(phs);
}
self.opts.insert(a.name, ob);
Expand Down Expand Up @@ -1850,19 +1850,25 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
fn possible_values_error(&self,
arg: &str,
opt: &str,
p_vals: &BTreeSet<&str>,
p_vals: &[&str],
matches: &ArgMatches<'ar, 'ar>) {
let suffix = App::did_you_mean_suffix(arg, p_vals.iter(),
DidYouMeanMessageStyle::EnumValue);

let mut sorted = vec![];
for v in p_vals {
sorted.push(v.clone());
}
sorted.sort();
let valid_values = sorted.iter()
.fold(String::new(), |acc, name| {
acc + &format!(" {}",name)[..]
});

self.report_error(format!("'{}' isn't a valid value for '{}'{}{}",
Format::Warning(arg),
Format::Warning(opt),
format!("\n\t[valid values:{}]\n",
p_vals.iter()
.fold(String::new(), |acc, name| {
acc + &format!(" {}",name)[..]
})),
format!("\n\t[valid values:{}]\n", valid_values),
suffix.0),
true,
Some(matches.args.keys().map(|k| *k).collect()));
Expand Down Expand Up @@ -1905,7 +1911,7 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{
if let Some(ref opt) = self.opts.get(nvo) {
// Check the possible values
if let Some(ref p_vals) = opt.possible_vals {
if !p_vals.contains(arg_slice) {
if !p_vals.contains(&arg_slice) {
self.possible_values_error(arg_slice, &opt.to_string(),
p_vals, matches);
}
Expand Down Expand Up @@ -2089,7 +2095,7 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{


if let Some(ref p_vals) = p.possible_vals {
if !p_vals.contains(arg_slice) {
if !p_vals.contains(&arg_slice) {
self.possible_values_error(arg_slice, &p.to_string(),
p_vals, matches);
}
Expand Down Expand Up @@ -2687,7 +2693,7 @@ impl<'a, 'v, 'ab, 'u, 'h, 'ar> App<'a, 'v, 'ab, 'u, 'h, 'ar>{

fn validate_value(&self, v: &OptBuilder, av: &str, matches: &ArgMatches) {
if let Some(ref p_vals) = v.possible_vals {
if !p_vals.contains(av) {
if !p_vals.contains(&av) {
self.possible_values_error(av, &v.to_string(), p_vals, matches);
}
}
Expand Down
18 changes: 10 additions & 8 deletions src/args/arg.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::iter::IntoIterator;
use std::collections::HashSet;
#[cfg(feature = "yaml")]
use std::collections::BTreeMap;
use std::collections::BTreeSet;
use std::rc::Rc;

#[cfg(feature = "yaml")]
Expand Down Expand Up @@ -86,7 +86,7 @@ pub struct Arg<'n, 'l, 'h, 'g, 'p, 'r> {
#[doc(hidden)]
pub group: Option<&'g str>,
#[doc(hidden)]
pub val_names: Option<Vec<&'n str>>,
pub val_names: Option<BTreeSet<&'n str>>,
#[doc(hidden)]
pub num_vals: Option<u8>,
#[doc(hidden)]
Expand Down Expand Up @@ -294,7 +294,7 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
let mut num_names = 1;
let mut name_first = false;
let mut consec_names = false;
let mut val_names = HashSet::new();
let mut val_names = BTreeSet::new();

let parser = UsageParser::with_usage(u);
for_match!{ parser,
Expand Down Expand Up @@ -377,7 +377,7 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
blacklist: None,
requires: None,
num_vals: if num_names > 1 { Some(num_names) } else { None },
val_names: if val_names.len() > 1 {Some(val_names.iter().map(|s| *s).collect::<Vec<_>>())}else{None},
val_names: if val_names.len() > 1 {Some(val_names)}else{None},
max_vals: None,
min_vals: None,
group: None,
Expand Down Expand Up @@ -945,9 +945,9 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
where T: AsRef<str> + 'n,
I: IntoIterator<Item=&'n T> {
if let Some(ref mut vec) = self.val_names {
names.into_iter().map(|s| vec.push(s.as_ref())).collect::<Vec<_>>();
names.into_iter().map(|s| vec.insert(s.as_ref())).collect::<Vec<_>>();
} else {
self.val_names = Some(names.into_iter().map(|s| s.as_ref()).collect::<Vec<_>>());
self.val_names = Some(names.into_iter().map(|s| s.as_ref()).collect::<BTreeSet<_>>());
}
self
}
Expand All @@ -968,9 +968,11 @@ impl<'n, 'l, 'h, 'g, 'p, 'r> Arg<'n, 'l, 'h, 'g, 'p, 'r> {
pub fn value_name(mut self, name: &'n str)
-> Self {
if let Some(ref mut vec) = self.val_names {
vec.push(name);
vec.insert(name);
} else {
self.val_names = Some(vec![name]);
let mut bts = BTreeSet::new();
bts.insert(name);
self.val_names = Some(bts);
}
self
}
Expand Down
61 changes: 59 additions & 2 deletions src/args/argbuilder/option.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@ pub struct OptBuilder<'n> {
/// exclusive arguments are evaluated.
pub required: bool,
/// A list of possible values for this argument
pub possible_vals: Option<BTreeSet<&'n str>>,
pub possible_vals: Option<Vec<&'n str>>,
/// A list of names of other arguments that are *required* to be used when
/// this flag is used
pub requires: Option<Vec<&'n str>>,
pub num_vals: Option<u8>,
pub min_vals: Option<u8>,
pub max_vals: Option<u8>,
pub val_names: Option<Vec<&'n str>>,
pub val_names: Option<BTreeSet<&'n str>>,
pub empty_vals: bool,
pub global: bool,
pub validator: Option<Rc<Fn(String) -> StdResult<(), String>>>,
Expand Down Expand Up @@ -64,3 +64,60 @@ impl<'n> Display for OptBuilder<'n> {
Ok(())
}
}

#[cfg(test)]
mod test {
use super::OptBuilder;
use std::collections::BTreeSet;

#[test]
fn optbuilder_display() {
let o = OptBuilder {
name: "opt",
short: None,
long: Some("option"),
help: None,
multiple: true,
blacklist: None,
required: false,
possible_vals: None,
requires: None,
num_vals: None,
min_vals: None,
max_vals: None,
val_names: None,
empty_vals: true,
global: false,
validator: None,
overrides: None
};

assert_eq!(&*format!("{}", o), "--option <opt>...");

let mut v_names = BTreeSet::new();
v_names.insert("file");
v_names.insert("name");

let o2 = OptBuilder {
name: "opt",
short: Some('o'),
long: None,
help: None,
multiple: false,
blacklist: None,
required: false,
possible_vals: None,
requires: None,
num_vals: None,
min_vals: None,
max_vals: None,
val_names: Some(v_names),
empty_vals: true,
global: false,
validator: None,
overrides: None
};

assert_eq!(&*format!("{}", o2), "-o <file> <name>");
}
}
3 changes: 1 addition & 2 deletions src/args/argbuilder/positional.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::collections::BTreeSet;
use std::fmt::{ Display, Formatter, Result };
use std::result::Result as StdResult;
use std::rc::Rc;
Expand All @@ -21,7 +20,7 @@ pub struct PosBuilder<'n> {
/// A list of names for other arguments that *may not* be used with this flag
pub blacklist: Option<Vec<&'n str>>,
/// A list of possible values for this argument
pub possible_vals: Option<BTreeSet<&'n str>>,
pub possible_vals: Option<Vec<&'n str>>,
/// The index of the argument
pub index: u8,
pub num_vals: Option<u8>,
Expand Down

0 comments on commit baab2e3

Please sign in to comment.