Repository: pacak/bpaf (rust)

Issue:
fallback for `some`
Currently it is possible to emulate `fallback` for for vectors of items using `map` and `parse`. Does it make sense to support this natively for `some`?

```rust
#[test]
fn default_for_many() {
    use bpaf;

    let roots = bpaf::positional::<String>("ROOTS").many().map(|items| {
        if items.is_empty() {
            vec![String::from(".")]
        } else {
            items
        }
    });

    struct Roots {
        roots: Vec<String>,
    }

    let r = construct!(Roots { roots })
        .to_options()
        .run_inner(Args::from(&[]));
    assert_eq!(vec![String::from(".")], r.unwrap().roots);
}
```

See https://github.com/pacak/bpaf/issues/79 for original question

Pull request:
failing `some` can be now recovered with `fallback`
fixes #98

Patch:
diff --git a/Changelog.md b/Changelog.md
index 888bcc4..2da338b 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -10,8 +10,10 @@
 - parsing combinators `many`, `some`, `optional` and `anywhere` will
   now propagate parsing errors outwards, you can regain the old
   behvior by specifying `catch`
+- better error messages related to `anywhere` parsers
 - support deriving `req_flag` consumers
 - support deriving `catch` annotation
+- errors generated by `some` can now be handled with `fallback`/`fallback_with`
 
 ## bpaf [0.7.10], bpaf_derive [0.3.5] - 2013-03-19
 - improve error messages for typos like `-llvm` instead of `--llvm`
diff --git a/src/args.rs b/src/args.rs
index 30ec85a..ee8582d 100644
--- a/src/args.rs
+++ b/src/args.rs
@@ -529,19 +529,25 @@ impl Args {
     }
 
     pub(crate) fn word_parse_error(&mut self, error: &str) -> Error {
-        Error::Message(if let Some(os) = self.current_word() {
-            format!("Couldn't parse {:?}: {}", os.to_string_lossy(), error)
-        } else {
-            format!("Couldn't parse: {}", error)
-        })
+        Error::Message(
+            if let Some(os) = self.current_word() {
+                format!("Couldn't parse {:?}: {}", os.to_string_lossy(), error)
+            } else {
+                format!("Couldn't parse: {}", error)
+            },
+            false,
+        )
     }
 
     pub(crate) fn word_validate_error(&mut self, error: &str) -> Error {
-        Error::Message(if let Some(os) = self.current_word() {
-            format!("{:?}: {}", os.to_string_lossy(), error)
-        } else {
-            error.to_owned()
-        })
+        Error::Message(
+            if let Some(os) = self.current_word() {
+                format!("{:?}: {}", os.to_string_lossy(), error)
+            } else {
+                error.to_owned()
+            },
+            false,
+        )
     }
 
     /// Get a short or long flag: `-f` / `--flag`
@@ -591,9 +597,14 @@ impl Args {
                     let os = os.to_string_lossy();
                     format!( "`{}` requires an argument, got a flag-like `{}`, try `{}={}` to use it as an argument", arg, os, arg,os)
                 };
-                return Err(Error::Message(msg));
+                return Err(Error::Message(msg, false));
+            }
+            _ => {
+                return Err(Error::Message(
+                    format!("{} requires an argument", arg),
+                    false,
+                ))
             }
-            _ => return Err(Error::Message(format!("{} requires an argument", arg))),
         };
         let val = val.clone();
         self.current = Some(val_ix);
@@ -623,10 +634,10 @@ impl Args {
                 self.remove(ix);
                 Ok(Some((false, w)))
             }
-            Some((_, arg)) => Err(Error::Message(format!(
-                "Expected an argument <{}>, got {}",
-                metavar, arg
-            ))),
+            Some((_, arg)) => Err(Error::Message(
+                format!("Expected an argument <{}>, got {}", metavar, arg),
+                false,
+            )),
             None => Ok(None),
         }
     }
diff --git a/src/help.rs b/src/help.rs
index 67773a4..46007a8 100644
--- a/src/help.rs
+++ b/src/help.rs
@@ -16,7 +16,10 @@ impl Parser<ExtraParams> for ParseExtraParams {
 
         match self.version {
             Some(ver) => Self::ver(ver).eval(args),
-            None => Err(Error::Message(String::from("Not a version or help flag"))),
+            None => Err(Error::Message(
+                String::from("Not a version or help flag"),
+                false,
+            )),
         }
     }
 
