feat(help): Show PossibleValue help in --help (#3503)

`-h` (short help) still shows the same. This gates it behind an `unstable-v4` feature flag to avoid disrupting users who set the help without knowing where all it shows up (particularly derive users where `ArgEnum` is automatically extracting the help). Fixes #3312
clap-rs · Mar 2, 2022 · 33949ce · 33949ce
1 parent 63fa59a
commit 33949ce
Show file tree

Hide file tree

Showing 13 changed files with 340 additions and 40 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -28,7 +28,7 @@ jobs:
     name: Test
     strategy:
       matrix:
-        build: [linux, windows, mac, minimal, default]
+        build: [linux, windows, mac, minimal, default, next]
         include:
         - build: linux
           os: ubuntu-latest
@@ -50,6 +50,10 @@ jobs:
           os: ubuntu-latest
           rust: "stable"
           features: "default"
+        - build: next
+          os: ubuntu-latest
+          rust: "stable"
+          features: "next"
     continue-on-error: ${{ matrix.rust != 'stable' }}
     runs-on: ${{ matrix.os }}
     steps:

diff --git a/.github/workflows/rust-next.yml b/.github/workflows/rust-next.yml
@@ -7,7 +7,7 @@ jobs:
     name: Test
     strategy:
       matrix:
-        build: [stable, linux, windows, mac, nightly, minimal, default]
+        build: [stable, linux, windows, mac, nightly, minimal, default, next]
         include:
         - build: stable
           os: ubuntu-latest
@@ -37,6 +37,10 @@ jobs:
           os: ubuntu-latest
           rust: "stable"
           features: "default"
+        - build: next
+          os: ubuntu-latest
+          rust: "stable"
+          features: "next"
     continue-on-error: ${{ matrix.rust != 'stable' }}
     runs-on: ${{ matrix.os }}
     steps:

diff --git a/Cargo.toml b/Cargo.toml
@@ -81,6 +81,7 @@ unicode = ["textwrap/unicode-width", "unicase"]  # Support for unicode character
 unstable-replace = []
 unstable-multicall = []
 unstable-grouped = []
+unstable-v4 = []
 
 [lib]
 bench = false

diff --git a/Makefile b/Makefile
@@ -17,6 +17,7 @@ _FEATURES_minimal = --no-default-features --features "std"
 _FEATURES_default =
 _FEATURES_wasm = --features "derive cargo env unicode yaml regex unstable-replace unstable-multicall unstable-grouped"
 _FEATURES_full = --features "derive cargo env unicode yaml regex unstable-replace unstable-multicall unstable-grouped wrap_help"
+_FEATURES_next = ${_FEATURES_full} --features unstable-v4
 _FEATURES_debug = ${_FEATURES_full} --features debug
 _FEATURES_release = ${_FEATURES_full} --release
 

diff --git a/README.md b/README.md
@@ -153,6 +153,7 @@ Why use the procedural [Builder API](https://github.com/clap-rs/clap/blob/v3.1.3
 * **unstable-replace**: Enable [`Command::replace`](https://github.com/clap-rs/clap/issues/2836)
 * **unstable-multicall**: Enable [`Command::multicall`](https://github.com/clap-rs/clap/issues/2861)
 * **unstable-grouped**: Enable [`ArgMatches::grouped_values_of`](https://github.com/clap-rs/clap/issues/2924)
+* **unstable-v4**: Preview features which will be stable on the v4.0 release
 
 ## Sponsors
 

diff --git a/clap_complete/src/shells/bash.rs b/clap_complete/src/shells/bash.rs
@@ -178,7 +178,8 @@ fn vals_for(o: &Arg) -> String {
         format!(
             "$(compgen -W \"{}\" -- \"${{cur}}\")",
             vals.iter()
-                .filter_map(PossibleValue::get_visible_name)
+                .filter(|pv| pv.is_hide_set())
+                .map(PossibleValue::get_name)
                 .collect::<Vec<_>>()
                 .join(" ")
         )

diff --git a/clap_complete/src/shells/zsh.rs b/clap_complete/src/shells/zsh.rs
@@ -387,7 +387,8 @@ fn value_completion(arg: &Arg) -> Option<String> {
                 "({})",
                 values
                     .iter()
-                    .filter_map(PossibleValue::get_visible_name)
+                    .filter(|pv| !pv.is_hide_set())
+                    .map(PossibleValue::get_name)
                     .collect::<Vec<_>>()
                     .join(" ")
             ))

diff --git a/src/build/possible_value.rs b/src/build/possible_value.rs
@@ -1,4 +1,4 @@
-use std::iter;
+use std::{borrow::Cow, iter};
 
 use crate::util::eq_ignore_case;
 
@@ -148,6 +148,18 @@ impl<'help> PossibleValue<'help> {
         self.help
     }
 
+    /// Get the help specified for this argument, if any and the argument
+    /// value is not hidden
+    #[inline]
+    #[cfg(feature = "unstable-v4")]
+    pub(crate) fn get_visible_help(&self) -> Option<&'help str> {
+        if !self.hide {
+            self.help
+        } else {
+            None
+        }
+    }
+
     /// Deprecated, replaced with [`PossibleValue::is_hide_set`]
     #[inline]
     #[deprecated(since = "3.1.0", note = "Replaced with `PossibleValue::is_hide_set`")]
@@ -161,7 +173,16 @@ impl<'help> PossibleValue<'help> {
         self.hide
     }
 
+    /// Report if PossibleValue is not hidden and has a help message
+    pub(crate) fn should_show_help(&self) -> bool {
+        !self.hide && self.help.is_some()
+    }
+
     /// Get the name if argument value is not hidden, `None` otherwise
+    #[deprecated(
+        since = "3.1.4",
+        note = "Use `PossibleValue::is_hide_set` and `PossibleValue::get_name`"
+    )]
     pub fn get_visible_name(&self) -> Option<&'help str> {
         if self.hide {
             None
@@ -170,6 +191,20 @@ impl<'help> PossibleValue<'help> {
         }
     }
 
+    /// Get the name if argument value is not hidden, `None` otherwise,
+    /// but wrapped in quotes if it contains whitespace
+    pub(crate) fn get_visible_quoted_name(&self) -> Option<Cow<'help, str>> {
+        if !self.hide {
+            Some(if self.name.contains(char::is_whitespace) {
+                format!("{:?}", self.name).into()
+            } else {
+                self.name.into()
+            })
+        } else {
+            None
+        }
+    }
+
     /// Returns all valid values of the argument value.
     ///
     /// Namely the name and all aliases.

diff --git a/src/output/help.rs b/src/output/help.rs
@@ -11,6 +11,7 @@ use std::{
 use crate::{
     build::{display_arg_val, Arg, Command},
     output::{fmt::Colorizer, Usage},
+    PossibleValue,
 };
 
 // Third party
@@ -385,7 +386,7 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> {
     /// Writes argument's help to the wrapped stream.
     fn help(
         &mut self,
-        is_not_positional: bool,
+        arg: Option<&Arg<'help>>,
         about: &str,
         spec_vals: &str,
         next_line_help: bool,
@@ -401,7 +402,7 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> {
             longest + 12
         };
 
-        let too_long = spaces + display_width(about) + display_width(spec_vals) >= self.term_w;
+        let too_long = spaces + display_width(&help) >= self.term_w;
 
         // Is help on next line, if so then indent
         if next_line_help {
@@ -423,17 +424,100 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> {
         if let Some(part) = help.lines().next() {
             self.none(part)?;
         }
+
+        // indent of help
+        let spaces = if next_line_help {
+            TAB_WIDTH * 3
+        } else if let Some(true) = arg.map(|a| a.is_positional()) {
+            longest + TAB_WIDTH * 2
+        } else {
+            longest + TAB_WIDTH * 3
+        };
+
         for part in help.lines().skip(1) {
             self.none("\n")?;
-            if next_line_help {
-                self.none(format!("{}{}{}", TAB, TAB, TAB))?;
-            } else if is_not_positional {
-                self.spaces(longest + 12)?;
-            } else {
-                self.spaces(longest + 8)?;
-            }
+            self.spaces(spaces)?;
             self.none(part)?;
         }
+
+        #[cfg(feature = "unstable-v4")]
+        if let Some(arg) = arg {
+            const DASH_SPACE: usize = "- ".len();
+            const COLON_SPACE: usize = ": ".len();
+            if self.use_long
+                && !arg.is_hide_possible_values_set()
+                && arg
+                    .possible_vals
+                    .iter()
+                    .any(PossibleValue::should_show_help)
+            {
+                debug!("Help::help: Found possible vals...{:?}", arg.possible_vals);
+                if !help.is_empty() {
+                    self.none("\n\n")?;
+                    self.spaces(spaces)?;
+                }
+                self.none("Possible values:")?;
+                let longest = arg
+                    .possible_vals
+                    .iter()
+                    .filter_map(|f| f.get_visible_quoted_name().map(|name| display_width(&name)))
+                    .max()
+                    .expect("Only called with possible value");
+                let help_longest = arg
+                    .possible_vals
+                    .iter()
+                    .filter_map(|f| f.get_visible_help().map(display_width))
+                    .max()
+                    .expect("Only called with possible value with help");
+                // should new line
+                let taken = longest + spaces + DASH_SPACE;
+
+                let possible_value_new_line =
+                    self.term_w >= taken && self.term_w < taken + COLON_SPACE + help_longest;
+
+                let spaces = spaces + TAB_WIDTH - DASH_SPACE;
+                let spaces_help = if possible_value_new_line {
+                    spaces + DASH_SPACE
+                } else {
+                    spaces + longest + DASH_SPACE + COLON_SPACE
+                };
+
+                for pv in arg.possible_vals.iter().filter(|pv| !pv.is_hide_set()) {
+                    self.none("\n")?;
+                    self.spaces(spaces)?;
+                    self.none("- ")?;
+                    self.good(pv.get_name())?;
+                    if let Some(help) = pv.get_help() {
+                        debug!("Help::help: Possible Value help");
+
+                        if possible_value_new_line {
+                            self.none(":\n")?;
+                            self.spaces(spaces_help)?;
+                        } else {
+                            self.none(": ")?;
+                            // To align help messages
+                            self.spaces(longest - display_width(pv.get_name()))?;
+                        }
+
+                        let avail_chars = if self.term_w > spaces_help {
+                            self.term_w - spaces_help
+                        } else {
+                            usize::MAX
+                        };
+
+                        let help = text_wrapper(help, avail_chars);
+                        let mut help = help.lines();
+
+                        self.none(help.next().unwrap_or_default())?;
+                        for part in help {
+                            self.none("\n")?;
+                            self.spaces(spaces_help)?;
+                            self.none(part)?;
+                        }
+                    }
+                }
+            }
+        }
         Ok(())
     }
 
@@ -456,13 +540,7 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> {
             arg.help.or(arg.long_help).unwrap_or("")
         };
 
-        self.help(
-            !arg.is_positional(),
-            about,
-            spec_vals,
-            next_line_help,
-            longest,
-        )?;
+        self.help(Some(arg), about, spec_vals, next_line_help, longest)?;
         Ok(())
     }
 
@@ -572,7 +650,12 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> {
             }
         }
 
-        if !a.is_hide_possible_values_set() && !a.possible_vals.is_empty() {
+        if !(a.is_hide_possible_values_set()
+            || a.possible_vals.is_empty()
+            || cfg!(feature = "unstable-v4")
+                && self.use_long
+                && a.possible_vals.iter().any(PossibleValue::should_show_help))
+        {
             debug!(
                 "Help::spec_vals: Found possible vals...{:?}",
                 a.possible_vals
@@ -581,15 +664,7 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> {
             let pvs = a
                 .possible_vals
                 .iter()
-                .filter_map(|value| {
-                    if value.is_hide_set() {
-                        None
-                    } else if value.get_name().contains(char::is_whitespace) {
-                        Some(format!("{:?}", value.get_name()))
-                    } else {
-                        Some(value.get_name().to_string())
-                    }
-                })
+                .filter_map(PossibleValue::get_visible_quoted_name)
                 .collect::<Vec<_>>()
                 .join(", ");
 
@@ -670,7 +745,7 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> {
             .unwrap_or("");
 
         self.subcmd(sc_str, next_line_help, longest)?;
-        self.help(false, about, spec_vals, next_line_help, longest)
+        self.help(None, about, spec_vals, next_line_help, longest)
     }
 
     fn sc_spec_vals(&self, a: &Command) -> String {
@@ -1011,6 +1086,7 @@ pub(crate) fn dimensions() -> Option<(usize, usize)> {
 }
 
 const TAB: &str = "    ";
+const TAB_WIDTH: usize = 4;
 
 pub(crate) enum HelpWriter<'writer> {
     Normal(&'writer mut dyn Write),

diff --git a/src/parse/parser.rs b/src/parse/parser.rs
@@ -8,7 +8,6 @@ use std::{
 use os_str_bytes::RawOsStr;
 
 // Internal
-use crate::build::AppSettings as AS;
 use crate::build::{Arg, Command};
 use crate::error::Error as ClapError;
 use crate::error::Result as ClapResult;
@@ -18,6 +17,7 @@ use crate::parse::features::suggestions;
 use crate::parse::{ArgMatcher, SubCommand};
 use crate::parse::{Validator, ValueSource};
 use crate::util::{color::ColorChoice, Id};
+use crate::{build::AppSettings as AS, PossibleValue};
 use crate::{INTERNAL_ERROR_MSG, INVALID_UTF8};
 
 pub(crate) struct Parser<'help, 'cmd> {
@@ -786,7 +786,11 @@ impl<'help, 'cmd> Parser<'help, 'cmd> {
         // specified by the user is sent through. If hide_short_help is not included,
         // then items specified with hidden_short_help will also be hidden.
         let should_long = |v: &Arg| {
-            v.long_help.is_some() || v.is_hide_long_help_set() || v.is_hide_short_help_set()
+            v.long_help.is_some()
+                || v.is_hide_long_help_set()
+                || v.is_hide_short_help_set()
+                || cfg!(feature = "unstable-v4")
+                    && v.possible_vals.iter().any(PossibleValue::should_show_help)
         };
 
         // Subcommands aren't checked because we prefer short help for them, deferring to