From 9d482d00ba63ce81e260488c2ce37978cd086953 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 2 Feb 2022 14:28:41 -0600 Subject: [PATCH] feat(error): Show possible values when none are supplied This is inspired by cargo which allows you to run `cargo test --test` and it will list the possible tests (obviously we can't support that atm because that requires a lot of runtime processing). When we do have a static list of possible values, we can at least show those. Fixes #3320 --- src/parse/errors.rs | 26 +++++++++++++++++++++++++- src/parse/validator.rs | 12 ++++++++++++ tests/builder/possible_values.rs | 27 +++++++++++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/src/parse/errors.rs b/src/parse/errors.rs index 70505bfe02f..b1604cb0346 100644 --- a/src/parse/errors.rs +++ b/src/parse/errors.rs @@ -605,13 +605,37 @@ impl Error { Self::for_app(app, c, ErrorKind::ArgumentConflict, others) } - pub(crate) fn empty_value(app: &App, arg: &Arg, usage: String) -> Self { + pub(crate) fn empty_value(app: &App, good_vals: &[&str], arg: &Arg, usage: String) -> Self { let mut c = Colorizer::new(true, app.get_color()); let arg = arg.to_string(); start_error(&mut c, "The argument '"); c.warning(&*arg); c.none("' requires a value but none was supplied"); + if !good_vals.is_empty() { + let good_vals: Vec = good_vals + .iter() + .map(|&v| { + if v.contains(char::is_whitespace) { + format!("{:?}", v) + } else { + v.to_owned() + } + }) + .collect(); + c.none("\n\t[possible values: "); + + if let Some((last, elements)) = good_vals.split_last() { + for v in elements { + c.good(v); + c.none(", "); + } + + c.good(last); + } + + c.none("]"); + } put_usage(&mut c, usage); try_help(app, &mut c); diff --git a/src/parse/validator.rs b/src/parse/validator.rs index 35e81466946..0e9ae2ccad0 100644 --- a/src/parse/validator.rs +++ b/src/parse/validator.rs @@ -49,6 +49,10 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> { if should_err { return Err(Error::empty_value( self.p.app, + &o.possible_vals + .iter() + .filter_map(PossibleValue::get_visible_name) + .collect::>(), o, Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]), )); @@ -133,6 +137,10 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> { debug!("Validator::validate_arg_values: illegal empty val found"); return Err(Error::empty_value( self.p.app, + &arg.possible_vals + .iter() + .filter_map(PossibleValue::get_visible_name) + .collect::>(), arg, Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]), )); @@ -407,6 +415,10 @@ impl<'help, 'app, 'parser> Validator<'help, 'app, 'parser> { if a.is_set(ArgSettings::TakesValue) && !min_vals_zero && ma.all_val_groups_empty() { return Err(Error::empty_value( self.p.app, + &a.possible_vals + .iter() + .filter_map(PossibleValue::get_visible_name) + .collect::>(), a, Usage::new(self.p.app, &self.p.required).create_usage_with_title(&[]), )); diff --git a/tests/builder/possible_values.rs b/tests/builder/possible_values.rs index e7fdb9104ed..a23f930ee71 100644 --- a/tests/builder/possible_values.rs +++ b/tests/builder/possible_values.rs @@ -265,6 +265,33 @@ fn escaped_possible_values_output() { )); } +#[test] +fn missing_possible_value_error() { + assert!(utils::compare_output( + App::new("test").arg( + Arg::new("option") + .short('O') + .possible_value("slow") + .possible_value(PossibleValue::new("fast").alias("fost")) + .possible_value(PossibleValue::new("ludicrous speed")) + .possible_value(PossibleValue::new("forbidden speed").hide(true)) + ), + "clap-test -O", + MISSING_PV_ERROR, + true + )); +} + +static MISSING_PV_ERROR: &str = + "error: The argument '-O