imp(Validators): improves the error messages for validators

Failing value validators now produce messages like the following:

error: Invalid value for '-j <val>': some message about the value

Closes #744

src/app/parser.rs

@@ -42,9 +42,9 @@ pub struct Parser<'a, 'b>
     pub long_list: Vec<&'b str>,
     blacklist: Vec<&'b str>,
     // A list of possible flags
-    pub flags: VecMap<FlagBuilder<'a, 'b>>,
+    pub flags: Vec<FlagBuilder<'a, 'b>>,
     // A list of possible options
-    pub opts: VecMap<OptBuilder<'a, 'b>>,
+    pub opts: Vec<OptBuilder<'a, 'b>>,
     // A list of positional arguments
     pub positionals: VecMap<PosBuilder<'a, 'b>>,
     // A list of subcommands
@@ -66,8 +66,8 @@ pub struct Parser<'a, 'b>
 impl<'a, 'b> Default for Parser<'a, 'b> {
     fn default() -> Self {
         Parser {
-            flags: VecMap::new(),
-            opts: VecMap::new(),
+            flags: vec![],
+            opts: vec![],
             positionals: VecMap::new(),
             subcommands: vec![],
             help_short: None,
@@ -137,8 +137,8 @@ impl<'a, 'b> Parser<'a, 'b>
 
     // actually adds the arguments
     pub fn add_arg(&mut self, a: &Arg<'a, 'b>) {
-        debug_assert!(!(self.flags.values().any(|f| &f.b.name == &a.name) ||
-                        self.opts.values().any(|o| o.b.name == a.name) ||
+        debug_assert!(!(self.flags.iter().any(|f| &f.b.name == &a.name) ||
+                        self.opts.iter().any(|o| o.b.name == a.name) ||
                         self.positionals.values().any(|p| p.b.name == a.name)),
                       format!("Non-unique argument name: {} is already in use", a.name));
         if let Some(ref grps) = a.groups {
@@ -271,16 +271,16 @@ impl<'a, 'b> Parser<'a, 'b>
     pub fn derive_display_order(&mut self) {
         if self.settings.is_set(AppSettings::DeriveDisplayOrder) {
             let unified = self.settings.is_set(AppSettings::UnifiedHelpMessage);
-            for (i, (_, o)) in self.opts
+            for (i, o) in self.opts
                 .iter_mut()
                 .enumerate()
-                .filter(|&(_, (_, ref o))| o.s.disp_ord == 999) {
+                .filter(|&(_, ref o)| o.s.disp_ord == 999) {
                 o.s.disp_ord = if unified { o.s.unified_ord } else { i };
             }
-            for (i, (_, f)) in self.flags
+            for (i, f) in self.flags
                 .iter_mut()
                 .enumerate()
-                .filter(|&(_, (_, ref f))| f.s.disp_ord == 999) {
+                .filter(|&(_, ref f)| f.s.disp_ord == 999) {
                 f.s.disp_ord = if unified { f.s.unified_ord } else { i };
             }
             for (i, sc) in &mut self.subcommands
@@ -678,7 +678,7 @@ impl<'a, 'b> Parser<'a, 'b>
     }
 
     #[inline]
-    fn check_is_new_arg(&mut self, arg_os: &OsStr, needs_val_of: Option<ArgId>) -> bool {
+    fn check_is_new_arg(&mut self, arg_os: &OsStr, needs_val_of: Option<&'a str>) -> bool {
         debugln!("fn=check_is_new_arg;nvo={:?}", needs_val_of);
         let app_wide_settings = if self.is_set(AppSettings::AllowLeadingHyphen) {
             true
@@ -1530,7 +1530,7 @@ impl<'a, 'b> Parser<'a, 'b>
         }
         if let Some(vtor) = arg.validator() {
             if let Err(e) = vtor(val.to_string_lossy().into_owned()) {
-                return Err(Error::value_validation(e, self.color()));
+                return Err(Error::value_validation(Some(arg), e, self.color()));
             }
         }
         if matcher.needs_more_vals(arg) {

src/args/arg.rs

@@ -1870,7 +1870,9 @@ impl<'a, 'b> Arg<'a, 'b> {
     /// message displayed to the user.
     ///
     /// **NOTE:** The error message does *not* need to contain the `error:` portion, only the
-    /// message.
+    /// message as all errors will appear as
+    /// `error: Invalid value for '<arg>': <YOUR MESSAGE>` where `<arg>` is replaced by the actual
+    /// arg, and `<YOUR MESSAGE>` is the `String` you return as the error.
     ///
     /// **NOTE:** There is a small performance hit for using validators, as they are implemented
     /// with [`Rc`] pointers. And the value to be checked will be allocated an extra time in order

src/errors.rs

@@ -8,7 +8,7 @@ use std::process;
 use std::result::Result as StdResult;
 
 // Internal
-use args::any_arg::AnyArg;
+use args::{FlagBuilder, AnyArg};
 use fmt;
 use suggestions;
 
@@ -701,21 +701,31 @@ impl Error {
     }
 
     #[doc(hidden)]
-    pub fn value_validation(err: String, color: fmt::ColorWhen) -> Self {
+    pub fn value_validation<'a, 'b, A>(arg: Option<&A>, err: String, color: fmt::ColorWhen) -> Self
+        where A: AnyArg<'a, 'b> + Display
+    {
         let c = fmt::Colorizer {
             use_stderr: true,
             when: color,
         };
         Error {
-            message: format!("{} {}", c.error("error:"), err),
+            message: format!("{} Invalid value{}: {}",
+                             c.error("error:"),
+                             if let Some(a) = arg {
+                                 format!(" for '{}'", c.warning(a.to_string()))
+                             } else {
+                                 "".to_string()
+                             },
+                             err),
             kind: ErrorKind::ValueValidation,
             info: None,
         }
     }
 
     #[doc(hidden)]
     pub fn value_validation_auto(err: String) -> Self {
-        Error::value_validation(err, fmt::ColorWhen::Auto)
+        let n: Option<&FlagBuilder> = None; 
+        Error::value_validation(n, err, fmt::ColorWhen::Auto)
     }
 
     #[doc(hidden)]