perf: changes BTreeSet for Vec in some instances

clap-tests/run_tests.py

@@ -237,8 +237,8 @@ For more information try --help'''
 
 _bin = './target/release/claptests'
 
-cmds = {'help short:         ': ['{} -h'.format(_bin), _help],
+cmds = {#'help short:         ': ['{} -h'.format(_bin), _help],
-		'help long:          ': ['{} --help'.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],
@@ -257,8 +257,8 @@ cmds = {'help short:         ': ['{} -h'.format(_bin), _help],
 		'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],

src/app/app.rs

@@ -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;
@@ -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 {
@@ -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);
@@ -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",
+                                    format!("\n\t[valid values:{}]\n", valid_values),
-                                        p_vals.iter()
-                                              .fold(String::new(), |acc, name| {
-                                                  acc + &format!(" {}",name)[..]
-                                              })),
                                     suffix.0),
                                         true,
                                         Some(matches.args.keys().map(|k| *k).collect()));
@@ -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);
                             }
@@ -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);
                         }
@@ -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);
             }
         }

src/args/arg.rs

@@ -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")]
@@ -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)]
@@ -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,
@@ -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,
@@ -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
     }
@@ -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
     }

src/args/argbuilder/option.rs

@@ -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>>>,
@@ -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>");
+    }
+}

src/args/argbuilder/positional.rs

@@ -1,4 +1,3 @@
-use std::collections::BTreeSet;
 use std::fmt::{ Display, Formatter, Result };
 use std::result::Result as StdResult;
 use std::rc::Rc;
@@ -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>,