Skip to content

Commit

Permalink
fix: Address pr review
Browse files Browse the repository at this point in the history
  • Loading branch information
@ModProg
ModProg committed Mar 1, 2022
1 parent cad5430 commit 592d981
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 71 deletions.
2 changes: 1 addition & 1 deletion clap_complete/src/shells/zsh.rs
View file
Expand Up @@ -362,7 +362,7 @@ fn value_completion(arg: &Arg) -> Option<String> {
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!(
"(({}))",
Expand Down
25 changes: 24 additions & 1 deletion src/build/possible_value.rs
View file
@@ -1,4 +1,4 @@
use std::iter;
use std::{borrow::Cow, iter};

use crate::util::eq_ignore_case;

Expand Down Expand Up @@ -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`")]
Expand Down Expand Up @@ -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<Cow<'help, str>> {
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.
Expand Down
30 changes: 12 additions & 18 deletions src/output/help.rs
View file
Expand Up @@ -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()
Expand All @@ -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()) {
Expand All @@ -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 {
Expand Down Expand Up @@ -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::<Vec<_>>()
.join(", ");

Expand Down
115 changes: 66 additions & 49 deletions tests/builder/help.rs
View file
Expand Up @@ -246,41 +246,6 @@ OPTIONS:

static HIDE_POS_VALS: &str = "ctest 0.1
USAGE:
ctest [OPTIONS]
OPTIONS:
-c, --cafe <FILE> A coffeehouse, coffee shop, or café.
-h, --help Print help information
-p, --pos <VAL> 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 <FILE>
A coffeehouse, coffee shop, or café.
-h, --help
Print help information
-p, --pos <VAL>
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]
Expand Down Expand Up @@ -1014,16 +979,22 @@ OPTIONS:
--possible-values <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_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_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
Expand All @@ -1032,29 +1003,40 @@ USAGE:
test [OPTIONS]
OPTIONS:
-h, --help Print help information
--possible-values <possible_values> [possible values: name, second]
--possible-values-with-new-line <possible_values_with_new_line> [possible values: "long enough name to trigger new line", second]
-h, --help Print help information
--possible-values <possible_values> [possible values: short_name, second]
--possible-values-with-new-line <possible_values_with_new_line> [possible values: "long enough name to trigger new line", second]
--possible-values-without-new-line <possible_values_without_new_line> [possible values: name, 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"),
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,
Expand Down Expand Up @@ -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 <FILE>
A coffeehouse, coffee shop, or café.
-h, --help
Print help information
-p, --pos <VAL>
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 <FILE> A coffeehouse, coffee shop, or café.
-h, --help Print help information
-p, --pos <VAL> Some vals [possible values: fast, slow]
-V, --version Print version information
";
let app = Command::new("ctest")
.version("0.1")
.arg(
Expand Down
23 changes: 21 additions & 2 deletions tests/builder/utils.rs
View file
@@ -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;
Expand All @@ -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::<Vec<_>>()
.join("\n");
let line_count_diff = (left_.lines().count() as isize) - right.lines().count() as isize;
if !b {
dbg!(&left_);
dbg!(&right);
Expand All @@ -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
}
Expand Down

0 comments on commit 592d981

Please sign in to comment.