Repository: kbknapp/clap-rs
Pull request #947 (2017-05-07 00:33:00)

Issues 940,944,946


<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/kbknapp/clap-rs/947)
<!-- Reviewable:end -->


coveralls: 
[![Coverage Status](https://coveralls.io/builds/11405838/badge)](https://coveralls.io/builds/11405838)

Coverage decreased (-0.006%) to 91.807% when pulling **82c3fbc6d47ea5a2172c33bb7d998f5b58cf1251 on issues-940,944,946** into **bd4dbb42142cb9c44f5bb0c7b602fcf696614f8f on master**.

kbknapp: @homu r+
homu: :pushpin: Commit 82c3fbc has been approved by `kbknapp`

<!-- @homu r=kbknapp 82c3fbc6d47ea5a2172c33bb7d998f5b58cf1251 -->
homu: :hourglass: Testing commit 82c3fbc with merge 932e247...
homu: :boom: Test timed out
kbknapp: @homu retry
homu: :hourglass: Testing commit 82c3fbc with merge 2812652...

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index eaeda815..5a08349b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,15 @@
+<a name="v2.24.0"></a>
+### v2.24.0 (2017-05-07)
+
+
+#### Bug Fixes
+
+*   fixes a bug where args with last(true) and required(true) set were not being printed in the usage string ([3ac533fe](https://github.com/kbknapp/clap-rs/commit/3ac533fedabf713943eedf006f830a5a486bbe80), closes [#944](https://github.com/kbknapp/clap-rs/issues/944))
+*   fixes a bug that was printing the arg name, instead of value name when Arg::last(true) was used ([e1fe8ac3](https://github.com/kbknapp/clap-rs/commit/e1fe8ac3bc1f9cf4e36df0d881f8419755f1787b), closes [#940](https://github.com/kbknapp/clap-rs/issues/940))
+*   fixes a bug where flags were parsed as flags AND positional values when specific combinations of settings were used ([20f83292](https://github.com/kbknapp/clap-rs/commit/20f83292d070038b8cee2a6b47e91f6b0a2f7871), closes [#946](https://github.com/kbknapp/clap-rs/issues/946))
+
+
+
 <a name="v2.24.0"></a>
 ## v2.24.0 (2017-05-05)
 
diff --git a/Cargo.toml b/Cargo.toml
index 20cbb849..23bf1f24 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 
 name = "clap"
-version = "2.24.0"
+version = "2.24.1"
 authors = ["Kevin K. <kbknapp@gmail.com>"]
 exclude = ["examples/*", "clap-test/*", "tests/*", "benches/*", "*.png", "clap-perf/*", "*.dot"]
 repository = "https://github.com/kbknapp/clap-rs.git"
diff --git a/README.md b/README.md
index fc502d51..ae9a8c7d 100644
--- a/README.md
+++ b/README.md
@@ -45,14 +45,17 @@ Created by [gh-md-toc](https://github.com/ekalinin/github-markdown-toc)
 
 ## What's New
 
-Here's the highlights for v2.24.0
+Here's the highlights for v2.24.1
+
+* fixes a bug where args with last(true) and required(true) set were not being printed in the usage string 
+* fixes a bug that was printing the arg name, instead of value name when Arg::last(true) was used 
+* fixes a bug where flags were parsed as flags AND positional values when specific combinations of settings were used 
+
+Here's the highlights for v2.21.0 to v2.24.0
 
 * **README.md:**  fix some typos 
 * **Arg:**  add `default_value_os` 
 * **arg_matches.rs:**  Added a Default implementation for Values and OsValues iterators. 
-
-Here's the highlights for v2.21.0 to v2.23.3
-
 * **PowerShell Completions:**
   * fixes a bug where powershells completions cant be used if no subcommands are defined
   * massively dedups subcommand names in the generate script to make smaller scripts that are still functionally equiv
diff --git a/src/app/parser.rs b/src/app/parser.rs
index 4c6b8664..84d999e7 100644
--- a/src/app/parser.rs
+++ b/src/app/parser.rs
@@ -33,6 +33,18 @@ use app::settings::AppSettings as AS;
 use app::validator::Validator;
 use app::usage;
 
+#[derive(Debug, PartialEq, Copy, Clone)]
+#[doc(hidden)]
+pub enum ParseResult<'a> {
+    Flag,
+    Opt(&'a str),
+    Pos(&'a str),
+    MaybeHyphenValue,
+    MaybeNegNum,
+    NotFound,
+    ValuesDone,
+}
+
 #[allow(missing_debug_implementations)]
 #[doc(hidden)]
 #[derive(Clone, Default)]
@@ -619,7 +631,7 @@ impl<'a, 'b> Parser<'a, 'b>
         (false, None)
     }
 
-    fn parse_help_subcommand<I, T>(&self, it: &mut I) -> ClapResult<()>
+    fn parse_help_subcommand<I, T>(&self, it: &mut I) -> ClapResult<ParseResult<'a>>
         where I: Iterator<Item = T>,
               T: Into<OsString>
     {
@@ -682,12 +694,12 @@ impl<'a, 'b> Parser<'a, 'b>
         if sc.meta.bin_name != self.meta.bin_name {
             sc.meta.bin_name = Some(format!("{} {}", bin_name, sc.meta.name));
         }
-        sc._help(false)
+        Err(sc._help(false))
     }
 
     // allow wrong self convention due to self.valid_neg_num = true and it's a private method
     #[cfg_attr(feature = "lints", allow(wrong_self_convention))]
-    fn is_new_arg(&mut self, arg_os: &OsStr, needs_val_of: Option<&'a str>) -> bool {
+    fn is_new_arg(&mut self, arg_os: &OsStr, needs_val_of: ParseResult<'a>) -> bool {
         debugln!("Parser::is_new_arg: arg={:?}, Needs Val of={:?}",
                  arg_os,
                  needs_val_of);
@@ -704,16 +716,22 @@ impl<'a, 'b> Parser<'a, 'b>
         } else {
             false
         };
-        let arg_allows_tac = if let Some(name) = needs_val_of {
-            if let Some(o) = find_by_name!(self, &name, opts, iter) {
+        let arg_allows_tac = match needs_val_of {
+            ParseResult::Opt(name) => {
+                let o = self.opts
+                    .iter()
+                    .find(|o| o.b.name == name)
+                    .expect(INTERNAL_ERROR_MSG);
                 (o.is_set(ArgSettings::AllowLeadingHyphen) || app_wide_settings)
-            } else if let Some(p) = find_by_name!(self, &name, positionals, values) {
+            }
+            ParseResult::Pos(name) => {
+                let p = self.positionals
+                    .values()
+                    .find(|p| p.b.name == name)
+                    .expect(INTERNAL_ERROR_MSG);
                 (p.is_set(ArgSettings::AllowLeadingHyphen) || app_wide_settings)
-            } else {
-                false
             }
-        } else {
-            false
+            _ => false,
         };
         debugln!("Parser::is_new_arg: Arg::allow_leading_hyphen({:?})",
                  arg_allows_tac);
@@ -771,7 +789,7 @@ impl<'a, 'b> Parser<'a, 'b>
         self.create_help_and_version();
 
         let mut subcmd_name: Option<String> = None;
-        let mut needs_val_of: Option<&'a str> = None;
+        let mut needs_val_of: ParseResult<'a> = ParseResult::NotFound;
         let mut pos_counter = 1;
         while let Some(arg) = it.next() {
             let arg_os = arg.into();
@@ -807,29 +825,44 @@ impl<'a, 'b> Parser<'a, 'b>
                 }
 
                 if !starts_new_arg {
-                    if let Some(name) = needs_val_of {
-                        // Check to see if parsing a value from a previous arg
-                        if let Some(arg) = find_by_name!(self, &name, opts, iter) {
+                    match needs_val_of {
+                        ParseResult::Opt(name) => {
+                            // Check to see if parsing a value from a previous arg
+                            let arg = self.opts
+                                .iter()
+                                .find(|o| o.b.name == name)
+                                .expect(INTERNAL_ERROR_MSG);
                             // get the OptBuilder so we can check the settings
-                            needs_val_of = try!(self.add_val_to_arg(&*arg, &arg_os, matcher));
+                            needs_val_of = try!(self.add_val_to_arg(arg, &arg_os, matcher));
                             // get the next value from the iterator
                             continue;
                         }
+                        _ => (),
                     }
                 } else {
                     if arg_os.starts_with(b"--") {
                         needs_val_of = try!(self.parse_long_arg(matcher, &arg_os));
-                        if !(needs_val_of.is_none() && self.is_set(AS::AllowLeadingHyphen)) {
-                            continue;
+                        debugln!("Parser:get_matches_with: After parse_long_arg {:?}",
+                                 needs_val_of);
+                        match needs_val_of {
+                            ParseResult::Flag |
+                            ParseResult::Opt(..) |
+                            ParseResult::ValuesDone => continue,
+                            _ => (),
                         }
+                        // if !(needs_val_of == ParseResult::Flag && self.is_set(AS::AllowLeadingHyphen)) {
+                        //     continue;
+                        // }
                     } else if arg_os.starts_with(b"-") && arg_os.len_() != 1 {
                         // Try to parse short args like normal, if AllowLeadingHyphen or
                         // AllowNegativeNumbers is set, parse_short_arg will *not* throw
                         // an error, and instead return Ok(None)
                         needs_val_of = try!(self.parse_short_arg(matcher, &arg_os));
                         // If it's None, we then check if one of those two AppSettings was set
-                        if needs_val_of.is_none() {
-                            if self.is_set(AS::AllowNegativeNumbers) {
+                        debugln!("Parser:get_matches_with: After parse_short_arg {:?}",
+                                 needs_val_of);
+                        match needs_val_of {
+                            ParseResult::MaybeNegNum => {
                                 if !(arg_os.to_string_lossy().parse::<i64>().is_ok() ||
                                      arg_os.to_string_lossy().parse::<f64>().is_ok()) {
                                     return Err(Error::unknown_argument(&*arg_os.to_string_lossy(),
@@ -837,11 +870,12 @@ impl<'a, 'b> Parser<'a, 'b>
                                         &*usage::create_error_usage(self, matcher, None),
                                         self.color()));
                                 }
-                            } else if !self.is_set(AS::AllowLeadingHyphen) {
-                                continue;
                             }
-                        } else {
-                            continue;
+                            // ParseResult::MaybeHyphenValue => (),
+                            ParseResult::Opt(..) |
+                            ParseResult::Flag |
+                            ParseResult::ValuesDone => continue,
+                            _ => (),
                         }
                     }
                 }
@@ -877,14 +911,14 @@ impl<'a, 'b> Parser<'a, 'b>
             if low_index_mults || missing_pos {
                 if let Some(na) = it.peek() {
                     let n = (*na).clone().into();
-                    needs_val_of = if needs_val_of.is_none() {
+                    needs_val_of = if needs_val_of != ParseResult::ValuesDone {
                         if let Some(p) = self.positionals.get(pos_counter) {
-                            Some(p.b.name)
+                            ParseResult::Pos(p.b.name)
                         } else {
-                            None
+                            ParseResult::ValuesDone
                         }
                     } else {
-                        None
+                        ParseResult::ValuesDone
                     };
                     let sc_match = {
                         self.possible_subcommand(&n).0
@@ -1241,11 +1275,11 @@ impl<'a, 'b> Parser<'a, 'b>
                arg.to_str().unwrap());
         if arg == "help" && self.is_set(AS::NeedsLongHelp) {
             sdebugln!("Help");
-            try!(self._help(true));
+            return Err(self._help(true));
         }
         if arg == "version" && self.is_set(AS::NeedsLongVersion) {
             sdebugln!("Version");
-            try!(self._version(true));
+            return Err(self._version(true));
         }
         sdebugln!("Neither");
 
@@ -1259,13 +1293,13 @@ impl<'a, 'b> Parser<'a, 'b>
         if let Some(h) = self.help_short {
             if arg == h && self.is_set(AS::NeedsLongHelp) {
                 sdebugln!("Help");
-                try!(self._help(false));
+                return Err(self._help(false));
             }
         }
         if let Some(v) = self.version_short {
             if arg == v && self.is_set(AS::NeedsLongVersion) {
                 sdebugln!("Version");
-                try!(self._version(false));
+                return Err(self._version(false));
             }
         }
         sdebugln!("Neither");
@@ -1285,34 +1319,40 @@ impl<'a, 'b> Parser<'a, 'b>
         ul
     }
 
-    fn _help(&self, mut use_long: bool) -> ClapResult<()> {
+    fn _help(&self, mut use_long: bool) -> Error {
         debugln!("Parser::_help: use_long={:?}", use_long);
         use_long = use_long && self.use_long_help();
         let mut buf = vec![];
-        try!(Help::write_parser_help(&mut buf, self, use_long));
-        Err(Error {
-                message: unsafe { String::from_utf8_unchecked(buf) },
-                kind: ErrorKind::HelpDisplayed,
-                info: None,
-            })
+        match Help::write_parser_help(&mut buf, self, use_long) {
+            Err(e) => return e,
+            _ => (),
+        }
+        Error {
+            message: unsafe { String::from_utf8_unchecked(buf) },
+            kind: ErrorKind::HelpDisplayed,
+            info: None,
+        }
     }
 
-    fn _version(&self, use_long: bool) -> ClapResult<()> {
+    fn _version(&self, use_long: bool) -> Error {
         debugln!("Parser::_version: ");
         let out = io::stdout();
         let mut buf_w = BufWriter::new(out.lock());
-        try!(self.print_version(&mut buf_w, use_long));
-        Err(Error {
-                message: String::new(),
-                kind: ErrorKind::VersionDisplayed,
-                info: None,
-            })
+        match self.print_version(&mut buf_w, use_long) {
+            Err(e) => return e,
+            _ => (),
+        }
+        Error {
+            message: String::new(),
+            kind: ErrorKind::VersionDisplayed,
+            info: None,
+        }
     }
 
     fn parse_long_arg(&mut self,
                       matcher: &mut ArgMatcher<'a>,
                       full_arg: &OsStr)
-                      -> ClapResult<Option<&'a str>> {
+                      -> ClapResult<ParseResult<'a>> {
         // maybe here lifetime should be 'a
         debugln!("Parser::parse_long_arg;");
         let mut val = None;
@@ -1349,28 +1389,28 @@ impl<'a, 'b> Parser<'a, 'b>
             try!(self.parse_flag(flag, matcher));
 
             // Handle conflicts, requirements, etc.
-            if self.cache.map_or(true, |name| name != flag.b.name) {
-                arg_post_processing!(self, flag, matcher);
-                self.cache = Some(flag.b.name);
-            }
+            // if self.cache.map_or(true, |name| name != flag.b.name) {
+            arg_post_processing!(self, flag, matcher);
+            // self.cache = Some(flag.b.name);
+            // }
 
-            return Ok(None);
+            return Ok(ParseResult::Flag);
         } else if self.is_set(AS::AllowLeadingHyphen) {
-            return Ok(None);
+            return Ok(ParseResult::MaybeHyphenValue);
         } else if self.is_set(AS::ValidNegNumFound) {
-            return Ok(None);
+            return Ok(ParseResult::MaybeNegNum);
         }
 
         debugln!("Parser::parse_long_arg: Didn't match anything");
         self.did_you_mean_error(arg.to_str().expect(INVALID_UTF8), matcher)
-            .map(|_| None)
+            .map(|_| ParseResult::NotFound)
     }
 
     #[cfg_attr(feature = "lints", allow(len_zero))]
     fn parse_short_arg(&mut self,
                        matcher: &mut ArgMatcher<'a>,
                        full_arg: &OsStr)
-                       -> ClapResult<Option<&'a str>> {
+                       -> ClapResult<ParseResult<'a>> {
         debugln!("Parser::parse_short_arg: full_arg={:?}", full_arg);
         let arg_os = full_arg.trim_left_matches(b'-');
         let arg = arg_os.to_string_lossy();
@@ -1381,15 +1421,16 @@ impl<'a, 'b> Parser<'a, 'b>
             if arg.chars().any(|c| !self.contains_short(c)) {
                 debugln!("Parser::parse_short_arg: LeadingHyphenAllowed yet -{} isn't valid",
                          arg);
-                return Ok(None);
+                return Ok(ParseResult::MaybeHyphenValue);
             }
         } else if self.is_set(AS::ValidNegNumFound) {
             // TODO: Add docs about having AllowNegativeNumbers and `-2` as a valid short
             // May be better to move this to *after* not finding a valid flag/opt?
             debugln!("Parser::parse_short_arg: Valid negative num...");
-            return Ok(None);
+            return Ok(ParseResult::MaybeNegNum);
         }
 
+        let mut ret = ParseResult::NotFound;
         for c in arg.chars() {
             debugln!("Parser::parse_short_arg:iter:{}", c);
             // Check for matching short options, and return the name if there is no trailing
@@ -1430,7 +1471,7 @@ impl<'a, 'b> Parser<'a, 'b>
                 self.settings.set(AS::ValidArgFound);
                 // Only flags can be help or version
                 try!(self.check_for_help_and_version_char(c));
-                try!(self.parse_flag(flag, matcher));
+                ret = try!(self.parse_flag(flag, matcher));
 
                 // Handle conflicts, requirements, overrides, etc.
                 // Must be called here due to mutablilty
@@ -1448,7 +1489,7 @@ impl<'a, 'b> Parser<'a, 'b>
                                                    self.color()));
             }
         }
-        Ok(None)
+        Ok(ret)
     }
 
     fn parse_opt(&self,
@@ -1456,7 +1497,7 @@ impl<'a, 'b> Parser<'a, 'b>
                  opt: &OptBuilder<'a, 'b>,
                  had_eq: bool,
                  matcher: &mut ArgMatcher<'a>)
-                 -> ClapResult<Option<&'a str>> {
+                 -> ClapResult<ParseResult<'a>> {
         debugln!("Parser::parse_opt; opt={}, val={:?}", opt.b.name, val);
         debugln!("Parser::parse_opt; opt.settings={:?}", opt.b.settings);
         let mut has_eq = false;
@@ -1497,59 +1538,61 @@ impl<'a, 'b> Parser<'a, 'b>
            (opt.is_set(ArgSettings::Multiple) && !opt.is_set(ArgSettings::RequireDelimiter) &&
             matcher.needs_more_vals(opt)) {
             debugln!("Parser::parse_opt: More arg vals required...");
-            return Ok(Some(opt.b.name));
+            return Ok(ParseResult::Opt(opt.b.name));
         }
         debugln!("Parser::parse_opt: More arg vals not required...");
-        Ok(None)
+        Ok(ParseResult::ValuesDone)
     }
 
     fn add_val_to_arg<A>(&self,
                          arg: &A,
                          val: &OsStr,
                          matcher: &mut ArgMatcher<'a>)
-                         -> ClapResult<Option<&'a str>>
+                         -> ClapResult<ParseResult<'a>>
         where A: AnyArg<'a, 'b> + Display
     {
         debugln!("Parser::add_val_to_arg; arg={}, val={:?}", arg.name(), val);
-        let mut ret = None;
+        let ret;
         debugln!("Parser::add_val_to_arg; trailing_vals={:?}, DontDelimTrailingVals={:?}",
                  self.is_set(AS::TrailingValues),
                  self.is_set(AS::DontDelimitTrailingValues));
         if !(self.is_set(AS::TrailingValues) && self.is_set(AS::DontDelimitTrailingValues)) {
             if let Some(delim) = arg.val_delim() {
+                let mut iret = ParseResult::ValuesDone;
                 if val.is_empty_() {
-                    ret = try!(self.add_single_val_to_arg(arg, val, matcher));
+                    iret = try!(self.add_single_val_to_arg(arg, val, matcher));
                 } else {
                     for v in val.split(delim as u32 as u8) {
-                        ret = try!(self.add_single_val_to_arg(arg, v, matcher));
+                        iret = try!(self.add_single_val_to_arg(arg, v, matcher));
                     }
                     // If there was a delimiter used, we're not looking for more values
                     if val.contains_byte(delim as u32 as u8) ||
                        arg.is_set(ArgSettings::RequireDelimiter) {
-                        ret = None;
+                        iret = ParseResult::ValuesDone;
                     }
                 }
+                ret = Ok(iret);
             } else {
-                ret = try!(self.add_single_val_to_arg(arg, val, matcher));
+                ret = self.add_single_val_to_arg(arg, val, matcher);
             }
         } else {
-            ret = try!(self.add_single_val_to_arg(arg, val, matcher));
+            ret = self.add_single_val_to_arg(arg, val, matcher);
         }
-        Ok(ret)
+        ret
     }
 
     fn add_single_val_to_arg<A>(&self,
                                 arg: &A,
                                 v: &OsStr,
                                 matcher: &mut ArgMatcher<'a>)
-                                -> ClapResult<Option<&'a str>>
+                                -> ClapResult<ParseResult<'a>>
         where A: AnyArg<'a, 'b> + Display
     {
         debugln!("Parser::add_single_val_to_arg;");
         debugln!("Parser::add_single_val_to_arg: adding val...{:?}", v);
         if let Some(t) = arg.val_terminator() {
             if t == v {
-                return Ok(None);
+                return Ok(ParseResult::ValuesDone);
             }
         }
         matcher.add_val_to(arg.name(), v);
@@ -1562,16 +1605,16 @@ impl<'a, 'b> Parser<'a, 'b>
         }
 
         if matcher.needs_more_vals(arg) {
-            return Ok(Some(arg.name()));
+            return Ok(ParseResult::Opt(arg.name()));
         }
-        Ok(None)
+        Ok(ParseResult::ValuesDone)
     }
 
 
     fn parse_flag(&self,
                   flag: &FlagBuilder<'a, 'b>,
                   matcher: &mut ArgMatcher<'a>)
-                  -> ClapResult<()> {
+                  -> ClapResult<ParseResult<'a>> {
         debugln!("Parser::parse_flag;");
 
         matcher.inc_occurrence_of(flag.b.name);
@@ -1579,7 +1622,7 @@ impl<'a, 'b> Parser<'a, 'b>
         self.groups_for_arg(flag.b.name)
             .and_then(|vec| Some(matcher.inc_occurrences_of(&*vec)));
 
-        Ok(())
+        Ok(ParseResult::Flag)
     }
 
     fn did_you_mean_error(&self, arg: &str, matcher: &mut ArgMatcher<'a>) -> ClapResult<()> {
diff --git a/src/app/settings.rs b/src/app/settings.rs
index abc544a7..6049ccba 100644
--- a/src/app/settings.rs
+++ b/src/app/settings.rs
@@ -162,11 +162,9 @@ pub enum AppSettings {
     /// Specifies that leading hyphens are allowed in argument *values*, such as negative numbers
     /// like `-10`. (which would otherwise be parsed as another flag or option)
     ///
-    /// **NOTE:** This can only be set application wide and not on a per argument basis.
-    ///
     /// **NOTE:** Use this setting with caution as it silences certain circumstances which would
     /// otherwise be an error (such as accidentally forgetting to specify a value for leading
-    /// option)
+    /// option). It is preferred to set this on a per argument basis, via [`Arg::allow_hyphen_values`]
     ///
     /// # Examples
     ///
@@ -183,6 +181,7 @@ pub enum AppSettings {
     /// assert_eq!(m.value_of("neg"), Some("-20"));
     /// # ;
     /// ```
+    /// [`Arg::allow_hyphen_values`]: ./struct.Arg.html#method.allow_hyphen_values
     AllowLeadingHyphen,
 
     /// Allows negative numbers to pass as values. This is similar to
diff --git a/src/app/usage.rs b/src/app/usage.rs
index 5ecf4be1..5fde641e 100644
--- a/src/app/usage.rs
+++ b/src/app/usage.rs
@@ -100,7 +100,7 @@ pub fn create_help_usage(p: &Parser, incl_reqs: bool) -> String {
         usage.push_str(" [--]");
     }
     let not_req_or_hidden =
-        |p: &PosBuilder| !p.is_set(ArgSettings::Required) && !p.is_set(ArgSettings::Hidden);
+        |p: &PosBuilder| (!p.is_set(ArgSettings::Required) || p.is_set(ArgSettings::Last)) && !p.is_set(ArgSettings::Hidden);
     if p.has_positionals() && p.positionals.values().any(not_req_or_hidden) {
         if let Some(args_tag) = get_args_tag(p, incl_reqs) {
             usage.push_str(&*args_tag);
@@ -112,14 +112,16 @@ pub fn create_help_usage(p: &Parser, incl_reqs: bool) -> String {
                 .values()
                 .find(|p| p.b.is_set(ArgSettings::Last))
                 .expect(INTERNAL_ERROR_MSG);
-            debugln!("usage::create_help_usage: {} has .last(true)", pos.name());
+            debugln!("usage::create_help_usage: '{}' has .last(true)", pos.name());
             let req = pos.is_set(ArgSettings::Required);
-            if req {
+            if req && p.positionals.values().any(|p| !p.is_set(ArgSettings::Required)) {
                 usage.push_str(" -- <");
+            } else if req {
+                usage.push_str(" [--] <");
             } else {
                 usage.push_str(" [-- <");
             }
-            usage.push_str(pos.name());
+            usage.push_str(&*pos.name_no_brackets());
             usage.push_str(">");
             usage.push_str(pos.multiple_str());
             if !req {
@@ -216,7 +218,7 @@ fn get_args_tag(p: &Parser, incl_reqs: bool) -> Option<String> {
                       !pos.is_set(ArgSettings::Last)
                   })
             .expect(INTERNAL_ERROR_MSG);
-        debugln!("usage::get_args_tag:iter: Exactly one, returning {}",
+        debugln!("usage::get_args_tag:iter: Exactly one, returning '{}'",
                  pos.name());
         return Some(format!(" [{}]{}", pos.name_no_brackets(), pos.multiple_str()));
     } else if p.is_set(AS::DontCollapseArgsInUsage) && !p.positionals.is_empty() && incl_reqs {
diff --git a/src/app/validator.rs b/src/app/validator.rs
index ad4aa3db..7e8ba468 100644
--- a/src/app/validator.rs
+++ b/src/app/validator.rs
@@ -10,7 +10,7 @@ use errors::{Error, ErrorKind};
 use errors::Result as ClapResult;
 use osstringext::OsStrExt2;
 use app::settings::AppSettings as AS;
-use app::parser::Parser;
+use app::parser::{Parser, ParseResult};
 use fmt::Colorizer;
 use app::usage;
 
@@ -22,30 +22,31 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> {
     pub fn new(p: &'z mut Parser<'a, 'b>) -> Self { Validator(p) }
 
     pub fn validate(&mut self,
-                    needs_val_of: Option<&'a str>,
+                    needs_val_of: ParseResult<'a>,
                     subcmd_name: Option<String>,
                     matcher: &mut ArgMatcher<'a>)
                     -> ClapResult<()> {
         debugln!("Validator::validate;");
         let mut reqs_validated = false;
         try!(self.0.add_defaults(matcher));
-        if let Some(a) = needs_val_of {
+        if let ParseResult::Opt(a) = needs_val_of {
             debugln!("Validator::validate: needs_val_of={:?}", a);
-            if let Some(o) = find_by_name!(self.0, &a, opts, iter) {
-                try!(self.validate_required(matcher));
-                reqs_validated = true;
-                let should_err = if let Some(v) = matcher.0.args.get(&*o.b.name) {
-                    v.vals.is_empty() && !(o.v.min_vals.is_some() && o.v.min_vals.unwrap() == 0)
-                } else {
-                    true
-                };
-                if should_err {
-                    return Err(Error::empty_value(o,
-                                                  &*usage::create_error_usage(self.0,
-                                                                              matcher,
-                                                                              None),
-                                                  self.0.color()));
-                }
+            let o = self.0
+                .opts
+                .iter()
+                .find(|o| o.b.name == a)
+                .expect(INTERNAL_ERROR_MSG);
+            try!(self.validate_required(matcher));
+            reqs_validated = true;
+            let should_err = if let Some(v) = matcher.0.args.get(&*o.b.name) {
+                v.vals.is_empty() && !(o.v.min_vals.is_some() && o.v.min_vals.unwrap() == 0)
+            } else {
+                true
+            };
+            if should_err {
+                return Err(Error::empty_value(o,
+                                              &*usage::create_error_usage(self.0, matcher, None),
+                                              self.0.color()));
             }
         }
 
@@ -166,10 +167,7 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> {
         for name in &self.0.blacklist {
             debugln!("Validator::validate_blacklist:iter: Checking blacklisted name: {}",
                      name);
-            if self.0
-                   .groups
-                   .iter()
-                   .any(|g| &g.name == name) {
+            if self.0.groups.iter().any(|g| &g.name == name) {
                 debugln!("Validator::validate_blacklist:iter: groups contains it...");
                 for n in self.0.arg_names_in_group(name) {
                     debugln!("Validator::validate_blacklist:iter:iter: Checking arg '{}' in group...",
@@ -377,16 +375,18 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> {
         where A: AnyArg<'a, 'b>
     {
         debugln!("Validator::validate_conflicts: a={:?};", a.name());
-        a.blacklist().map(|bl| {
-            bl.iter().any(|conf| {
-                matcher.contains(conf) ||
-                self.0
-                    .groups
-                    .iter()
-                    .find(|g| &g.name == conf)
-                    .map_or(false, |g| g.args.iter().any(|arg| matcher.contains(arg)))
+        a.blacklist()
+            .map(|bl| {
+                bl.iter()
+                    .any(|conf| {
+                        matcher.contains(conf) ||
+                        self.0
+                            .groups
+                            .iter()
+                            .find(|g| &g.name == conf)
+                            .map_or(false, |g| g.args.iter().any(|arg| matcher.contains(arg)))
+                    })
             })
-        })
     }
 
     fn validate_required_unless<A>(&self, a: &A, matcher: &ArgMatcher) -> Option<bool>
@@ -437,7 +437,8 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> {
                 .iter()
                 .fold(String::new(),
                       |acc, s| acc + &format!("\n    {}", c.error(s))[..]);
-        debugln!("Validator::missing_required_error: req_args={:#?}", req_args);
+        debugln!("Validator::missing_required_error: req_args={:#?}",
+                 req_args);
         Err(Error::missing_required_argument(&*req_args,
                                              &*usage::create_error_usage(self.0, matcher, extra),
                                              self.0.color()))
@@ -449,6 +450,7 @@ impl<'a, 'b, 'z> Validator<'a, 'b, 'z> {
     {
         debugln!("Validator::is_missing_required_ok: a={}", a.name());
         self.validate_conflicts(a, matcher).unwrap_or(false) ||
-        self.validate_required_unless(a, matcher).unwrap_or(false)
+        self.validate_required_unless(a, matcher)
+            .unwrap_or(false)
     }
 }
\ No newline at end of file
diff --git a/src/args/arg_builder/option.rs b/src/args/arg_builder/option.rs
index 64dd9c46..6d77df04 100644
--- a/src/args/arg_builder/option.rs
+++ b/src/args/arg_builder/option.rs
@@ -49,7 +49,7 @@ impl<'n, 'e> From<Arg<'n, 'e>> for OptBuilder<'n, 'e> {
 
 impl<'n, 'e> Display for OptBuilder<'n, 'e> {
     fn fmt(&self, f: &mut Formatter) -> Result {
-        debugln!("OptBuilder::fmt");
+        debugln!("OptBuilder::fmt:{}", self.b.name);
         let sep = if self.b.is_set(ArgSettings::RequireEquals) {
             "="
         } else {
diff --git a/src/args/arg_builder/positional.rs b/src/args/arg_builder/positional.rs
index c77976b1..82c2f11e 100644
--- a/src/args/arg_builder/positional.rs
+++ b/src/args/arg_builder/positional.rs
@@ -12,6 +12,7 @@ use vec_map::{self, VecMap};
 // Internal
 use Arg;
 use args::{ArgSettings, Base, Valued, AnyArg, DispOrder};
+use INTERNAL_ERROR_MSG;
 
 #[allow(missing_debug_implementations)]
 #[doc(hidden)]
@@ -59,7 +60,11 @@ impl<'n, 'e> PosBuilder<'n, 'e> {
     }
 
     pub fn multiple_str(&self) -> &str {
-        if self.b.settings.is_set(ArgSettings::Multiple) && self.v.val_names.is_none() {
+        let mult_vals = self.v
+            .val_names
+            .as_ref()
+            .map_or(true, |ref names| names.len() < 2);
+        if self.is_set(ArgSettings::Multiple) && mult_vals {
             "..."
         } else {
             ""
@@ -67,12 +72,20 @@ impl<'n, 'e> PosBuilder<'n, 'e> {
     }
 
     pub fn name_no_brackets(&self) -> Cow<str> {
+        debugln!("PosBuilder::name_no_brackets;");
         if let Some(ref names) = self.v.val_names {
-            Cow::Owned(names.values()
-                .map(|n| format!("<{}>", n))
-                .collect::<Vec<_>>()
-                .join(" "))
+            debugln!("PosBuilder:name_no_brackets: val_names={:#?}", names);
+            if names.len() > 1 {
+                Cow::Owned(names
+                               .values()
+                               .map(|n| format!("<{}>", n))
+                               .collect::<Vec<_>>()
+                               .join(" "))
+            } else {
+                Cow::Borrowed(names.values().next().expect(INTERNAL_ERROR_MSG))
+            }
         } else {
+            debugln!("PosBuilder:name_no_brackets: just name");
             Cow::Borrowed(self.b.name)
         }
     }
@@ -83,7 +96,8 @@ impl<'n, 'e> Display for PosBuilder<'n, 'e> {
         if let Some(ref names) = self.v.val_names {
             try!(write!(f,
                         "{}",
-                        names.values()
+                        names
+                            .values()
                             .map(|n| format!("<{}>", n))
                             .collect::<Vec<_>>()
                             .join(" ")));
@@ -140,9 +154,7 @@ impl<'n, 'e> DispOrder for PosBuilder<'n, 'e> {
 }
 
 impl<'n, 'e> PartialEq for PosBuilder<'n, 'e> {
-    fn eq(&self, other: &PosBuilder<'n, 'e>) -> bool {
-        self.b == other.b
-    }
+    fn eq(&self, other: &PosBuilder<'n, 'e>) -> bool { self.b == other.b }
 }
 
 #[cfg(test)]


Test patch:
diff --git a/tests/delimiters.rs b/tests/delimiters.rs
index 30c998bd..d5b60b9c 100644
--- a/tests/delimiters.rs
+++ b/tests/delimiters.rs
@@ -51,7 +51,7 @@ fn opt_s_eq_no_delim() {
             "-o=val1,val2,val3",
         ]);
 
-    assert!(m.is_ok());
+    assert!(m.is_ok(), "{:?}", m.unwrap_err());
     let m = m.unwrap();
 
     assert!(m.is_present("option"));
@@ -70,7 +70,7 @@ fn opt_s_default_no_delim() {
             "-o", "val1,val2,val3",
         ]);
 
-    assert!(m.is_ok());
+    assert!(m.is_ok(), "{:?}", m.unwrap_err());
     let m = m.unwrap();
 
     assert!(m.is_present("option"));
diff --git a/tests/help.rs b/tests/help.rs
index 0d515007..73b695cc 100644
--- a/tests/help.rs
+++ b/tests/help.rs
@@ -1,3 +1,4 @@
+#[macro_use]
 extern crate clap;
 extern crate regex;
 
@@ -382,6 +383,37 @@ FLAGS:
 OPTIONS:
         --arg <argument>    Pass an argument to the program. [default: default-argument]";
 
+static LAST_ARG_USAGE: &'static str = "flamegraph 0.1
+
+USAGE:
+    flamegraph [FLAGS] [OPTIONS] [BINFILE] [-- <ARGS>...]
+
+FLAGS:
+    -h, --help       Prints help information
+    -V, --version    Prints version information
+    -v, --verbose    Prints out more stuff.
+
+OPTIONS:
+    -f, --frequency <HERTZ>    The sampling frequency. By default, this is 99 Hz.
+    -t, --timeout <SECONDS>    Timeout in seconds. By default, there is no timeout.
+
+ARGS:
+    <BINFILE>    The path of the binary to be profiled. If empty, Cargo.toml is searched for a binary.
+    <ARGS>...       Any arguments you wish to pass to the binary being profiled.";
+
+static LAST_ARG_REQ_MULT: &'static str = "example 1.0
+
+USAGE:
+    example <FIRST>... [--] <SECOND>...
+
+FLAGS:
+    -h, --help       Prints help information
+    -V, --version    Prints version information
+
+ARGS:
+    <FIRST>...     First
+    <SECOND>...    Second";
+
 #[test]
 fn help_short() {
     let m = App::new("test")
@@ -432,6 +464,55 @@ fn help_subcommand() {
     assert!(m.is_err());
     assert_eq!(m.unwrap_err().kind, ErrorKind::HelpDisplayed);
 }
+ 
+#[test]
+fn req_last_arg_usage() {
+    let app = clap_app!(example =>
+        (version: "1.0")
+        (@arg FIRST: ... * "First")
+        (@arg SECOND: ... * +last "Second")
+    );
+    assert!(test::compare_output(app, "example --help", LAST_ARG_REQ_MULT, false));
+}
+
+#[test]
+fn args_with_last_usage() {
+    let app = App::new("flamegraph")
+        .version("0.1")
+        .setting(AppSettings::TrailingVarArg)
+        .arg(Arg::with_name("verbose")
+            .help("Prints out more stuff.")
+            .short("v")
+            .long("verbose")
+            .multiple(true)
+        )
+        .arg(Arg::with_name("timeout")
+            .help("Timeout in seconds. By default, there is no timeout.")
+            .short("t")
+            .long("timeout")
+            .value_name("SECONDS")
+            .takes_value(true)
+        )
+        .arg(Arg::with_name("frequency")
+            .help("The sampling frequency. By default, this is 99 Hz.")
+            .short("f")
+            .long("frequency")
+            .value_name("HERTZ")
+            .takes_value(true)
+        )
+        .arg(Arg::with_name("binary path")
+            .help("The path of the binary to be profiled. If empty, Cargo.toml is searched for a binary.")
+            .takes_value(true)
+            .value_name("BINFILE")
+        )
+        .arg(Arg::with_name("pass through args")
+            .help("Any arguments you wish to pass to the binary being profiled.")
+            .value_name("ARGS")
+            .last(true)
+            .multiple(true)
+        );
+    assert!(test::compare_output(app, "flamegraph --help", LAST_ARG_USAGE, false));
+}
 
 #[test]
 fn subcommand_short_help() {
diff --git a/tests/positionals.rs b/tests/positionals.rs
index 3a4ecac9..5e37668b 100644
--- a/tests/positionals.rs
+++ b/tests/positionals.rs
@@ -15,15 +15,34 @@ fn only_pos_follow() {
     assert_eq!(m.value_of("arg").unwrap(), "-f");
 }
 
+#[test]
+fn issue_946() {
+    let r = App::new("compiletest")
+        .setting(clap::AppSettings::AllowLeadingHyphen)
+        .args_from_usage("--exact    'filters match exactly'")
+        .arg(clap::Arg::with_name("filter")
+            .index(1)
+            .takes_value(true)
+            .help("filters to apply to output"))
+    .get_matches_from_safe(vec!["compiletest", "--exact"]);
+    assert!(r.is_ok(), "{:#?}", r);
+    let matches = r.unwrap();
+
+    assert!(matches.is_present("exact"));
+    assert!(matches.value_of("filter").is_none());
+}
+
 #[test]
 fn positional() {
-    let m = App::new("positional")
+    let r = App::new("positional")
         .args(&[
             Arg::from_usage("-f, --flag 'some flag'"),
             Arg::with_name("positional")
                 .index(1)
             ])
-        .get_matches_from(vec!["", "-f", "test"]);
+        .get_matches_from_safe(vec!["", "-f", "test"]);
+    assert!(r.is_ok(), "{:#?}", r);
+    let m = r.unwrap();
     assert!(m.is_present("positional"));
     assert!(m.is_present("flag"));
     assert_eq!(m.value_of("positional").unwrap(), "test");
@@ -83,26 +102,33 @@ fn lots_o_vals() {
 
 #[test]
 fn positional_multiple() {
-    let m = App::new("positional_multiple")
+    let r = App::new("positional_multiple")
         .args(&[
             Arg::from_usage("-f, --flag 'some flag'"),
             Arg::with_name("positional")
                 .index(1)
                 .multiple(true)
             ])
-        .get_matches_from(vec!["", "-f", "test1", "test2", "test3"]);
+        .get_matches_from_safe(vec!["", "-f", "test1", "test2", "test3"]);
+    assert!(r.is_ok(), "{:#?}", r);
+    let m = r.unwrap();
     assert!(m.is_present("positional"));
     assert!(m.is_present("flag"));
     assert_eq!(&*m.values_of("positional").unwrap().collect::<Vec<_>>(), &["test1", "test2", "test3"]);
+}
 
-    let m = App::new("positional_multiple")
+#[test]
+fn positional_multiple_3() {
+    let r = App::new("positional_multiple")
         .args(&[
             Arg::from_usage("-f, --flag 'some flag'"),
             Arg::with_name("positional")
                 .index(1)
                 .multiple(true)
             ])
-        .get_matches_from(vec!["", "test1", "test2", "test3", "--flag"]);
+        .get_matches_from_safe(vec!["", "test1", "test2", "test3", "--flag"]);
+    assert!(r.is_ok(), "{:#?}", r);
+    let m = r.unwrap();
     assert!(m.is_present("positional"));
     assert!(m.is_present("flag"));
     assert_eq!(&*m.values_of("positional").unwrap().collect::<Vec<_>>(), &["test1", "test2", "test3"]);
@@ -124,14 +150,16 @@ fn positional_multiple_2() {
 
 #[test]
 fn positional_possible_values() {
-    let m = App::new("positional_possible_values")
+    let r = App::new("positional_possible_values")
         .args(&[
             Arg::from_usage("-f, --flag 'some flag'"),
             Arg::with_name("positional")
                 .index(1)
                 .possible_value("test123")
             ])
-        .get_matches_from(vec!["", "-f", "test123"]);
+        .get_matches_from_safe(vec!["", "-f", "test123"]);
+    assert!(r.is_ok(), "{:#?}", r);
+    let m = r.unwrap();
     assert!(m.is_present("positional"));
     assert!(m.is_present("flag"));
     assert_eq!(&*m.values_of("positional").unwrap().collect::<Vec<_>>(), &["test123"]);