perf: avoid retrieving possible_values unless they're used

In some sophisticated situations, these may be expensive to calculate. One example might be a '--branch' option accepting any single Git branch that exists on the remote -- in such a case, the remote would need to be queried for all possible_values. The cost is ultimately unavoidable at runtime since this validation has to happen eventually, but there's no need to pay it when generating help text if `is_hide_possible_values_set`. To keep '-h' fast, avoid collecting `possible_values` during '-h' unless we're actually going to use the values in display.
clap-rs · Dec 24, 2023 · 402fa91 · 402fa91
1 parent d092896
commit 402fa91
Showing 1 changed file with 48 additions and 49 deletions.
diff --git a/clap_builder/src/output/help_template.rs b/clap_builder/src/output/help_template.rs
@@ -684,57 +684,56 @@ impl<'cmd, 'writer> HelpTemplate<'cmd, 'writer> {
         let help_is_empty = help.is_empty();
         self.writer.push_styled(&help);
         if let Some(arg) = arg {
-            const DASH_SPACE: usize = "- ".len();
-            let possible_vals = arg.get_possible_values();
-            if !possible_vals.is_empty()
-                && !arg.is_hide_possible_values_set()
-                && self.use_long_pv(arg)
-            {
-                debug!("HelpTemplate::help: Found possible vals...{possible_vals:?}");
-                let longest = possible_vals
-                    .iter()
-                    .filter(|f| !f.is_hide_set())
-                    .map(|f| display_width(f.get_name()))
-                    .max()
-                    .expect("Only called with possible value");
-
-                let spaces = spaces + TAB_WIDTH - DASH_SPACE;
-                let trailing_indent = spaces + DASH_SPACE;
-                let trailing_indent = self.get_spaces(trailing_indent);
-
-                if !help_is_empty {
-                    let _ = write!(self.writer, "\n\n{:spaces$}", "");
-                }
-                self.writer.push_str("Possible values:");
-                for pv in possible_vals.iter().filter(|pv| !pv.is_hide_set()) {
-                    let name = pv.get_name();
-
-                    let mut descr = StyledStr::new();
-                    let _ = write!(
-                        &mut descr,
-                        "{}{name}{}",
-                        literal.render(),
-                        literal.render_reset()
-                    );
-                    if let Some(help) = pv.get_help() {
-                        debug!("HelpTemplate::help: Possible Value help");
-                        // To align help messages
-                        let padding = longest - display_width(name);
-                        let _ = write!(&mut descr, ": {:padding$}", "");
-                        descr.push_styled(help);
+            if !arg.is_hide_possible_values_set() && self.use_long_pv(arg) {
+                const DASH_SPACE: usize = "- ".len();
+                let possible_vals = arg.get_possible_values();
+                if !possible_vals.is_empty() {
+                    debug!("HelpTemplate::help: Found possible vals...{possible_vals:?}");
+                    let longest = possible_vals
+                        .iter()
+                        .filter(|f| !f.is_hide_set())
+                        .map(|f| display_width(f.get_name()))
+                        .max()
+                        .expect("Only called with possible value");
+
+                    let spaces = spaces + TAB_WIDTH - DASH_SPACE;
+                    let trailing_indent = spaces + DASH_SPACE;
+                    let trailing_indent = self.get_spaces(trailing_indent);
+
+                    if !help_is_empty {
+                        let _ = write!(self.writer, "\n\n{:spaces$}", "");
                     }
+                    self.writer.push_str("Possible values:");
+                    for pv in possible_vals.iter().filter(|pv| !pv.is_hide_set()) {
+                        let name = pv.get_name();
 
-                    let avail_chars = if self.term_w > trailing_indent.len() {
-                        self.term_w - trailing_indent.len()
-                    } else {
-                        usize::MAX
-                    };
-                    descr.replace_newline_var();
-                    descr.wrap(avail_chars);
-                    descr.indent("", &trailing_indent);
-
-                    let _ = write!(self.writer, "\n{:spaces$}- ", "",);
-                    self.writer.push_styled(&descr);
+                        let mut descr = StyledStr::new();
+                        let _ = write!(
+                            &mut descr,
+                            "{}{name}{}",
+                            literal.render(),
+                            literal.render_reset()
+                        );
+                        if let Some(help) = pv.get_help() {
+                            debug!("HelpTemplate::help: Possible Value help");
+                            // To align help messages
+                            let padding = longest - display_width(name);
+                            let _ = write!(&mut descr, ": {:padding$}", "");
+                            descr.push_styled(help);
+                        }
+
+                        let avail_chars = if self.term_w > trailing_indent.len() {
+                            self.term_w - trailing_indent.len()
+                        } else {
+                            usize::MAX
+                        };
+                        descr.replace_newline_var();
+                        descr.wrap(avail_chars);
+                        descr.indent("", &trailing_indent);
+
+                        let _ = write!(self.writer, "\n{:spaces$}- ", "",);
+                        self.writer.push_styled(&descr);
+                    }
                 }
             }
         }