Merge pull request #4885 from epage/possible

refactor(help): Clean up long possible value handling
clap-rs · May 4, 2023 · ba9cad6 · ba9cad6
2 parents 47c3b6f + 1351c56
commit ba9cad6
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 49 deletions.
diff --git a/clap_builder/src/builder/possible_value.rs b/clap_builder/src/builder/possible_value.rs
@@ -162,18 +162,6 @@ impl PossibleValue {
         self.help.as_ref()
     }
 
-    /// Get the help specified for this argument, if any and the argument
-    /// value is not hidden
-    #[inline]
-    #[cfg(feature = "help")]
-    pub(crate) fn get_visible_help(&self) -> Option<&StyledStr> {
-        if !self.hide {
-            self.get_help()
-        } else {
-            None
-        }
-    }
-
     /// Report if [`PossibleValue::hide`] is set
     #[inline]
     pub fn is_hide_set(&self) -> bool {

diff --git a/clap_builder/src/output/help_template.rs b/clap_builder/src/output/help_template.rs
@@ -654,38 +654,24 @@ impl<'cmd, 'writer> HelpTemplate<'cmd, 'writer> {
         self.writer.push_styled(&help);
         if let Some(arg) = arg {
             const DASH_SPACE: usize = "- ".len();
-            const COLON_SPACE: usize = ": ".len();
             let possible_vals = arg.get_possible_values();
-            if self.use_long
+            if !possible_vals.is_empty()
                 && !arg.is_hide_possible_values_set()
-                && possible_vals.iter().any(PossibleValue::should_show_help)
+                && self.use_long_pv(arg)
             {
                 debug!(
                     "HelpTemplate::help: Found possible vals...{:?}",
                     possible_vals
                 );
                 let longest = possible_vals
                     .iter()
-                    .filter_map(|f| f.get_visible_quoted_name().map(|name| display_width(&name)))
+                    .filter(|f| !f.is_hide_set())
+                    .map(|f| display_width(f.get_name()))
                     .max()
                     .expect("Only called with possible value");
-                let help_longest = possible_vals
-                    .iter()
-                    .filter_map(|f| f.get_visible_help().map(|h| h.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 trailing_indent = if possible_value_new_line {
-                    spaces + DASH_SPACE
-                } else {
-                    spaces + longest + DASH_SPACE + COLON_SPACE
-                };
+                let trailing_indent = spaces + DASH_SPACE;
                 let trailing_indent = self.get_spaces(trailing_indent);
 
                 if !help_is_empty {
@@ -704,14 +690,9 @@ impl<'cmd, 'writer> HelpTemplate<'cmd, 'writer> {
                     if let Some(help) = pv.get_help() {
                         debug!("HelpTemplate::help: Possible Value help");
 
-                        if possible_value_new_line {
-                            let padding = trailing_indent.len();
-                            let _ = write!(self.writer, ":\n{:padding$}", "");
-                        } else {
-                            // To align help messages
-                            let padding = longest - display_width(pv.get_name());
-                            let _ = write!(self.writer, ": {:padding$}", "");
-                        }
+                        // To align help messages
+                        let padding = longest - display_width(pv.get_name());
+                        let _ = write!(self.writer, ": {:padding$}", "");
 
                         let avail_chars = if self.term_w > trailing_indent.len() {
                             self.term_w - trailing_indent.len()
@@ -835,10 +816,7 @@ impl<'cmd, 'writer> HelpTemplate<'cmd, 'writer> {
         }
 
         let possible_vals = a.get_possible_values();
-        if !(a.is_hide_possible_values_set()
-            || possible_vals.is_empty()
-            || self.use_long && possible_vals.iter().any(PossibleValue::should_show_help))
-        {
+        if !possible_vals.is_empty() && !a.is_hide_possible_values_set() && !self.use_long_pv(a) {
             debug!(
                 "HelpTemplate::spec_vals: Found possible vals...{:?}",
                 possible_vals
@@ -864,6 +842,14 @@ impl<'cmd, 'writer> HelpTemplate<'cmd, 'writer> {
         use std::fmt::Write as _;
         let _ = write!(self.writer, "{:amount$}", "");
     }
+
+    fn use_long_pv(&self, arg: &Arg) -> bool {
+        self.use_long
+            && arg
+                .get_possible_values()
+                .iter()
+                .any(PossibleValue::should_show_help)
+    }
 }
 
 /// Subcommand handling

diff --git a/tests/builder/help.rs b/tests/builder/help.rs
@@ -449,15 +449,12 @@ Usage: test [OPTIONS]
 Options:
       --possible-values <possible_values>
           Possible values:
-          - short_name:
-            Long enough help message, barely warrant wrapping
-          - second:
-            Short help gets handled the same
+          - short_name: Long enough help message, barely warrant wrapping
+          - second:     Short help gets handled the same
 
       --possible-values-with-new-line <possible_values_with_new_line>
           Possible values:
-          - long enough name to trigger new line:
-            Really long enough help message to clearly warrant
+          - long enough name to trigger new line: Really long enough help message to clearly warrant
             wrapping believe me
           - second