Arg-specific API for MKeyMap

.gitignore

@@ -26,3 +26,6 @@ Cargo.lock
 .vscode/*
 .idea/*
 clap-rs.iml
+
+# Auxiliary files
+test-results.test

src/build/app/mod.rs

@@ -18,11 +18,11 @@ use yaml_rust::Yaml;
 // Internal
 use build::{Arg, ArgGroup, ArgSettings};
 use completions::{ComplGen, Shell};
+use mkeymap::{KeyType, MKeyMap};
 use output::fmt::ColorWhen;
 use output::{Help, Usage};
 use parse::errors::Result as ClapResult;
 use parse::{ArgMatcher, ArgMatches, Parser};
-use mkeymap::{MKeyMap, KeyType};
 
 #[doc(hidden)]
 #[allow(dead_code)]
@@ -638,7 +638,6 @@ impl<'a, 'b> App<'a, 'b> {
             None
         };
         let arg = a.into().help_heading(help_heading);
-        //TODO add push functionality to MKeyMap
         self.args.push(arg);
         self
     }
@@ -679,7 +678,7 @@ impl<'a, 'b> App<'a, 'b> {
         // @TODO @perf @p4 @v3-beta: maybe extend_from_slice would be possible and perform better?
         // But that may also not let us do `&["-a 'some'", "-b 'other']` because of not Into<Arg>
         for arg in args.into_iter() {
-            self.args.push(arg.into());
+            self.args.make_entries(arg.into());
         }
         self
     }
@@ -988,19 +987,8 @@ impl<'a, 'b> App<'a, 'b> {
     where
         F: FnOnce(Arg<'a, 'b>) -> Arg<'a, 'b>,
     {
-        // let i = self
+        self.args.mut_arg(arg, f);
-        //     .args
+
-        //     .values()
-        //     .enumerate()
-        //     .filter_map(|(i, a)| if a.name == arg { Some(i) } else { None })
-        //     .next();
-        let a = if let Some(x) = self.args.remove_by_name(arg) {
-            f(x)
-        } else {
-            let mut x = Arg::with_name(arg);
-            f(x)
-        };
-        self.args.push(a);
         self
     }
 
@@ -1545,7 +1533,7 @@ impl<'a, 'b> App<'a, 'b> {
 
             // we have to set short manually because we're dealing with char's
             arg.short = self.help_short;
-            self.args.push(arg);
+            self.args.make_entries(arg);
         } else {
             self.settings.unset(AppSettings::NeedsLongHelp);
         }
@@ -1560,7 +1548,7 @@ impl<'a, 'b> App<'a, 'b> {
                 .help(self.version_message.unwrap_or("Prints version information"));
             // we have to set short manually because we're dealing with char's
             arg.short = self.version_short;
-            self.args.push(arg);
+            self.args.make_entries(arg);
         } else {
             self.settings.unset(AppSettings::NeedsLongVersion);
         }
@@ -1773,13 +1761,9 @@ impl<'a, 'b> App<'a, 'b> {
 
     pub fn has_positionals(&self) -> bool { positionals!(self).count() > 0 }
 
-    pub fn has_visible_opts(&self) -> bool {
+    pub fn has_visible_opts(&self) -> bool { opts!(self).any(|o| !o.is_set(ArgSettings::Hidden)) }
-        opts!(self).any(|o| !o.is_set(ArgSettings::Hidden))
-    }
 
-    pub fn has_visible_flags(&self) -> bool {
+    pub fn has_visible_flags(&self) -> bool { flags!(self).any(|o| !o.is_set(ArgSettings::Hidden)) }
-        flags!(self).any(|o| !o.is_set(ArgSettings::Hidden))
-    }
 
     pub fn has_visible_positionals(&self) -> bool {
         positionals!(self).any(|o| !o.is_set(ArgSettings::Hidden))
@@ -1944,7 +1928,7 @@ impl<'a, 'b> App<'a, 'b> {
         note = "Use `App::arg(Arg::from(&str)` instead. Will be removed in v3.0-beta"
     )]
     pub fn arg_from_usage(mut self, usage: &'a str) -> Self {
-        self.args.push(Arg::from(usage));
+        self.args.make_entries(Arg::from(usage));
         self
     }
 
@@ -1959,7 +1943,7 @@ impl<'a, 'b> App<'a, 'b> {
             if l.is_empty() {
                 continue;
             }
-            self.args.push(Arg::from(l));
+            self.args.make_entries(Arg::from(l));
         }
         self
     }

src/completions/bash.rs

@@ -1,6 +1,6 @@
 // Std
-use std::io::Write;
 use std::ffi::OsStr;
+use std::io::Write;
 
 // Internal
 use build::{App, Arg};
@@ -182,7 +182,9 @@ complete -F _{name} -o bashdefault -o default {name}
             shorts = shorts!(p).fold(String::new(), |acc, s| format!("{} -{}", acc, s)),
             // Handles aliases too
             // error-handling?
-            longs = longs!(p).fold(String::new(), |acc, l| format!("{} --{}", acc, l.to_str().unwrap())),
+            longs = longs!(p).fold(String::new(), |acc, l| {
+                format!("{} --{}", acc, l.to_str().unwrap())
+            }),
             pos = positionals!(p).fold(String::new(), |acc, p| format!("{} {}", acc, p)),
             // Handles aliases too
             subcmds = sc_names!(p).fold(String::new(), |acc, s| format!("{} {}", acc, s))

src/mkeymap.rs

@@ -4,6 +4,7 @@ use std::collections::hash_map::DefaultHasher;
 use std::collections::{HashMap, HashSet};
 use std::ffi::OsString;
 use std::hash::{Hash, Hasher};
+use std::mem;
 use std::slice;
 // ! rustdoc
 
@@ -18,7 +19,7 @@ pub struct MKeyMap<T> {
 pub enum KeyType {
     Short(char),
     Long(OsString),
-    Position(usize),
+    Position(u64),
 }
 
 impl<T> MKeyMap<T>
@@ -77,16 +78,6 @@ where
     }
     //TODO ::insert_keyset([Long, Key2])
 
-    // pub fn insert_key_by_name(&mut self, key: KeyType, name: &str) {
-    //     let index = self
-    //         .value_index
-    //         .iter()
-    //         .position(|x| x.name == name)
-    //         .expect("No such name found");
-
-    //     self.keys.insert(key, index);
-    // }
-
     // ! Arg mutation functionality
 
     pub fn get(&self, key: KeyType) -> Option<&T> {
@@ -113,7 +104,7 @@ where
     //? probably shouldn't add a possibility for removal?
     //? or remove by replacement by some dummy object, so the order is preserved
 
-    pub fn remove_key(&mut self, key: KeyType) { unimplemented!() }
+    pub fn remove_key(&mut self, key: KeyType) { self.keys.remove(&key); }
     //TODO ::remove_keys([KeyA, KeyB])
 
     pub fn keys(&self) -> Keys<usize> {
@@ -142,6 +133,113 @@ where
     }
 }
 
+impl<'a, 'b> MKeyMap<Arg<'a, 'b>> {
+    pub fn insert_key_by_name(&mut self, key: KeyType, name: &str) {
+        let index = self.find_by_name(name);
+
+        self.keys.insert(key, index);
+    }
+
+    pub fn make_entries(&mut self, arg: Arg<'a, 'b>) -> usize {
+        let short = arg.short.map(|c| KeyType::Short(c));
+        let positional = arg.index.map(|n| KeyType::Position(n));
+
+        let mut longs = arg
+            .aliases
+            .clone()
+            .map(|v| {
+                v.iter()
+                    .map(|(n, _)| KeyType::Long(OsString::from(n)))
+                    .collect()
+            })
+            .unwrap_or(Vec::new());
+
+        longs.extend(arg.long.map(|l| KeyType::Long(OsString::from(l))));
+
+        let index = self.push(arg);
+        short.map(|s| self.insert_key(s, index));
+        positional.map(|p| self.insert_key(p, index));
+        longs.into_iter().map(|l| self.insert_key(l, index)).count();
+
+        index
+    }
+
+    pub fn make_entries_by_index(&mut self, index: usize) {
+        let short;
+        let positional;
+        let mut longs;
+
+        {
+            let arg = &self.value_index[index];
+            short = arg.short.map(|c| KeyType::Short(c));
+            positional = arg.index.map(|n| KeyType::Position(n));
+
+            longs = arg
+                .aliases
+                .clone()
+                .map(|v| {
+                    v.iter()
+                        .map(|(n, _)| KeyType::Long(OsString::from(n)))
+                        .collect()
+                })
+                .unwrap_or(Vec::new());
+            longs.extend(arg.long.map(|l| KeyType::Long(OsString::from(l))));
+        }
+
+        short.map(|s| self.insert_key(s, index));
+        positional.map(|p| self.insert_key(p, index));
+        longs.into_iter().map(|l| self.insert_key(l, index)).count();
+    }
+
+    pub fn mut_arg<F>(&mut self, name: &str, f: F)
+    where
+        F: FnOnce(Arg<'a, 'b>) -> Arg<'a, 'b>,
+    {
+        let index = self.find_by_name(name);
+        let new_arg = f(self.value_index[index].clone());
+
+        let value_key = self
+            .values
+            .iter()
+            .filter(|(_, v)| v.contains(&index))
+            .map(|(k, _)| k)
+            .next()
+            .map(|&x| x);
+        value_key.map(|k| {
+            self.values.entry(k).and_modify(|v| {
+                v.remove(&index);
+            })
+        });
+
+        let mut hasher = DefaultHasher::new();
+
+        new_arg.hash(&mut hasher);
+
+        let hash = hasher.finish();
+        self.values
+            .entry(hash)
+            .and_modify(|x| {
+                x.insert(index);
+            })
+            .or_insert({
+                let mut set = HashSet::new();
+                set.insert(index);
+                set
+            });
+
+        self.value_index.push(new_arg);
+        self.value_index.swap_remove(index);
+        self.make_entries_by_index(index);
+    }
+
+    pub fn find_by_name(&mut self, name: &str) -> usize {
+        self.value_index
+            .iter()
+            .position(|x| x.name == name)
+            .expect("No such name found")
+    }
+}
+
 #[derive(Debug)]
 pub struct Keys<'a, V: 'a> {
     iter: hash_map::Keys<'a, KeyType, V>,
@@ -218,7 +316,6 @@ mod tests {
     }
 
     #[test]
-    #[should_panic]
     fn get_none_value() {
         let mut map: MKeyMap<Arg> = MKeyMap::new();
 

src/parse/parser.rs

@@ -142,7 +142,7 @@ where
             .count();
 
         assert!(
-            highest_idx == num_p,
+            highest_idx == num_p as u64,
             "Found positional argument whose index is {} but there \
              are only {} positional arguments defined",
             highest_idx,
@@ -310,7 +310,7 @@ where
         let mut key: Vec<(KeyType, usize)> = Vec::new();
         for (i, a) in self.app.args.values().enumerate() {
             if let Some(ref index) = a.index {
-                key.push((KeyType::Position((*index) as usize), i));
+                key.push((KeyType::Position(*index), i));
             } else {
                 if let Some(ref c) = a.short {
                     key.push((KeyType::Short(*c), i));
@@ -367,7 +367,6 @@ where
                             }
                         })
                         .count())
-            //? what is happening below?
         })
             && self
                 .app
@@ -641,6 +640,8 @@ where
                 {
                     self.app.settings.set(AS::TrailingValues);
                 }
+                //not sure
+                self.seen.push(p.name);
                 let _ = self.add_val_to_arg(p, &arg_os, matcher)?;
 
                 matcher.inc_occurrence_of(p.name);
@@ -1084,7 +1085,6 @@ where
             sdebugln!("No");
             full_arg.trim_left_matches(b'-')
         };
-        // opts?? Should probably now check once, then check whether it's opt or flag, or sth else
         if let Some(opt) = self.app.args.get(KeyType::Long(arg.into())) {
             debugln!(
                 "Parser::parse_long_arg: Found valid opt '{}'",
@@ -1094,6 +1094,9 @@ where
 
             if opt.is_set(ArgSettings::TakesValue) {
                 let ret = self.parse_opt(val, opt, val.is_some(), matcher)?;
+
+                //not sure
+                self.seen.push(opt.name);
                 // if self.cache.map_or(true, |name| name != opt.name) {
                 //     self.cache = Some(opt.name);
                 // }
@@ -1106,6 +1109,7 @@ where
 
                 self.parse_flag(opt, matcher)?;
 
+                self.seen.push(opt.name);
                 // if self.cache.map_or(true, |name| name != opt.name) {
                 //     self.cache = Some(opt.name);
                 // }
@@ -1188,6 +1192,9 @@ where
                 // Default to "we're expecting a value later"
                 let ret = self.parse_opt(val, opt, false, matcher)?;
 
+                //not sure
+                self.seen.push(opt.name);
+
                 return Ok(ret);
             } else if let Some(flag) = self.app.args.get(KeyType::Short(c)) {
                 debugln!("Parser::parse_short_arg:iter:{}: Found valid flag", c);
@@ -1195,6 +1202,9 @@ where
                 // Only flags can be help or version
                 self.check_for_help_and_version_char(c)?;
                 ret = self.parse_flag(flag, matcher)?;
+
+                //not sure
+                self.seen.push(flag.name);
             } else {
                 let arg = format!("-{}", c);
                 return Err(ClapError::unknown_argument(
@@ -1453,6 +1463,9 @@ where
                             $a.name
                         );
                         $_self.add_val_to_arg($a, OsStr::new(val), $m)?;
+
+                        //not sure
+                        $_self.seen.push($a.name);
                     } else if $m.get($a.name).is_some() {
                         debugln!(
                             "Parser::add_defaults:iter:{}: has user defined vals",
@@ -1462,6 +1475,9 @@ where
                         debugln!("Parser::add_defaults:iter:{}: wasn't used", $a.name);
 
                         $_self.add_val_to_arg($a, OsStr::new(val), $m)?;
+
+                        //not sure
+                        $_self.seen.push($a.name);
                     }
                 } else {
                     debugln!(
@@ -1487,6 +1503,10 @@ where
                             };
                             if add {
                                 $_self.add_val_to_arg($a, OsStr::new(default), $m)?;
+
+                                //not sure
+                                $_self.seen.push($a.name);
+
                                 done = true;
                                 break;
                             }
@@ -1526,10 +1546,16 @@ where
                     if let Some(ref val) = val.1 {
                         self.add_val_to_arg(a, OsStr::new(val), matcher)?;
                     }
+
+                    //not sure
+                    self.seen.push(a.name);
                 } else {
                     if let Some(ref val) = val.1 {
                         self.add_val_to_arg(a, OsStr::new(val), matcher)?;
                     }
+
+                    //not sure
+                    self.seen.push(a.name);
                 }
             }
         }

tests/indices.rs

@@ -8,31 +8,45 @@ use clap::{App, Arg};
 #[test]
 fn indices_mult_opts() {
     let m = App::new("ind")
-		.arg(Arg::with_name("exclude")
+        .arg(
+            Arg::with_name("exclude")
                 .short('e')
                 .takes_value(true)
-			.multiple(true))
+                .multiple(true),
-		.arg(Arg::with_name("include")
+        )
+        .arg(
+            Arg::with_name("include")
                 .short('i')
                 .takes_value(true)
-			.multiple(true))
+                .multiple(true),
+        )
         .get_matches_from(vec!["ind", "-e", "A", "B", "-i", "B", "C", "-e", "C"]);
 
-		assert_eq!(m.indices_of("exclude").unwrap().collect::<Vec<_>>(), &[2, 3, 8]);
+    assert_eq!(
-		assert_eq!(m.indices_of("include").unwrap().collect::<Vec<_>>(), &[5, 6]);
+        m.indices_of("exclude").unwrap().collect::<Vec<_>>(),
+        &[2, 3, 8]
+    );
+    assert_eq!(
+        m.indices_of("include").unwrap().collect::<Vec<_>>(),
+        &[5, 6]
+    );
 }
 
 #[test]
 fn index_mult_opts() {
     let m = App::new("ind")
-		.arg(Arg::with_name("exclude")
+        .arg(
+            Arg::with_name("exclude")
                 .short('e')
                 .takes_value(true)
-			.multiple(true))
+                .multiple(true),
-		.arg(Arg::with_name("include")
+        )
+        .arg(
+            Arg::with_name("include")
                 .short('i')
                 .takes_value(true)
-			.multiple(true))
+                .multiple(true),
+        )
         .get_matches_from(vec!["ind", "-e", "A", "B", "-i", "B", "C", "-e", "C"]);
 
     assert_eq!(m.index_of("exclude"), Some(2));
@@ -42,10 +56,8 @@ fn index_mult_opts() {
 #[test]
 fn index_flag() {
     let m = App::new("ind")
-		.arg(Arg::with_name("exclude")
+        .arg(Arg::with_name("exclude").short('e'))
-			.short('e'))
+        .arg(Arg::with_name("include").short('i'))
-		.arg(Arg::with_name("include")
-			.short('i'))
         .get_matches_from(vec!["ind", "-e", "-i"]);
 
     assert_eq!(m.index_of("exclude"), Some(1));
@@ -55,12 +67,16 @@ fn index_flag() {
 #[test]
 fn index_flags() {
     let m = App::new("ind")
-		.arg(Arg::with_name("exclude")
+        .arg(
+            Arg::with_name("exclude")
                 .short('e')
-			.multiple_occurrences(true))
+                .multiple_occurrences(true),
-		.arg(Arg::with_name("include")
+        )
+        .arg(
+            Arg::with_name("include")
                 .short('i')
-			.multiple_occurrences(true))
+                .multiple_occurrences(true),
+        )
         .get_matches_from(vec!["ind", "-e", "-i", "-e", "-e", "-i"]);
 
     assert_eq!(m.index_of("exclude"), Some(1));
@@ -70,90 +86,133 @@ fn index_flags() {
 #[test]
 fn indices_mult_flags() {
     let m = App::new("ind")
-		.arg(Arg::with_name("exclude")
+        .arg(
+            Arg::with_name("exclude")
                 .short('e')
-			.multiple_occurrences(true))
+                .multiple_occurrences(true),
-		.arg(Arg::with_name("include")
+        )
+        .arg(
+            Arg::with_name("include")
                 .short('i')
-			.multiple_occurrences(true))
+                .multiple_occurrences(true),
+        )
         .get_matches_from(vec!["ind", "-e", "-i", "-e", "-e", "-i"]);
 
-		assert_eq!(m.indices_of("exclude").unwrap().collect::<Vec<_>>(), &[1, 3, 4]);
+    assert_eq!(
-		assert_eq!(m.indices_of("include").unwrap().collect::<Vec<_>>(), &[2, 5]);
+        m.indices_of("exclude").unwrap().collect::<Vec<_>>(),
+        &[1, 3, 4]
+    );
+    assert_eq!(
+        m.indices_of("include").unwrap().collect::<Vec<_>>(),
+        &[2, 5]
+    );
 }
 
 #[test]
 fn indices_mult_flags_combined() {
     let m = App::new("ind")
-		.arg(Arg::with_name("exclude")
+        .arg(
+            Arg::with_name("exclude")
                 .short('e')
-			.multiple_occurrences(true))
+                .multiple_occurrences(true),
-		.arg(Arg::with_name("include")
+        )
+        .arg(
+            Arg::with_name("include")
                 .short('i')
-			.multiple_occurrences(true))
+                .multiple_occurrences(true),
+        )
         .get_matches_from(vec!["ind", "-eieei"]);
 
-		assert_eq!(m.indices_of("exclude").unwrap().collect::<Vec<_>>(), &[1, 3, 4]);
+    assert_eq!(
-		assert_eq!(m.indices_of("include").unwrap().collect::<Vec<_>>(), &[2, 5]);
+        m.indices_of("exclude").unwrap().collect::<Vec<_>>(),
+        &[1, 3, 4]
+    );
+    assert_eq!(
+        m.indices_of("include").unwrap().collect::<Vec<_>>(),
+        &[2, 5]
+    );
 }
 
 #[test]
 fn indices_mult_flags_opt_combined() {
     let m = App::new("ind")
-		.arg(Arg::with_name("exclude")
+        .arg(
+            Arg::with_name("exclude")
                 .short('e')
-			.multiple_occurrences(true))
+                .multiple_occurrences(true),
-		.arg(Arg::with_name("include")
+        )
+        .arg(
+            Arg::with_name("include")
                 .short('i')
-			.multiple_occurrences(true))
+                .multiple_occurrences(true),
-		.arg(Arg::with_name("option")
+        )
-			.short('o')
+        .arg(Arg::with_name("option").short('o').takes_value(true))
-			.takes_value(true))
         .get_matches_from(vec!["ind", "-eieeio", "val"]);
 
-		assert_eq!(m.indices_of("exclude").unwrap().collect::<Vec<_>>(), &[1, 3, 4]);
+    assert_eq!(
-		assert_eq!(m.indices_of("include").unwrap().collect::<Vec<_>>(), &[2, 5]);
+        m.indices_of("exclude").unwrap().collect::<Vec<_>>(),
+        &[1, 3, 4]
+    );
+    assert_eq!(
+        m.indices_of("include").unwrap().collect::<Vec<_>>(),
+        &[2, 5]
+    );
     assert_eq!(m.indices_of("option").unwrap().collect::<Vec<_>>(), &[7]);
 }
 
 #[test]
 fn indices_mult_flags_opt_combined_eq() {
     let m = App::new("ind")
-		.arg(Arg::with_name("exclude")
+        .arg(
+            Arg::with_name("exclude")
                 .short('e')
-			.multiple_occurrences(true))
+                .multiple_occurrences(true),
-		.arg(Arg::with_name("include")
+        )
+        .arg(
+            Arg::with_name("include")
                 .short('i')
-			.multiple_occurrences(true))
+                .multiple_occurrences(true),
-		.arg(Arg::with_name("option")
+        )
-			.short('o')
+        .arg(Arg::with_name("option").short('o').takes_value(true))
-			.takes_value(true))
         .get_matches_from(vec!["ind", "-eieeio=val"]);
 
-		assert_eq!(m.indices_of("exclude").unwrap().collect::<Vec<_>>(), &[1, 3, 4]);
+    assert_eq!(
-		assert_eq!(m.indices_of("include").unwrap().collect::<Vec<_>>(), &[2, 5]);
+        m.indices_of("exclude").unwrap().collect::<Vec<_>>(),
+        &[1, 3, 4]
+    );
+    assert_eq!(
+        m.indices_of("include").unwrap().collect::<Vec<_>>(),
+        &[2, 5]
+    );
     assert_eq!(m.indices_of("option").unwrap().collect::<Vec<_>>(), &[7]);
 }
 
 #[test]
 fn indices_mult_opt_value_delim_eq() {
     let m = App::new("myapp")
-	    .arg(Arg::with_name("option")
+        .arg(
+            Arg::with_name("option")
                 .short('o')
                 .takes_value(true)
                 .use_delimiter(true)
-		    .multiple(true))
+                .multiple(true),
+        )
         .get_matches_from(vec!["myapp", "-o=val1,val2,val3"]);
-    assert_eq!(m.indices_of("option").unwrap().collect::<Vec<_>>(), &[2, 3, 4]);
+    assert_eq!(
+        m.indices_of("option").unwrap().collect::<Vec<_>>(),
+        &[2, 3, 4]
+    );
 }
 
 #[test]
 fn indices_mult_opt_value_no_delim_eq() {
     let m = App::new("myapp")
-	    .arg(Arg::with_name("option")
+        .arg(
+            Arg::with_name("option")
                 .short('o')
                 .takes_value(true)
-		    .multiple(true))
+                .multiple(true),
+        )
         .get_matches_from(vec!["myapp", "-o=val1,val2,val3"]);
     assert_eq!(m.indices_of("option").unwrap().collect::<Vec<_>>(), &[2]);
 }
@@ -161,13 +220,13 @@ fn indices_mult_opt_value_no_delim_eq() {
 #[test]
 fn indices_mult_opt_mult_flag() {
     let m = App::new("myapp")
-    .arg(Arg::with_name("option")
+        .arg(
+            Arg::with_name("option")
                 .short('o')
                 .takes_value(true)
-	    .multiple_occurrences(true))
+                .multiple_occurrences(true),
-    .arg(Arg::with_name("flag")
+        )
-	    .short('f')
+        .arg(Arg::with_name("flag").short('f').multiple_occurrences(true))
-	    .multiple_occurrences(true))
         .get_matches_from(vec!["myapp", "-o", "val1", "-f", "-o", "val2", "-f"]);
 
     assert_eq!(m.indices_of("option").unwrap().collect::<Vec<_>>(), &[2, 5]);