From fe83170ac407f9d7bf3895e8a095310eb75524fc Mon Sep 17 00:00:00 2001 From: Roland Fredenhagen Date: Wed, 16 Feb 2022 16:44:00 +0100 Subject: [PATCH 1/8] chore: Add unstable-v4 feature --- Cargo.toml | 1 + Makefile | 1 + README.md | 1 + 3 files changed, 3 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 9faaee57035..d270b10ed54 100644 --- 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 index e001b23f3fd..de7391502cf 100644 --- a/Makefile +++ b/Makefile @@ -15,6 +15,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 index 6f620359e94..97644a9ff85 100644 --- 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.1 * **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: Show help messages for possible values in long help [#3312](https://github.com/clap-rs/clap/issues/3312) ## Sponsors From 9839cf8707f137b2248d4d1eaf64591a9a8d4577 Mon Sep 17 00:00:00 2001 From: Roland Fredenhagen Date: Wed, 23 Feb 2022 10:14:24 +0100 Subject: [PATCH 2/8] feat(help): Show help for possible values --- src/build/possible_value.rs | 5 ++ src/output/help.rs | 116 ++++++++++++++++++++++++++----- src/parse/parser.rs | 8 ++- tests/builder/help.rs | 131 ++++++++++++++++++++++++++++++++++++ 4 files changed, 240 insertions(+), 20 deletions(-) diff --git a/src/build/possible_value.rs b/src/build/possible_value.rs index bf611aa9523..6c205dc4d8a 100644 --- a/src/build/possible_value.rs +++ b/src/build/possible_value.rs @@ -161,6 +161,11 @@ impl<'help> PossibleValue<'help> { self.hide } + /// Report if PossibleValue is not hidden and has a help message + pub fn should_show_help(&self) -> bool { + !self.hide && self.help.is_some() + } + /// Get the name if argument value is not hidden, `None` otherwise pub fn get_visible_name(&self) -> Option<&'help str> { if self.hide { diff --git a/src/output/help.rs b/src/output/help.rs index 7f5fb3bd7bc..0a9ace45c20 100644 --- 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,96 @@ 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 { + if self.use_long + && 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_name().map(display_width)) + .max() + .expect("Only called with possible value"); + let help_longest = arg + .possible_vals + .iter() + .filter_map(|f| f.get_help().map(display_width)) + .max() + .expect("Only called with possible value with help"); + // should new line + let taken = longest + spaces + 2; // 2 = "- " + let possible_value_new_line = self.term_w >= taken + && (taken as f32 / self.term_w as f32) > 0.60 + && help_longest + 2 // 2 = ": " + > (self.term_w - taken); + + let spaces = spaces + TAB_WIDTH; + let spaces_help = if possible_value_new_line { + spaces + TAB_WIDTH + } else { + spaces + longest + 4 // 2 = "- " + ": " + }; + + 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(": ")?; + } + + 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 +536,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 +646,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 @@ -670,7 +749,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 +1090,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 index 9321b2822c5..0b515b51a53 100644 --- 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 diff --git a/tests/builder/help.rs b/tests/builder/help.rs index d07829a26eb..17789d371e1 100644 --- a/tests/builder/help.rs +++ b/tests/builder/help.rs @@ -246,6 +246,41 @@ OPTIONS: static HIDE_POS_VALS: &str = "ctest 0.1 +USAGE: + ctest [OPTIONS] + +OPTIONS: + -c, --cafe A coffeehouse, coffee shop, or café. + -h, --help Print help information + -p, --pos Some vals [possible values: fast, slow] + -V, --version Print version information +"; +#[cfg(feature = "unstable-v4")] +static POS_VALS_HELP: &str = "ctest 0.1 + +USAGE: + ctest [OPTIONS] + +OPTIONS: + -c, --cafe + A coffeehouse, coffee shop, or café. + + -h, --help + Print help information + + -p, --pos + Some vals + + Possible values: + - fast + - slow: not as fast + + -V, --version + Print version information +"; +#[cfg(not(feature = "unstable-v4"))] +static POS_VALS_HELP: &str = "ctest 0.1 + USAGE: ctest [OPTIONS] @@ -964,6 +999,71 @@ OPTIONS: )); } +#[test] +#[cfg(all(feature = "wrap_help"))] +fn possible_value_wrapped_help() { + #[cfg(feature = "unstable-v4")] + static WRAPPED_HELP: &str = "test + +USAGE: + test [OPTIONS] + +OPTIONS: + -h, --help + Print help information + + --possible-values + Possible values: + - name: Long enough help message to clearly + warrant wrapping + - second + + --possible-values-with-new-line + Possible values: + - long enough name to trigger new line: + Long enough help message to clearly warrant + wrapping + - second +"; + #[cfg(not(feature = "unstable-v4"))] + static WRAPPED_HELP: &str = r#"test + +USAGE: + test [OPTIONS] + +OPTIONS: + -h, --help Print help information + --possible-values [possible values: name, second] + --possible-values-with-new-line [possible values: "long enough name to trigger new line", second] +"#; + let cmd = Command::new("test") + .term_width(67) + .arg( + Arg::new("possible_values") + .long("possible-values") + .possible_value( + PossibleValue::new("name") + .help("Long enough help message to clearly warrant wrapping"), + ) + .possible_value("second"), + ) + .arg( + Arg::new("possible_values_with_new_line") + .long("possible-values-with-new-line") + .possible_value( + PossibleValue::new("long enough name to trigger new line") + .help("Long enough help message to clearly warrant wrapping"), + ) + .possible_value("second"), + ); + assert!(utils::compare_output( + cmd, + "test --help", + WRAPPED_HELP, + false + )); +} + #[test] fn complex_subcommand_help_output() { let a = utils::complex_app(); @@ -1061,6 +1161,37 @@ fn hide_single_possible_val() { )); } +#[test] +fn possible_vals_with_help() { + let app = Command::new("ctest") + .version("0.1") + .arg( + Arg::new("pos") + .short('p') + .long("pos") + .value_name("VAL") + .possible_value("fast") + .possible_value(PossibleValue::new("slow").help("not as fast")) + .possible_value(PossibleValue::new("secret speed").hide(true)) + .help("Some vals") + .takes_value(true), + ) + .arg( + Arg::new("cafe") + .short('c') + .long("cafe") + .value_name("FILE") + .help("A coffeehouse, coffee shop, or café.") + .takes_value(true), + ); + assert!(utils::compare_output( + app, + "ctest --help", + POS_VALS_HELP, + false + )); +} + #[test] fn issue_626_panic() { let cmd = Command::new("ctest") From 22412defa9d6335e8d70569ac2fb37cd195fbf68 Mon Sep 17 00:00:00 2001 From: Roland Fredenhagen Date: Wed, 23 Feb 2022 15:29:08 +0100 Subject: [PATCH 3/8] docs(readme): Bold for `unstable-v4` feature --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 97644a9ff85..de25cae97c1 100644 --- a/README.md +++ b/README.md @@ -153,7 +153,7 @@ Why use the procedural [Builder API](https://github.com/clap-rs/clap/blob/v3.1.1 * **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: Show help messages for possible values in long help [#3312](https://github.com/clap-rs/clap/issues/3312) +* **unstable-v4**: Show help messages for possible values in long help [#3312](https://github.com/clap-rs/clap/issues/3312) ## Sponsors From cad5430922813ba6086d66ac90e2ab295b360c73 Mon Sep 17 00:00:00 2001 From: Roland Fredenhagen Date: Wed, 23 Feb 2022 15:33:54 +0100 Subject: [PATCH 4/8] chore(ci): Add `next` feature to ci --- .github/workflows/ci.yml | 6 +++++- .github/workflows/rust-next.yml | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0fe3f3f4280..be3e6779556 100644 --- 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 index f66a64970b3..50a46298721 100644 --- 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: From 592d9817b221372aac3803ec3d6b671a3f854312 Mon Sep 17 00:00:00 2001 From: Roland Fredenhagen Date: Tue, 1 Mar 2022 13:17:59 +0100 Subject: [PATCH 5/8] fix: Address pr review --- clap_complete/src/shells/zsh.rs | 2 +- src/build/possible_value.rs | 25 ++++++- src/output/help.rs | 30 ++++----- tests/builder/help.rs | 115 ++++++++++++++++++-------------- tests/builder/utils.rs | 23 ++++++- 5 files changed, 124 insertions(+), 71 deletions(-) diff --git a/clap_complete/src/shells/zsh.rs b/clap_complete/src/shells/zsh.rs index 2b785c60c7e..1f3786abbfb 100644 --- a/clap_complete/src/shells/zsh.rs +++ b/clap_complete/src/shells/zsh.rs @@ -362,7 +362,7 @@ fn value_completion(arg: &Arg) -> Option { if let Some(values) = &arg.get_possible_values() { if values .iter() - .any(|value| !value.is_hide_set() && value.get_help().is_some()) + .any(|value| value.should_show_help()) { Some(format!( "(({}))", diff --git a/src/build/possible_value.rs b/src/build/possible_value.rs index 6c205dc4d8a..3417edb28d8 100644 --- 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,17 @@ impl<'help> PossibleValue<'help> { self.help } + /// Get the help specified for this argument, if any and the argument + /// value is not hidden + #[inline] + pub 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`")] @@ -175,6 +186,18 @@ impl<'help> PossibleValue<'help> { } } + /// Get the name if argument value is not hidden, `None` otherwise, + /// but wrapped in quotes if it contains whitespace + pub fn get_visible_quoted_name(&self) -> Option> { + self.get_visible_name().map(|name| { + if name.contains(char::is_whitespace) { + format!("{name:?}").into() + } else { + name.into() + } + }) + } + /// 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 index 0a9ace45c20..9f0e962eb36 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -443,6 +443,7 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> { #[cfg(feature = "unstable-v4")] if let Some(arg) = arg { if self.use_long + && !arg.is_hide_possible_values_set() && arg .possible_vals .iter() @@ -457,27 +458,26 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> { let longest = arg .possible_vals .iter() - .filter_map(|f| f.get_visible_name().map(display_width)) + .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_help().map(display_width)) + .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 + 2; // 2 = "- " - let possible_value_new_line = self.term_w >= taken - && (taken as f32 / self.term_w as f32) > 0.60 - && help_longest + 2 // 2 = ": " - > (self.term_w - taken); - let spaces = spaces + TAB_WIDTH; + let possible_value_new_line = + self.term_w >= taken && self.term_w < taken + 2 + help_longest; // 2 = ": " + + let spaces = spaces + TAB_WIDTH - 2; // 2 = "- " let spaces_help = if possible_value_new_line { - spaces + TAB_WIDTH + spaces + 2 // 2 = "- " } else { - spaces + longest + 4 // 2 = "- " + ": " + spaces + longest + 4 // 4 = "- " + ": " }; for pv in arg.possible_vals.iter().filter(|pv| !pv.is_hide_set()) { @@ -493,6 +493,8 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> { 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 { @@ -660,15 +662,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::>() .join(", "); diff --git a/tests/builder/help.rs b/tests/builder/help.rs index 17789d371e1..1df2aa6244f 100644 --- a/tests/builder/help.rs +++ b/tests/builder/help.rs @@ -246,41 +246,6 @@ OPTIONS: static HIDE_POS_VALS: &str = "ctest 0.1 -USAGE: - ctest [OPTIONS] - -OPTIONS: - -c, --cafe A coffeehouse, coffee shop, or café. - -h, --help Print help information - -p, --pos Some vals [possible values: fast, slow] - -V, --version Print version information -"; -#[cfg(feature = "unstable-v4")] -static POS_VALS_HELP: &str = "ctest 0.1 - -USAGE: - ctest [OPTIONS] - -OPTIONS: - -c, --cafe - A coffeehouse, coffee shop, or café. - - -h, --help - Print help information - - -p, --pos - Some vals - - Possible values: - - fast - - slow: not as fast - - -V, --version - Print version information -"; -#[cfg(not(feature = "unstable-v4"))] -static POS_VALS_HELP: &str = "ctest 0.1 - USAGE: ctest [OPTIONS] @@ -1014,16 +979,22 @@ OPTIONS: --possible-values Possible values: - - name: Long enough help message to clearly - warrant wrapping - - second + - short_name: + Long enough help message, barely warrant wrapping + - second: + Short help gets handled the same --possible-values-with-new-line Possible values: - - long enough name to trigger new line: - Long enough help message to clearly warrant - wrapping - - second + - long enough name to trigger new line: + Really long enough help message to clearly warrant + wrapping believe me + - second + + --possible-values-without-new-line + Possible values: + - name: Short enough help message with no wrapping + - second: short help "; #[cfg(not(feature = "unstable-v4"))] static WRAPPED_HELP: &str = r#"test @@ -1032,9 +1003,10 @@ USAGE: test [OPTIONS] OPTIONS: - -h, --help Print help information - --possible-values [possible values: name, second] - --possible-values-with-new-line [possible values: "long enough name to trigger new line", second] + -h, --help Print help information + --possible-values [possible values: short_name, second] + --possible-values-with-new-line [possible values: "long enough name to trigger new line", second] + --possible-values-without-new-line [possible values: name, second] "#; let cmd = Command::new("test") .term_width(67) @@ -1042,19 +1014,29 @@ OPTIONS: Arg::new("possible_values") .long("possible-values") .possible_value( - PossibleValue::new("name") - .help("Long enough help message to clearly warrant wrapping"), + PossibleValue::new("short_name") + .help("Long enough help message, barely warrant wrapping"), ) - .possible_value("second"), + .possible_value( + PossibleValue::new("second").help("Short help gets handled the same"), + ), ) .arg( Arg::new("possible_values_with_new_line") .long("possible-values-with-new-line") .possible_value( PossibleValue::new("long enough name to trigger new line") - .help("Long enough help message to clearly warrant wrapping"), + .help("Really long enough help message to clearly warrant wrapping believe me"), ) .possible_value("second"), + ) + .arg( + Arg::new("possible_values_without_new_line") + .long("possible-values-without-new-line") + .possible_value( + PossibleValue::new("name").help("Short enough help message with no wrapping"), + ) + .possible_value(PossibleValue::new("second").help("short help")), ); assert!(utils::compare_output( cmd, @@ -1163,6 +1145,41 @@ fn hide_single_possible_val() { #[test] fn possible_vals_with_help() { + #[cfg(feature = "unstable-v4")] + static POS_VALS_HELP: &str = "ctest 0.1 + +USAGE: + ctest [OPTIONS] + +OPTIONS: + -c, --cafe + A coffeehouse, coffee shop, or café. + + -h, --help + Print help information + + -p, --pos + Some vals + + Possible values: + - fast + - slow: not as fast + + -V, --version + Print version information +"; + #[cfg(not(feature = "unstable-v4"))] + static POS_VALS_HELP: &str = "ctest 0.1 + +USAGE: + ctest [OPTIONS] + +OPTIONS: + -c, --cafe A coffeehouse, coffee shop, or café. + -h, --help Print help information + -p, --pos Some vals [possible values: fast, slow] + -V, --version Print version information +"; let app = Command::new("ctest") .version("0.1") .arg( diff --git a/tests/builder/utils.rs b/tests/builder/utils.rs index 5a87684382b..05c58934986 100644 --- a/tests/builder/utils.rs +++ b/tests/builder/utils.rs @@ -1,6 +1,6 @@ #![allow(unused_imports, dead_code)] -use std::io::{Cursor, Write}; +use std::io::{BufRead, Cursor, Write}; use std::str; use regex::Regex; @@ -19,6 +19,20 @@ where let left_ = re.replace_all(&*ls, ""); let right = re.replace_all(&*rs, ""); let b = left_ == right; + let line_diff = left_ + .lines() + .zip(right.lines()) + .enumerate() + .filter_map(|(line, (left, right))| { + if left != right { + Some(format!("Line {line}:\n{left}\n{right}")) + } else { + None + } + }) + .collect::>() + .join("\n"); + let line_count_diff = (left_.lines().count() as isize) - right.lines().count() as isize; if !b { dbg!(&left_); dbg!(&right); @@ -27,7 +41,12 @@ where println!("{}", left_); println!("--> right"); println!("{}", right); - println!("--") + println!("--> diff"); + println!("{line_diff}"); + println!("--"); + if line_count_diff != 0 { + println!("left line count - right line count = {line_count_diff}"); + } } b } From b25d0de2c512f2bd8b99d2bf75da60738eb7ab84 Mon Sep 17 00:00:00 2001 From: Roland Fredenhagen Date: Tue, 1 Mar 2022 13:50:01 +0100 Subject: [PATCH 6/8] chore: MSRV and rustfmt --- clap_complete/src/shells/zsh.rs | 5 +---- src/build/possible_value.rs | 2 +- tests/builder/help.rs | 5 +++-- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/clap_complete/src/shells/zsh.rs b/clap_complete/src/shells/zsh.rs index 1f3786abbfb..838e9b2283b 100644 --- a/clap_complete/src/shells/zsh.rs +++ b/clap_complete/src/shells/zsh.rs @@ -360,10 +360,7 @@ fn get_args_of(parent: &Command, p_global: Option<&Command>) -> String { // Uses either `possible_vals` or `value_hint` to give hints about possible argument values fn value_completion(arg: &Arg) -> Option { if let Some(values) = &arg.get_possible_values() { - if values - .iter() - .any(|value| value.should_show_help()) - { + if values.iter().any(|value| value.should_show_help()) { Some(format!( "(({}))", values diff --git a/src/build/possible_value.rs b/src/build/possible_value.rs index 3417edb28d8..343d9e875c0 100644 --- a/src/build/possible_value.rs +++ b/src/build/possible_value.rs @@ -191,7 +191,7 @@ impl<'help> PossibleValue<'help> { pub fn get_visible_quoted_name(&self) -> Option> { self.get_visible_name().map(|name| { if name.contains(char::is_whitespace) { - format!("{name:?}").into() + format!("{:?}", name).into() } else { name.into() } diff --git a/tests/builder/help.rs b/tests/builder/help.rs index 1df2aa6244f..211f7e0d483 100644 --- a/tests/builder/help.rs +++ b/tests/builder/help.rs @@ -1025,8 +1025,9 @@ OPTIONS: Arg::new("possible_values_with_new_line") .long("possible-values-with-new-line") .possible_value( - PossibleValue::new("long enough name to trigger new line") - .help("Really long enough help message to clearly warrant wrapping believe me"), + PossibleValue::new("long enough name to trigger new line").help( + "Really long enough help message to clearly warrant wrapping believe me", + ), ) .possible_value("second"), ) From 975d1fcbdfaf8ed8f870a2b582f7856de8f5be95 Mon Sep 17 00:00:00 2001 From: Roland Fredenhagen Date: Tue, 1 Mar 2022 14:27:14 +0100 Subject: [PATCH 7/8] fix: MSRV --- tests/builder/utils.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/builder/utils.rs b/tests/builder/utils.rs index 05c58934986..1e146ea4056 100644 --- a/tests/builder/utils.rs +++ b/tests/builder/utils.rs @@ -25,7 +25,7 @@ where .enumerate() .filter_map(|(line, (left, right))| { if left != right { - Some(format!("Line {line}:\n{left}\n{right}")) + Some(format!("Line {}:\n{}\n{}", line, left, right)) } else { None } @@ -42,10 +42,10 @@ where println!("--> right"); println!("{}", right); println!("--> diff"); - println!("{line_diff}"); + println!("{}", line_diff); println!("--"); if line_count_diff != 0 { - println!("left line count - right line count = {line_count_diff}"); + println!("left line count - right line count = {}", line_count_diff); } } b From a796429c769b3a7ba537fdc4e0b84e2460c4b3c2 Mon Sep 17 00:00:00 2001 From: Roland Fredenhagen Date: Wed, 2 Mar 2022 08:47:56 +0100 Subject: [PATCH 8/8] chore: Address pr review --- README.md | 2 +- clap_complete/src/shells/bash.rs | 3 ++- clap_complete/src/shells/zsh.rs | 8 ++++++-- src/build/possible_value.rs | 25 ++++++++++++++++--------- src/output/help.rs | 12 +++++++----- src/parse/validator.rs | 12 ++++++++---- 6 files changed, 40 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index de25cae97c1..b04b123013a 100644 --- a/README.md +++ b/README.md @@ -153,7 +153,7 @@ Why use the procedural [Builder API](https://github.com/clap-rs/clap/blob/v3.1.1 * **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**: Show help messages for possible values in long help [#3312](https://github.com/clap-rs/clap/issues/3312) +* **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 index 1625d0ce9c0..08bf1190cc3 100644 --- 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::>() .join(" ") ) diff --git a/clap_complete/src/shells/zsh.rs b/clap_complete/src/shells/zsh.rs index 838e9b2283b..2b64739cedf 100644 --- a/clap_complete/src/shells/zsh.rs +++ b/clap_complete/src/shells/zsh.rs @@ -360,7 +360,10 @@ fn get_args_of(parent: &Command, p_global: Option<&Command>) -> String { // Uses either `possible_vals` or `value_hint` to give hints about possible argument values fn value_completion(arg: &Arg) -> Option { if let Some(values) = &arg.get_possible_values() { - if values.iter().any(|value| value.should_show_help()) { + if values + .iter() + .any(|value| !value.is_hide_set() && value.get_help().is_some()) + { Some(format!( "(({}))", values @@ -384,7 +387,8 @@ fn value_completion(arg: &Arg) -> Option { "({})", values .iter() - .filter_map(PossibleValue::get_visible_name) + .filter(|pv| !pv.is_hide_set()) + .map(PossibleValue::get_name) .collect::>() .join(" ") )) diff --git a/src/build/possible_value.rs b/src/build/possible_value.rs index 343d9e875c0..e56fb858ce6 100644 --- a/src/build/possible_value.rs +++ b/src/build/possible_value.rs @@ -151,7 +151,8 @@ impl<'help> PossibleValue<'help> { /// Get the help specified for this argument, if any and the argument /// value is not hidden #[inline] - pub fn get_visible_help(&self) -> Option<&'help str> { + #[cfg(feature = "unstable-v4")] + pub(crate) fn get_visible_help(&self) -> Option<&'help str> { if !self.hide { self.help } else { @@ -173,11 +174,15 @@ impl<'help> PossibleValue<'help> { } /// Report if PossibleValue is not hidden and has a help message - pub fn should_show_help(&self) -> bool { + 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 @@ -188,14 +193,16 @@ impl<'help> PossibleValue<'help> { /// Get the name if argument value is not hidden, `None` otherwise, /// but wrapped in quotes if it contains whitespace - pub fn get_visible_quoted_name(&self) -> Option> { - self.get_visible_name().map(|name| { - if name.contains(char::is_whitespace) { - format!("{:?}", name).into() + pub(crate) fn get_visible_quoted_name(&self) -> Option> { + if !self.hide { + Some(if self.name.contains(char::is_whitespace) { + format!("{:?}", self.name).into() } else { - name.into() - } - }) + self.name.into() + }) + } else { + None + } } /// Returns all valid values of the argument value. diff --git a/src/output/help.rs b/src/output/help.rs index 9f0e962eb36..03221686714 100644 --- a/src/output/help.rs +++ b/src/output/help.rs @@ -442,6 +442,8 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> { #[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 @@ -468,16 +470,16 @@ impl<'help, 'cmd, 'writer> Help<'help, 'cmd, 'writer> { .max() .expect("Only called with possible value with help"); // should new line - let taken = longest + spaces + 2; // 2 = "- " + let taken = longest + spaces + DASH_SPACE; let possible_value_new_line = - self.term_w >= taken && self.term_w < taken + 2 + help_longest; // 2 = ": " + self.term_w >= taken && self.term_w < taken + COLON_SPACE + help_longest; - let spaces = spaces + TAB_WIDTH - 2; // 2 = "- " + let spaces = spaces + TAB_WIDTH - DASH_SPACE; let spaces_help = if possible_value_new_line { - spaces + 2 // 2 = "- " + spaces + DASH_SPACE } else { - spaces + longest + 4 // 4 = "- " + ": " + spaces + longest + DASH_SPACE + COLON_SPACE }; for pv in arg.possible_vals.iter().filter(|pv| !pv.is_hide_set()) { diff --git a/src/parse/validator.rs b/src/parse/validator.rs index 18e1bb7609e..e1d17ac2843 100644 --- a/src/parse/validator.rs +++ b/src/parse/validator.rs @@ -46,7 +46,8 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { self.p.cmd, &o.possible_vals .iter() - .filter_map(PossibleValue::get_visible_name) + .filter(|pv| !pv.is_hide_set()) + .map(PossibleValue::get_name) .collect::>(), o, Usage::new(self.p.cmd) @@ -141,7 +142,8 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { val_str.into_owned(), &arg.possible_vals .iter() - .filter_map(PossibleValue::get_visible_name) + .filter(|pv| !pv.is_hide_set()) + .map(PossibleValue::get_name) .collect::>(), arg, Usage::new(self.p.cmd) @@ -156,7 +158,8 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { self.p.cmd, &arg.possible_vals .iter() - .filter_map(PossibleValue::get_visible_name) + .filter(|pv| !pv.is_hide_set()) + .map(PossibleValue::get_name) .collect::>(), arg, Usage::new(self.p.cmd) @@ -454,7 +457,8 @@ impl<'help, 'cmd, 'parser> Validator<'help, 'cmd, 'parser> { self.p.cmd, &a.possible_vals .iter() - .filter_map(PossibleValue::get_visible_name) + .filter(|pv| !pv.is_hide_set()) + .map(PossibleValue::get_name) .collect::>(), a, Usage::new(self.p.cmd)