fix(help): Optional arg values in brackets

When an Arg uses .min_values(0), that arg's value(s) are effectively
optional. This is conventionaly denoted in help messages by wrapping the
arg's values in square brackets. For example:

    --foo[=value]
    --bar [value]

This kind of argument can be seen in the wild in many git commands; e.g.
git-status(1).

Signed-off-by: Peter Grayson <pete@jpgrayson.net>

src/build/arg/mod.rs

@@ -5013,9 +5013,19 @@ impl<'help> Display for Arg<'help> {
         } else if let Some(s) = self.short {
             write!(f, "-{}", s)?;
         }
+        let mut need_closing_bracket = false;
         if !self.is_positional() && self.is_set(ArgSettings::TakesValue) {
+            let is_optional_val = self.min_vals == Some(0);
             let sep = if self.is_set(ArgSettings::RequireEquals) {
-                "="
+                if is_optional_val {
+                    need_closing_bracket = true;
+                    "[="
+                } else {
+                    "="
+                }
+            } else if is_optional_val {
+                need_closing_bracket = true;
+                " ["
             } else {
                 " "
             };
@@ -5024,6 +5034,9 @@ impl<'help> Display for Arg<'help> {
         if self.is_set(ArgSettings::TakesValue) || self.is_positional() {
             display_arg_val(self, |s, _| write!(f, "{}", s))?;
         }
+        if need_closing_bracket {
+            write!(f, "]")?;
+        }
 
         Ok(())
     }

src/output/help.rs

@@ -272,49 +272,65 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
     /// Writes argument's long command to the wrapped stream.
     fn long(&mut self, arg: &Arg<'help>) -> io::Result<()> {
         debug!("Help::long");
-        if arg.is_positional() {
-            return Ok(());
-        }
-        if arg.is_set(ArgSettings::TakesValue) {
-            if let Some(l) = arg.long {
-                if arg.short.is_some() {
-                    self.none(", ")?;
-                }
-                self.good(&format!("--{}", l))?
-            }
-
-            let sep = if arg.is_set(ArgSettings::RequireEquals) {
-                "="
-            } else {
-                " "
-            };
-            self.none(sep)?;
-        } else if let Some(l) = arg.long {
+        if let Some(long) = arg.long {
             if arg.short.is_some() {
                 self.none(", ")?;
             }
-            self.good(&format!("--{}", l))?;
+            self.good(&format!("--{}", long))?;
         }
         Ok(())
     }
 
     /// Writes argument's possible values to the wrapped stream.
-    fn val(&mut self, arg: &Arg<'help>, next_line_help: bool, longest: usize) -> io::Result<()> {
+    fn val(&mut self, arg: &Arg<'help>) -> io::Result<()> {
         debug!("Help::val: arg={}", arg.name);
+        let mut need_closing_bracket = false;
+        if arg.is_set(ArgSettings::TakesValue) && !arg.is_positional() {
+            let is_optional_val = arg.min_vals == Some(0);
+            let sep = if arg.is_set(ArgSettings::RequireEquals) {
+                if is_optional_val {
+                    need_closing_bracket = true;
+                    "[="
+                } else {
+                    "="
+                }
+            } else if is_optional_val {
+                need_closing_bracket = true;
+                " ["
+            } else {
+                " "
+            };
+            self.none(sep)?;
+        }
+
         if arg.is_set(ArgSettings::TakesValue) || arg.is_positional() {
             display_arg_val(
                 arg,
                 |s, good| if good { self.good(s) } else { self.none(s) },
             )?;
         }
 
-        debug!("Help::val: Has switch...");
+        if need_closing_bracket {
+            self.none("]")?;
+        }
+        Ok(())
+    }
+
+    /// Write alignment padding between arg's switches/values and its about message.
+    fn align_to_about(
+        &mut self,
+        arg: &Arg<'help>,
+        next_line_help: bool,
+        longest: usize,
+    ) -> io::Result<()> {
+        debug!("Help::align_to_about: arg={}", arg.name);
+        debug!("Help::align_to_about: Has switch...");
         if self.use_long {
-            // long help prints messages on the next line so it don't need to align text
-            debug!("Help::val: printing long help so skip alignment");
+            // long help prints messages on the next line so it doesn't need to align text
+            debug!("Help::align_to_about: printing long help so skip alignment");
         } else if !arg.is_positional() {
             debug!("Yes");
-            debug!("Help::val: nlh...{:?}", next_line_help);
+            debug!("Help::align_to_about: nlh...{:?}", next_line_help);
             if !next_line_help {
                 let self_len = display_width(arg.to_string().as_str());
                 // subtract ourself
@@ -439,7 +455,8 @@ impl<'help, 'app, 'parser, 'writer> Help<'help, 'app, 'parser, 'writer> {
     ) -> io::Result<()> {
         self.short(arg)?;
         self.long(arg)?;
-        self.val(arg, next_line_help, longest)?;
+        self.val(arg)?;
+        self.align_to_about(arg, next_line_help, longest)?;
 
         let about = if self.use_long {
             arg.long_help.unwrap_or_else(|| arg.help.unwrap_or(""))

tests/builder/help.rs

@@ -38,6 +38,8 @@ OPTIONS:
         --multvalsmo <one> <two>     Tests multiple values, and mult occs
     -o, --option <opt>...            tests options
     -O, --option3 <option3>          specific vals [possible values: fast, slow]
+        --optvaleq[=<optval>]        Tests optional value, require = sign
+        --optvalnoeq [<optval>]      Tests optional value
     -V, --version                    Print version information
 
 SUBCOMMANDS:

tests/builder/utils.rs

@@ -94,6 +94,15 @@ pub fn complex_app() -> App<'static> {
             arg!(--maxvals3 <maxvals> "Tests 3 max vals")
                 .required(false)
                 .max_values(3),
+            arg!(--optvaleq <optval> "Tests optional value, require = sign")
+                .required(false)
+                .min_values(0)
+                .number_of_values(1)
+                .require_equals(true),
+            arg!(--optvalnoeq <optval> "Tests optional value")
+                .required(false)
+                .min_values(0)
+                .number_of_values(1),
         ])
         .subcommand(
             App::new("subcmd")

tests/derive/options.rs

@@ -199,8 +199,8 @@ fn option_option_type_help() {
         arg: Option<Option<i32>>,
     }
     let help = utils::get_help::<Opt>();
-    assert!(help.contains("--arg <val>"));
-    assert!(!help.contains("--arg <val>..."));
+    assert!(help.contains("--arg [<val>]"));
+    assert!(!help.contains("--arg [<val>]..."));
 }
 
 #[test]