Tests on flags passing

src/build/app/mod.rs

@@ -18,7 +18,7 @@ use yaml_rust::Yaml;
 // Internal
 use build::{Arg, ArgGroup, ArgSettings};
 use completions::{ComplGen, Shell};
-use mkeymap::{KeyType, MKeyMap};
+use mkeymap::MKeyMap;
 use output::fmt::ColorWhen;
 use output::{Help, Usage};
 use parse::errors::Result as ClapResult;
@@ -639,7 +639,7 @@ impl<'a, 'b> App<'a, 'b> {
             None
         };
         let arg = a.into().help_heading(help_heading);
-        self.args.push(arg);
+        self.args.make_entries(arg);
         self
     }
 
@@ -1508,7 +1508,7 @@ impl<'a, 'b> App<'a, 'b> {
             }
             {
                 for a in self.args.values().filter(|a| a.is_set(ArgSettings::Global)) {
-                    sc.args.push(a.clone());
+                    sc.args.make_entries(a.clone());
                 }
             }
             // @TODO @deadcode @perf @v3-alpha: Currently we're not propagating

src/completions/bash.rs

@@ -1,5 +1,4 @@
 // Std
-use std::ffi::OsStr;
 use std::io::Write;
 
 // Internal

src/macros.rs

@@ -903,14 +903,14 @@ macro_rules! flags {
         use mkeymap::KeyType::*;
         $app.args
             .$how()
-            .filter(|(k, a)| !a.settings.is_set(::build::ArgSettings::TakesValue))
-            .filter(|(k, a)| match k {
+            .filter(|(_, a)| !a.settings.is_set(::build::ArgSettings::TakesValue))
+            .filter(|(k, _)| match k {
                 Long(_) => true,
                 Short(_) => true,
                 Position(_) => false,
             })
-            .filter(|(k, a)| !a.help_heading.is_some())
-            .map(|(k, v)| v)
+            .filter(|(_, a)| !a.help_heading.is_some())
+            .map(|(_, v)| v)
     }};
     ($app:expr) => {
         flags!($app, iter)
@@ -929,14 +929,14 @@ macro_rules! opts {
         use mkeymap::KeyType::*;
         $app.args
             .$how()
-            .filter(|(k, a)| a.settings.is_set(::build::ArgSettings::TakesValue))
-            .filter(|(k, a)| match k {
+            .filter(|(_, a)| a.settings.is_set(::build::ArgSettings::TakesValue))
+            .filter(|(k, _)| match k {
                 Long(_) => true,
                 Short(_) => true,
                 Position(_) => false,
             })
-            .filter(|(k, a)| !a.help_heading.is_some())
-            .map(|(k, v)| v)
+            .filter(|(_, a)| !a.help_heading.is_some())
+            .map(|(_, v)| v)
     }};
     ($app:expr) => {
         opts!($app, iter)

src/mkeymap.rs

@@ -4,7 +4,6 @@ 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
 
@@ -97,9 +96,9 @@ where
 
     pub fn is_empty(&self) -> bool { self.keys.is_empty() && self.values.is_empty() }
 
-    pub fn remove_by_name(&mut self, name: &str) -> Option<T> { unimplemented!() }
+    pub fn remove_by_name(&mut self, _name: &str) -> Option<T> { unimplemented!() }
 
-    pub fn remove(&mut self, key: KeyType) -> Option<T> { unimplemented!() }
+    pub fn remove(&mut self, _key: KeyType) -> Option<T> { unimplemented!() }
     //TODO ::remove_many([KeyA, KeyB])
     //? probably shouldn't add a possibility for removal?
     //? or remove by replacement by some dummy object, so the order is preserved
@@ -301,7 +300,6 @@ where
 mod tests {
     use self::KeyType::*;
     use super::*;
-    use std::ffi::OsStr;
 
     #[test]
     fn get_some_value() {

src/parse/parser.rs

@@ -19,9 +19,6 @@ use std::mem;
 )]
 use std::os::unix::ffi::OsStrExt;
 
-// Third party facade
-use util::VecMap;
-
 // Internal
 use build::app::Propagation;
 use build::AppSettings as AS;
@@ -60,9 +57,6 @@ where
     pub app: &'c mut App<'a, 'b>,
     pub required: ChildGraph<&'a str>,
     pub overriden: Vec<&'a str>,
-    //cache: Option<&'a str>,
-    num_opts: usize,
-    num_flags: usize,
     seen: Vec<&'a str>,
     cur_idx: Cell<usize>,
 }
@@ -88,8 +82,6 @@ where
             app: app,
             required: ChildGraph::from(reqs),
             overriden: Vec::new(),
-            num_opts: 0,
-            num_flags: 0,
             seen: Vec::new(),
             cur_idx: Cell::new(0),
         }
@@ -157,14 +149,6 @@ where
             //  * a value terminator
             //  * ArgSettings::Last
             //  * The last arg is Required
-            let mut it = self.app.args.keys().filter(|x| {
-                if let KeyType::Position(_) = x {
-                    true
-                } else {
-                    false
-                }
-            });
-            //self.positionals.values().rev();
 
             // We can't pass the closure (it.next()) to the macro directly because each call to
             // find() (iterator, not macro) gets called repeatedly.
@@ -173,8 +157,7 @@ where
                 .args
                 .get(KeyType::Position(highest_idx))
                 .expect(INTERNAL_ERROR_MSG);
-            //let second_to_last_name = it.next().expect(INTERNAL_ERROR_MSG);
-            //let last = find!(self.app, last_name).expect(INTERNAL_ERROR_MSG);
+
             let second_to_last = self
                 .app
                 .args
@@ -306,6 +289,8 @@ where
     // Does all the initializing and prepares the parser
     pub(crate) fn _build(&mut self) {
         debugln!("Parser::_build;");
+
+        //I wonder whether this part is even needed if we insert all Args using make_entries
         let mut key: Vec<(KeyType, usize)> = Vec::new();
         for (i, a) in self.app.args.values().enumerate() {
             if let Some(ref index) = a.index {
@@ -530,30 +515,20 @@ where
                 }
             }
 
-            let low_index_mults = self.is_set(AS::LowIndexMultiplePositional)
-                && pos_counter
-                    == (
-                    //TODO make a macro for that
-                    self
+            let positional_count = self
                         .app
                         .args
                         .keys()
                         .filter(|x| if let KeyType::Position(_) = x { true } else { false })
-                        .count() - 1);
-            let missing_pos = self.is_set(AS::AllowMissingPositional)
+                        .count();
+            let is_second_to_last = positional_count > 1         
                 && (pos_counter
-                    == (self
-                        .app
-                        .args
-                        .keys()
-                        .filter(|x| {
-                            if let KeyType::Position(_) = x {
-                                true
-                            } else {
-                                false
-                            }
-                        })
-                        .count() - 1) && !self.is_set(AS::TrailingValues));
+                    == (positional_count - 1));
+
+            let low_index_mults = self.is_set(AS::LowIndexMultiplePositional)
+                && is_second_to_last;
+            let missing_pos = self.is_set(AS::AllowMissingPositional)
+                && is_second_to_last && !self.is_set(AS::TrailingValues);
             debugln!(
                 "Parser::get_matches_with: Positional counter...{}",
                 pos_counter
@@ -825,7 +800,7 @@ where
             }
             sc
         };
-        let mut parser = Parser::new(&mut sc);
+        let parser = Parser::new(&mut sc);
         if help_help {
             let mut pb = Arg::with_name("subcommand")
                 .index(1)
@@ -1106,8 +1081,15 @@ where
             // Option: -o
             // Value: val
             if let Some(opt) = self.app.args.get(KeyType::Short(c)) {
-                debugln!("Parser::parse_short_arg:iter:{}: Found valid opt", c);
+                debugln!("Parser::parse_short_arg:iter:{}: Found valid opt or flag", c);
                 self.app.settings.set(AS::ValidArgFound);
+
+                if !opt.is_set(ArgSettings::TakesValue) {
+                    self.check_for_help_and_version_char(c)?;
+                    ret = self.parse_flag(opt, matcher)?;
+                    continue;
+                }
+
                 // Check for trailing concatenated value
                 let p: Vec<_> = arg.splitn(2, c).collect();
                 debugln!(
@@ -1133,12 +1115,6 @@ where
                 let ret = self.parse_opt(val, opt, false, matcher)?;
 
                 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);
-                self.app.settings.set(AS::ValidArgFound);
-                // Only flags can be help or version
-                self.check_for_help_and_version_char(c)?;
-                ret = self.parse_flag(flag, matcher)?;
             } else {
                 let arg = format!("-{}", c);
                 return Err(ClapError::unknown_argument(