diff --git a/clap_builder/src/output/help_template.rs b/clap_builder/src/output/help_template.rs index 9fe7211b306..dff15ff1279 100644 --- 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 !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(); - 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); + } - 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); } - - 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); } } } @@ -844,17 +843,19 @@ impl<'cmd, 'writer> HelpTemplate<'cmd, 'writer> { spec_vals.push(format!("[short aliases: {als}]")); } - let possible_vals = a.get_possible_values(); - 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:?}"); + if !a.is_hide_possible_values_set() && !self.use_long_pv(a) { + let possible_vals = a.get_possible_values(); + if !possible_vals.is_empty() { + debug!("HelpTemplate::spec_vals: Found possible vals...{possible_vals:?}"); - let pvs = possible_vals - .iter() - .filter_map(PossibleValue::get_visible_quoted_name) - .collect::>() - .join(", "); + let pvs = possible_vals + .iter() + .filter_map(PossibleValue::get_visible_quoted_name) + .collect::>() + .join(", "); - spec_vals.push(format!("[possible values: {pvs}]")); + spec_vals.push(format!("[possible values: {pvs}]")); + } } let connector = if self.use_long { "\n" } else { " " }; spec_vals.join(connector) diff --git a/clap_mangen/src/render.rs b/clap_mangen/src/render.rs index 95f2740938f..ef4981d80d6 100644 --- a/clap_mangen/src/render.rs +++ b/clap_mangen/src/render.rs @@ -305,11 +305,15 @@ fn option_default_values(opt: &clap::Arg) -> Option { } fn get_possible_values(arg: &clap::Arg) -> Option<(Vec, bool)> { + if arg.is_hide_possible_values_set() { + return None; + } + let possibles = &arg.get_possible_values(); let possibles: Vec<&clap::builder::PossibleValue> = possibles.iter().filter(|pos| !pos.is_hide_set()).collect(); - if !(possibles.is_empty() || arg.is_hide_possible_values_set()) { + if !possibles.is_empty() { return Some(format_possible_values(&possibles)); } None diff --git a/tests/builder/possible_values.rs b/tests/builder/possible_values.rs index b39517279be..de66cf8650f 100644 --- a/tests/builder/possible_values.rs +++ b/tests/builder/possible_values.rs @@ -472,3 +472,145 @@ fn ignore_case_multiple_fail() { assert!(m.is_err()); assert_eq!(m.unwrap_err().kind(), ErrorKind::InvalidValue); } + +#[cfg(feature = "string")] +mod expensive { + use std::sync::{Arc, Mutex}; + + use clap::{Arg, Command}; + use clap_builder::builder::{PossibleValue, PossibleValuesParser, TypedValueParser}; + + #[cfg(feature = "error-context")] + use super::utils; + + #[derive(Clone)] + struct ExpensiveValues { + iterated: Arc>, + } + + impl ExpensiveValues { + pub fn new() -> Self { + ExpensiveValues { + iterated: Arc::new(Mutex::new(false)), + } + } + } + + impl IntoIterator for ExpensiveValues { + type Item = String; + + type IntoIter = ExpensiveValuesIntoIterator; + + fn into_iter(self) -> Self::IntoIter { + ExpensiveValuesIntoIterator { me: self, index: 0 } + } + } + + struct ExpensiveValuesIntoIterator { + me: ExpensiveValues, + index: usize, + } + + impl Iterator for ExpensiveValuesIntoIterator { + type Item = String; + fn next(&mut self) -> Option { + let mut guard = self + .me + .iterated + .lock() + .expect("not working across multiple threads"); + + *guard = true; + self.index += 1; + + if self.index < 3 { + Some(format!("expensive-value-{}", self.index)) + } else { + None + } + } + } + + impl TypedValueParser for ExpensiveValues { + type Value = String; + + fn parse_ref( + &self, + _cmd: &clap_builder::Command, + _arg: Option<&clap_builder::Arg>, + _value: &std::ffi::OsStr, + ) -> Result { + todo!() + } + + fn possible_values(&self) -> Option + '_>> { + Some(Box::new(self.clone().into_iter().map(PossibleValue::from))) + } + } + + #[test] + fn no_iterate_when_hidden() { + static PV_EXPECTED: &str = "\ +Usage: clap-test [some-cheap-option] [some-expensive-option] + +Arguments: + [some-cheap-option] cheap [possible values: some, cheap, values] + [some-expensive-option] expensive + +Options: + -h, --help Print help +"; + let expensive = ExpensiveValues::new(); + utils::assert_output( + Command::new("test") + .arg( + Arg::new("some-cheap-option") + .help("cheap") + .value_parser(PossibleValuesParser::new(["some", "cheap", "values"])), + ) + .arg( + Arg::new("some-expensive-option") + .help("expensive") + .hide_possible_values(true) + .value_parser(expensive.clone()), + ), + "clap-test -h", + PV_EXPECTED, + false, + ); + assert_eq!(*expensive.iterated.lock().unwrap(), false); + } + + #[test] + fn iterate_when_displayed() { + static PV_EXPECTED: &str = "\ +Usage: clap-test [some-cheap-option] [some-expensive-option] + +Arguments: + [some-cheap-option] cheap [possible values: some, cheap, values] + [some-expensive-option] expensive [possible values: expensive-value-1, expensive-value-2] + +Options: + -h, --help Print help +"; + let expensive = ExpensiveValues::new(); + utils::assert_output( + Command::new("test") + .arg( + Arg::new("some-cheap-option") + .help("cheap") + .value_parser(PossibleValuesParser::new(["some", "cheap", "values"])), + ) + .arg( + Arg::new("some-expensive-option") + .help("expensive") + .hide_possible_values(false) + .value_parser(expensive.clone()), + ), + "clap-test -h", + PV_EXPECTED, + false, + ); + assert_eq!(*expensive.iterated.lock().unwrap(), true); + } +}