diff --git a/src/info.rs b/src/info.rs
index 28a52a7..92a1a0f 100644
--- a/src/info.rs
+++ b/src/info.rs
@@ -14,7 +14,9 @@ use crate::{
 #[derive(Debug)]
 pub enum Error {
     /// Parsing failed, it is still possible to improve the error message
-    Message(String),
+    /// bool shows if this error can be caught with fallback, messages with it set to true are
+    /// generated by .some(msg) modifier and allow to use it with .fallback
+    Message(String, bool),
     /// Parsing failed and this is the final output
     ParseFailure(ParseFailure),
     /// Expected one of those values
@@ -33,8 +35,8 @@ impl Error {
             (_, b @ Error::ParseFailure(_)) => b,
 
             // parsing failure takes priority
-            (a @ Error::Message(_), _) => a,
-            (_, b @ Error::Message(_)) => b,
+            (a @ Error::Message(_, _), _) => a,
+            (_, b @ Error::Message(_, _)) => b,
 
             // combine missing elements
             (Error::Missing(mut a), Error::Missing(mut b)) => {
@@ -72,7 +74,7 @@ fn check_unexpected(args: &Args) -> Result<(), Error> {
                 use std::fmt::Write;
                 write!(msg, ": {} cannot be used at the same time as {}", rej, acc).unwrap();
             }
-            Err(Error::Message(msg))
+            Err(Error::Message(msg, false))
         }
     }
 }
@@ -586,7 +588,7 @@ impl<T> OptionParser<T> {
 impl From<Error> for ParseFailure {
     fn from(value: Error) -> Self {
         match value {
-            Error::Message(msg) => ParseFailure::Stderr(msg),
+            Error::Message(msg, _) => ParseFailure::Stderr(msg),
             Error::ParseFailure(pf) => pf,
             Error::Missing(items) => ParseFailure::Stderr(format!(
                 "Expected {}, pass --help for usage information",
diff --git a/src/meta_youmean.rs b/src/meta_youmean.rs
index b9e1fe9..d676d94 100644
--- a/src/meta_youmean.rs
+++ b/src/meta_youmean.rs
@@ -7,7 +7,7 @@ use crate::{
 
 pub(crate) fn should_suggest(err: &Error) -> bool {
     match err {
-        Error::Message(_) => true,
+        Error::Message(_, _) => true,
         Error::ParseFailure(_) => false,
         Error::Missing(xs) => {
             let mut hi = crate::meta_help::HelpItems::default();
diff --git a/src/params.rs b/src/params.rs
index 33d23f3..35952e0 100644
--- a/src/params.rs
+++ b/src/params.rs
@@ -875,10 +875,10 @@ fn parse_word(
             #[cfg(feature = "autocomplete")]
             args.push_value("--", &Some("-- Positional only items".to_owned()), false);
 
-            return Err(Error::Message(format!(
-                "Expected <{}> to be on the right side of --",
-                metavar,
-            )));
+            return Err(Error::Message(
+                format!("Expected <{}> to be on the right side of --", metavar,),
+                false,
+            ));
         }
         #[cfg(feature = "autocomplete")]
         if args.touching_last_remove() && !args.no_pos_ahead {
diff --git a/src/structs.rs b/src/structs.rs
index f2fca4b..415322d 100644
--- a/src/structs.rs
+++ b/src/structs.rs
@@ -23,10 +23,10 @@ where
     fn eval(&self, args: &mut Args) -> Result<T, Error> {
         match self.inner.eval(args) {
             Ok(ok) => Ok(ok),
-            e @ Err(Error::Message(_) | Error::ParseFailure(_)) => e,
-            Err(Error::Missing(_)) => match (self.fallback)() {
+            e @ Err(Error::Message(_, false) | Error::ParseFailure(_)) => e,
+            Err(Error::Missing(_) | Error::Message(_, true)) => match (self.fallback)() {
                 Ok(ok) => Ok(ok),
-                Err(e) => Err(Error::Message(e.to_string())),
+                Err(e) => Err(Error::Message(e.to_string(), false)),
             },
         }
     }
@@ -101,7 +101,7 @@ where
         }
 
         if res.is_empty() {
-            Err(Error::Message(self.message.to_string()))
+            Err(Error::Message(self.message.to_string(), true))
         } else {
             Ok(res)
         }
@@ -377,8 +377,8 @@ where
                 std::mem::swap(args, &mut clone);
                 Ok(ok)
             }
-            e @ Err(Error::Message(_) | Error::ParseFailure(_)) => e,
-            Err(Error::Missing(_)) => {
+            e @ Err(Error::Message(_, false) | Error::ParseFailure(_)) => e,
+            Err(Error::Missing(_) | Error::Message(_, true)) => {
                 #[cfg(feature = "autocomplete")]
                 args.swap_comps(&mut clone);
                 Ok(self.value.clone())
@@ -495,8 +495,8 @@ where
             }
 
             match err {
-                Error::Message(_) if catch => Ok(None),
-                Error::Message(_) | Error::ParseFailure(_) => Err(err),
+                Error::Message(_, _) if catch => Ok(None),
+                Error::Message(_, _) | Error::ParseFailure(_) => Err(err),
                 Error::Missing(_) => {
                     if args.len() == orig_args.len() {
                         Ok(None)
@@ -558,7 +558,7 @@ impl<T: Clone + 'static, F: Fn() -> Result<T, E>, E: ToString> Parser<T>
     fn eval(&self, _args: &mut Args) -> Result<T, Error> {
         match (self.0)() {
             Ok(ok) => Ok(ok),
-            Err(e) => Err(Error::Message(e.to_string())),
+            Err(e) => Err(Error::Message(e.to_string(), true)),
         }
     }
 
@@ -575,7 +575,7 @@ pub struct ParseFail<T> {
 impl<T> Parser<T> for ParseFail<T> {
     fn eval(&self, args: &mut Args) -> Result<T, Error> {
         args.current = None;
-        Err(Error::Message(self.field1.to_owned()))
+        Err(Error::Message(self.field1.to_owned(), true))
     }
 
     fn meta(&self) -> Meta {
@@ -879,7 +879,7 @@ where
                 }
 
                 // otherwise return the error message we get
-                otherwise @ Err(Error::Message(_)) => {
+                otherwise @ Err(Error::Message(_, _)) => {
                     let consumed = args.len() - this_arg.len();
                     if consumed > 0 && !self.catch {
                         this_arg.copy_usage_from(args, 0..start);


Test patch:
diff --git a/tests/tests.rs b/tests/tests.rs
index c3ff372..c34e50d 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -1,4 +1,6 @@
 #![allow(deprecated)]
+use std::convert::Infallible;
+
 use bpaf::*;
 
 #[test]
@@ -1864,3 +1866,42 @@ fn some_env() {
     let r = parser.run_inner(Args::from(&[])).unwrap();
     assert_eq!(r, vec!["top s3cr3t".to_owned()]);
 }
+
+#[test]
+fn option_requires_other_option1() {
+    let a = short('a').switch();
+    let b = short('b').argument::<String>("B");
+    let parser = construct!(a, b).optional().to_options();
+
+    let r = parser
+        .run_inner(Args::from(&["-a"]))
+        .unwrap_err()
+        .unwrap_stderr();
+    assert_eq!(r, "Expected -b B, pass --help for usage information");
+}
+
+#[test]
+fn option_requires_other_option2() {
+    let a = short('a').switch();
+    let b = short('b').argument::<String>("B");
+    let parser = construct!(b, a).optional().to_options();
+
+    let r = parser
+        .run_inner(Args::from(&["-a"]))
+        .unwrap_err()
+        .unwrap_stderr();
+    // error message sucks...
+    assert_eq!(r, "-a is not expected in this context");
+}
+
+#[test]
+fn default_for_some() {
+    let parser = bpaf::positional::<u32>("ROOTS")
+        .some("msg")
+        .fallback_with(|| Ok::<_, Infallible>(vec![1, 2, 3]))
+        .to_options();
+
+    let r = parser.run_inner(Args::from(&[])).unwrap();
+
+    assert_eq!(r, vec![1, 2, 3]);
+}