From 9d482d00ba63ce81e260488c2ce37978cd086953 Mon Sep 17 00:00:00 2001
From: Ed Page <eopage@gmail.com>
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<String> = 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::<Vec<_>>(),
                     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::<Vec<_>>(),
                     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::<Vec<_>>(),
                 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 <option>' requires a value but none was supplied
+\t[possible values: slow, fast, \"ludicrous speed\"]
+
+USAGE:
+    clap-test [OPTIONS]
+
+For more information try --help
+";
+
 #[test]
 fn alias() {
     let m = App::new("pv")