Repository: pacak/bpaf
Pull request #192 (2023-03-26 14:05:57)

Better errors
Fixes #155 
Should be giving better error messages:
- less repetition on conflicting options
- point out at unexpected parts when requesting something expected


Patch:
diff --git a/docs/src/adjacent_0/cases.txt b/docs/src/adjacent_0/cases.txt
index 9535a38..f41c01c 100644
--- a/docs/src/adjacent_0/cases.txt
+++ b/docs/src/adjacent_0/cases.txt
@@ -12,4 +12,4 @@ Options { multi: [Multi { m: (), val_1: 10, val_2: 20, val_3: 3.1415 }, Multi {
 ? `-s` can't go in the middle as the parser expects the second item
 > -m 10 20 -s 3.1415
 Stderr
-Expected <V3>, pass --help for usage information
+Expected <V3>, got "-m". Pass --help for usage information
diff --git a/docs/src/adjacent_1/cases.txt b/docs/src/adjacent_1/cases.txt
index 0e93126..da981f6 100644
--- a/docs/src/adjacent_1/cases.txt
+++ b/docs/src/adjacent_1/cases.txt
@@ -11,4 +11,4 @@ Options { switch: true, multi: [Rect { item: (), width: 10, height: 10, painted:
 ? But with `adjacent` they cannot interleave
 > --rect --rect --width 10 --painted --height 10 --height 10 --width 10
 Stderr
-Expected --width PX, pass --help for usage information
+Expected --width PX, got "--rect". Pass --help for usage information
diff --git a/docs/src/boxed/combine.rs b/docs/src/boxed/combine.rs
index 515ff99..550f30c 100644
--- a/docs/src/boxed/combine.rs
+++ b/docs/src/boxed/combine.rs
@@ -21,7 +21,7 @@ pub fn options() -> OptionParser<Command> {
     let a = if sneaky {
         construct!(a)
     } else {
-        let f = fail("command a is not active");
+        let f = fail("No such command: `a`, did you mean `b`?");
         construct!(f)
     };
     construct!([a, b]).to_options()
diff --git a/docs/src/cargo_helper/cases.txt b/docs/src/cargo_helper/cases.txt
index ac668f7..f3d16df 100644
--- a/docs/src/cargo_helper/cases.txt
+++ b/docs/src/cargo_helper/cases.txt
@@ -7,7 +7,7 @@ Options { argument: 15, switch: false }
 ? uses _wrong_ subcommand name to bypass the helper and show how it would look without it
 > wrong --argument 15
 Stderr
-No such command: `wrong`, did you mean `-s`?
+No such command or positional: `wrong`, did you mean `--argument`?
 
 ? When used with the right command - helper simply consumes it
 > pretty --argument 42 -s
diff --git a/docs/src/req_flag/cases.txt b/docs/src/req_flag/cases.txt
index 620026e..75e5cab 100644
--- a/docs/src/req_flag/cases.txt
+++ b/docs/src/req_flag/cases.txt
@@ -14,7 +14,7 @@ Options { decision: Undecided }
 ? `--on` and `--off` are mutually exclusive:
 > --on --off
 Stderr
---off is not expected in this context: --off cannot be used at the same time as --on
+--off cannot be used at the same time as --on
 
 ? help
 > --help
diff --git a/src/args.rs b/src/args.rs
index ee8582d..b68cf67 100644
--- a/src/args.rs
+++ b/src/args.rs
@@ -23,7 +23,7 @@ impl Conflict {
 
 #[derive(Clone)]
 pub struct Improve(
-    pub(crate) fn(args: &mut Args, info: &Info, inner: &Meta, err: Error) -> ParseFailure,
+    pub(crate) fn(args: &mut Args, info: &Info, inner: &Meta, err: Option<Error>) -> ParseFailure,
 );
 
 impl std::fmt::Debug for Improve {
@@ -171,6 +171,7 @@ mod inner {
     impl Args {
         #[inline(never)]
         /// Get a list of command line arguments from OS
+        #[must_use]
         pub fn current_args() -> Self {
             let mut arg_vec = Vec::new();
             #[cfg(feature = "autocomplete")]
@@ -355,11 +356,17 @@ mod inner {
                     *removed = true;
                 }
             }
+            self.sync_remaining();
+        }
+
+        fn sync_remaining(&mut self) {
+            self.remaining = self.removed.iter().filter(|x| !**x).count();
         }
 
         /// Copy a range of removals from args to self
         pub(crate) fn copy_usage_from(&mut self, args: &Args, range: Range<usize>) {
             self.removed[range.start..range.end].copy_from_slice(&args.removed[range]);
+            self.sync_remaining();
         }
 
         #[inline(never)]
diff --git a/src/batteries.rs b/src/batteries.rs
index 5ba9264..f276560 100644
--- a/src/batteries.rs
+++ b/src/batteries.rs
@@ -182,6 +182,7 @@ where
 ///     }
 ///     ...
 /// ```
+#[allow(clippy::needless_pass_by_value)]
 pub fn get_usage<T>(parser: crate::OptionParser<T>) -> String
 where
     T: std::fmt::Debug,
diff --git a/src/docs/adjacent_0.md b/src/docs/adjacent_0.md
index 36f9921..96f64f9 100644
--- a/src/docs/adjacent_0.md
+++ b/src/docs/adjacent_0.md
@@ -91,7 +91,7 @@ Options { multi: [Multi { m: (), val_1: 10, val_2: 20, val_3: 3.1415 }, Multi {
 `-s` can't go in the middle as the parser expects the second item
 ```console
 % app -m 10 20 -s 3.1415
-Expected <V3>, pass --help for usage information
+Expected <V3>, got "-m". Pass --help for usage information
 ```
 
 </details>
diff --git a/src/docs/adjacent_1.md b/src/docs/adjacent_1.md
index ee40e77..fb1228f 100644
--- a/src/docs/adjacent_1.md
+++ b/src/docs/adjacent_1.md
@@ -60,7 +60,7 @@ Options { switch: true, multi: [Rect { item: (), width: 10, height: 10, painted:
 But with `adjacent` they cannot interleave
 ```console
 % app --rect --rect --width 10 --painted --height 10 --height 10 --width 10
-Expected --width PX, pass --help for usage information
+Expected --width PX, got "--rect". Pass --help for usage information
 ```
 
 </details>
diff --git a/src/docs/boxed.md b/src/docs/boxed.md
index b250687..535263f 100644
--- a/src/docs/boxed.md
+++ b/src/docs/boxed.md
@@ -23,7 +23,7 @@ pub fn options() -> OptionParser<Command> {
     let a = if sneaky {
         construct!(a)
     } else {
-        let f = fail("command a is not active");
+        let f = fail("No such command: `a`, did you mean `b`?");
         construct!(f)
     };
     construct!([a, b]).to_options()
diff --git a/src/docs/cargo_helper.md b/src/docs/cargo_helper.md
index 3c8cc7d..9757b3f 100644
--- a/src/docs/cargo_helper.md
+++ b/src/docs/cargo_helper.md
@@ -54,7 +54,7 @@ But when used as a `cargo` subcommand, cargo will also pass the command name, th
 uses _wrong_ subcommand name to bypass the helper and show how it would look without it
 ```console
 % app wrong --argument 15
-No such command: `wrong`, did you mean `-s`?
+No such command or positional: `wrong`, did you mean `--argument`?
 ```
 
 When used with the right command - helper simply consumes it
diff --git a/src/docs/req_flag.md b/src/docs/req_flag.md
index d7ec655..a128494 100644
--- a/src/docs/req_flag.md
+++ b/src/docs/req_flag.md
@@ -80,7 +80,7 @@ Options { decision: Undecided }
 `--on` and `--off` are mutually exclusive:
 ```console
 % app --on --off
---off is not expected in this context: --off cannot be used at the same time as --on
+--off cannot be used at the same time as --on
 ```
 
 help
diff --git a/src/help.rs b/src/help.rs
index 46007a8..6df6718 100644
--- a/src/help.rs
+++ b/src/help.rs
@@ -66,8 +66,9 @@ pub(crate) fn improve_error(
     args: &mut Args,
     info: &Info,
     inner: &Meta,
-    err: Error,
+    err: Option<Error>,
 ) -> ParseFailure {
+    // handle --help and --version messages
     match info.help_parser().eval(args) {
         Ok(ExtraParams::Help) => {
             let msg = render_help(info, inner, &info.help_parser().meta());
@@ -79,10 +80,51 @@ pub(crate) fn improve_error(
         Err(_) => {}
     }
 
-    if crate::meta_youmean::should_suggest(&err) {
-        if let Some(msg) = crate::meta_youmean::suggest(args, inner) {
-            return ParseFailure::Stderr(msg);
+    // at this point the input user gave us is invalid and we need to propose a step towards
+    // improving it. Improving steps can be:
+    // 1. adding something that is required but missing
+    //    + works best if there's no unexpected items left
+    //
+    // 2. suggesting to replace something that was typed wrongly: --asmm instead of --asm
+    //    + works best if there's something close enough to current item
+    //
+    // 3. suggesting to remove something that is totally not expected in this context
+    //    + safest fallback if earlier approaches failed
+
+    ParseFailure::Stderr(match err {
+        // parse succeeded, need to explain an unused argument
+        None => {
+            if let Some(msg) = crate::info::check_conflicts(args) {
+                msg
+            } else if let Some(msg) = crate::meta_youmean::suggest(args, inner) {
+                msg
+            } else if let Some((_ix, item)) = args.items_iter().next() {
+                format!("{} is not expected in this context", item)
+            } else {
+                // if parse succeeds and there's no unused items on a command line
+                // run_subparser returns the result.
+                unreachable!("Please open a ticket with bpaf, should not be reachable")
+            }
         }
-    }
-    ParseFailure::from(err)
+        Some(Error::ParseFailure(f)) => return f,
+        Some(Error::Message(msg, _)) => msg,
+        Some(Error::Missing(xs)) => {
+            if let Some(x) = args.peek() {
+                if let Some(msg) = crate::meta_youmean::suggest(args, inner) {
+                    msg
+                } else {
+                    format!(
+                        "Expected {}, got \"{}\". Pass --help for usage information",
+                        Meta::Or(xs.iter().map(|i| Meta::from(i.clone())).collect::<Vec<_>>()),
+                        x
+                    )
+                }
+            } else {
+                format!(
+                    "Expected {}, pass --help for usage information",
+                    Meta::Or(xs.iter().map(|i| Meta::from(i.clone())).collect::<Vec<_>>())
+                )
+            }
+        }
+    })
 }
diff --git a/src/info.rs b/src/info.rs
index 2ce6012..460e81e 100644
--- a/src/info.rs
+++ b/src/info.rs
@@ -65,17 +65,15 @@ pub struct Info {
     pub usage: Option<&'static str>,
 }
 
-fn check_unexpected(args: &Args) -> Result<(), Error> {
-    match args.items_iter().next() {
-        None => Ok(()),
-        Some((ix, item)) => {
-            let mut msg = format!("{} is not expected in this context", item);
-            if let Some(Conflict::Conflicts(acc, rej)) = args.conflicts.get(&ix) {
-                use std::fmt::Write;
-                write!(msg, ": {} cannot be used at the same time as {}", rej, acc).unwrap();
-            }
-            Err(Error::Message(msg, false))
-        }
+pub(crate) fn check_conflicts(args: &Args) -> Option<String> {
+    let (ix, _item) = args.items_iter().next()?;
+    if let Conflict::Conflicts(acc, rej) = args.conflicts.get(&ix)? {
+        Some(format!(
+            "{} cannot be used at the same time as {}",
+            rej, acc
+        ))
+    } else {
+        None
     }
 }
 
@@ -255,21 +253,16 @@ impl<T> OptionParser<T> {
             return Err(ParseFailure::Stdout(comp));
         }
 
-        let err = match res {
-            Ok(r) => {
-                if let Err(err) = check_unexpected(args) {
-                    err
-                } else {
-                    return Ok(r);
-                }
+        if args.is_empty() {
+            if let Ok(ok) = res {
+                return Ok(ok);
             }
-            Err(err) => err,
-        };
+        }
         Err((args.improve_error.0)(
             args,
             &self.info,
             &self.inner.meta(),
-            err,
+            res.err(),
         ))
     }
 
@@ -585,19 +578,6 @@ impl<T> OptionParser<T> {
     }
 }
 
-impl From<Error> for ParseFailure {
-    fn from(value: Error) -> Self {
-        match value {
-            Error::Message(msg, _) => ParseFailure::Stderr(msg),
-            Error::ParseFailure(pf) => pf,
-            Error::Missing(items) => ParseFailure::Stderr(format!(
-                "Expected {}, pass --help for usage information",
-                Meta::Or(items.into_iter().map(Meta::from).collect::<Vec<_>>())
-            )),
-        }
-    }
-}
-
 /// do a nested invariant check
 
 /// the check itself is performed as part of `to_usage_meta` transformation `fresh` parameter
@@ -617,7 +597,7 @@ fn perform_invariant_check(meta: &Meta, fresh: bool) {
             perform_invariant_check(x, false);
         }
         Meta::Item(i) => match &**i {
-            Item::Command { meta, .. } => perform_invariant_check(&meta, true),
+            Item::Command { meta, .. } => perform_invariant_check(meta, true),
             Item::Positional { .. }
             | Item::Flag { .. }
             | Item::Argument { .. }
diff --git a/src/meta_help.rs b/src/meta_help.rs
index 5961abe..231a7e3 100644
--- a/src/meta_help.rs
+++ b/src/meta_help.rs
@@ -221,7 +221,7 @@ impl<'a> HelpItems<'a> {
                 xs.push(HelpItem::Decor {
                     help,
                     margin: DecorPlace::Suffix,
-                })
+                });
             }
             Meta::Skip => (),
         }
diff --git a/src/meta_youmean.rs b/src/meta_youmean.rs
index 2146b6f..be15b41 100644
--- a/src/meta_youmean.rs
+++ b/src/meta_youmean.rs
@@ -2,37 +2,12 @@ use crate::{
     args::Arg,
     item::{Item, ShortLong},
     meta_help::{Long, Short},
-    Args, Error, Meta,
+    Args, Meta,
 };
 
-pub(crate) fn should_suggest(err: &Error) -> bool {
-    match err {
-        Error::Message(_, _) => true,
-        Error::ParseFailure(_) => false,
-        Error::Missing(xs) => {
-            let mut hi = crate::meta_help::HelpItems::default();
-            for x in xs.iter() {
-                hi.classify_item(x);
-            }
-            hi.flgs.is_empty() && hi.psns.is_empty()
-        }
-    }
-}
-
 /// Looks for potential typos
 pub(crate) fn suggest(args: &Args, meta: &Meta) -> Option<String> {
-    let arg = match args.peek() {
-        Some(arg) => arg,
-        None => return None,
-    };
-
-    if args.items.iter().filter(|&a| a == arg).count() > 1 {
-        // args contains more than one copy of unexpected item. Either user specified
-        // several of those or parser accepts only limited number of them.
-        // Or a different branch handles them. Give up and produce a default
-        // "not expected in this context" error
-        return None;
-    }
+    let arg = args.peek()?;
 
     let mut variants = Vec::new();
     collect_suggestions(arg, meta, &mut variants, true);
@@ -47,7 +22,7 @@ pub(crate) fn suggest(args: &Args, meta: &Meta) -> Option<String> {
                 w_flag!(cmd),
             );
             return Some(best);
-        } else if l > 0 {
+        } else if l > 0 && l < usize::MAX {
             let best = format!(
                 "No such {:?}, did you mean `{}`?",
                 actual,
@@ -76,7 +51,7 @@ impl std::fmt::Debug for I<'_> {
             Self::ShortFlag(s) => write!(f, "flag: `{}`", w_err!(Short(*s))),
             Self::LongFlag(s) => write!(f, "flag: `{}`", w_err!(Long(s))),
             Self::ShortCmd(s) => write!(f, "command alias: `{}`", w_err!(s)),
-            Self::LongCmd(s) => write!(f, "command: `{}`", w_err!(s)),
+            Self::LongCmd(s) => write!(f, "command or positional: `{}`", w_err!(s)),
             Self::Ambiguity(s) => write!(f, "flag: {} (with one dash)", w_err!(s)),
             Self::Nested(s) => write!(f, "command {}", w_err!(s)),
         }
@@ -98,7 +73,7 @@ impl std::fmt::Display for I<'_> {
 
 fn ins<'a>(expected: I<'a>, actual: I<'a>, variants: &mut Vec<(usize, (I<'a>, I<'a>))>) {
     variants.push((
-        levenshtein(&expected.to_string(), &actual.to_string()),
+        damerau_levenshtein(&expected.to_string(), &actual.to_string()),
         (actual, expected),
     ));
 }
@@ -177,39 +152,50 @@ fn collect_suggestions<'a>(
     }
 }
 
-fn levenshtein(a: &str, b: &str) -> usize {
-    let mut result = 0;
-    let mut cache = a.chars().enumerate().map(|i| i.0 + 1).collect::<Vec<_>>();
-    let mut distance_a;
-    let mut distance_b;
-
-    for (index_b, code_b) in b.chars().enumerate() {
-        result = index_b;
-        distance_a = index_b;
+/// Damerau-Levenshtein distance function
+///
+/// returns `usize::MAX` if there's no common characters at all mostly to avoid
+/// confusing error messages - "you typed 'foo', maybe you ment 'bar'" where
+/// 'foo' and 'bar' don't have anything in common
+fn damerau_levenshtein(a: &str, b: &str) -> usize {
+    #![allow(clippy::many_single_char_names)]
+    let a_len = a.chars().count();
+    let b_len = b.chars().count();
+    let mut d = vec![0; (a_len + 1) * (b_len + 1)];
+
+    let ix = |ib, ia| a_len * ia + ib;
+
+    for i in 0..=a_len {
+        d[ix(i, 0)] = i;
+    }
 
-        for (index_a, code_a) in a.chars().enumerate() {
-            distance_b = if code_a == code_b {
-                distance_a
-            } else {
-                distance_a + 1
-            };
+    for j in 0..=b_len {
+        d[ix(0, j)] = j;
+    }
 
-            distance_a = cache[index_a];
+    let mut pa = '\0';
+    let mut pb = '\0';
+    for (i, ca) in a.chars().enumerate() {
+        let i = i + 1;
+        for (j, cb) in b.chars().enumerate() {
+            let j = j + 1;
+            let cost = usize::from(ca != cb);
+            d[ix(i, j)] = (d[ix(i - 1, j)] + 1)
+                .min(d[ix(i, j - 1)] + 1)
+                .min(d[ix(i - 1, j - 1)] + cost);
+            if i > 1 && j > 1 && ca == pb && cb == pa {
+                d[ix(i, j)] = d[ix(i, j)].min(d[ix(i - 2, j - 2)] + 1);
+            }
+            pb = cb;
+        }
+        pa = ca;
+    }
 
-            result = if distance_a > result {
-                if distance_b > result {
-                    result + 1
-                } else {
-                    distance_b
-                }
-            } else if distance_b > distance_a {
-                distance_a + 1
-            } else {
-                distance_b
-            };
+    let diff = d[ix(a_len, b_len)];
 
-            cache[index_a] = result;
-        }
+    if diff >= a_len.max(b_len) {
+        usize::MAX
+    } else {
+        diff
     }
-    result
 }
diff --git a/src/structs.rs b/src/structs.rs
index db7885b..594d966 100644
--- a/src/structs.rs
+++ b/src/structs.rs
@@ -405,6 +405,7 @@ where
 impl<P, T: std::fmt::Display> ParseFallback<P, T> {
     /// Show [`fallback`](Parser::fallback) value in `--help` using [`Display`](std::fmt::Display)
     /// representation
+    #[must_use]
     pub fn display_fallback(mut self) -> Self {
         self.value_str = format!("[default: {}]", self.value);
         self
@@ -414,6 +415,7 @@ impl<P, T: std::fmt::Display> ParseFallback<P, T> {
 impl<P, T: std::fmt::Debug> ParseFallback<P, T> {
     /// Show [`fallback`](Parser::fallback) value in `--help` using [`Debug`](std::fmt::Debug)
     /// representation
+    #[must_use]
     pub fn debug_fallback(mut self) -> Self {
         self.value_str = format!("[default: {:?}]", self.value);
         self
@@ -831,9 +833,6 @@ where
     P: Parser<T> + Sized,
 {
     fn eval(&self, args: &mut Args) -> Result<T, Error> {
-        #[cfg(feature = "autocomplete")]
-        let mut comp_items = Vec::new();
-
         fn meta_items(meta: &Meta) -> Vec<Item> {
             match meta {
                 Meta::And(xs) => {
@@ -851,11 +850,14 @@ where
                     res
                 }
                 Meta::Item(i) => vec![*i.clone()],
-                Meta::Optional(m) | Meta::Many(m) | Meta::Decorated(m, _, _) => meta_items(&*m),
+                Meta::Optional(m) | Meta::Many(m) | Meta::Decorated(m, _, _) => meta_items(m),
                 Meta::Skip | Meta::HideUsage(_) => Vec::new(),
             }
         }
 
+        #[cfg(feature = "autocomplete")]
+        let mut comp_items = Vec::new();
+
         let mut best_missing = meta_items(&self.meta());
 
         let mut best_consumed = 0;
@@ -871,6 +873,7 @@ where
             // without multi step approach first command line also parses into 42
 
             let mut scratch = this_arg.clone();
+            #[allow(clippy::range_plus_one)] // inclusive range is the wrong type
             scratch.restrict_to_range(&(start..start + 1));
             let before = scratch.len();
             // nothing to consume, might as well stop right now


Test patch:
diff --git a/tests/adjacent.rs b/tests/adjacent.rs
index 1e0faf1..83f5a3c 100644
--- a/tests/adjacent.rs
+++ b/tests/adjacent.rs
@@ -66,7 +66,10 @@ fn adjacent_error_message_arg_single() {
         .run_inner(Args::from(&["-a", "10"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "Expected -b B, pass --help for usage information");
+    assert_eq!(
+        r,
+        "Expected -b B, got \"10\". Pass --help for usage information"
+    );
 }
 
 #[test]
@@ -82,7 +85,10 @@ fn adjacent_error_message_pos_many() {
         .run_inner(Args::from(&["-a", "10"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "Expected <C>, pass --help for usage information");
+    assert_eq!(
+        r,
+        "Expected <C>, got \"-a\". Pass --help for usage information"
+    );
 }
 
 #[test]
@@ -98,5 +104,8 @@ fn adjacent_error_message_arg_many() {
         .run_inner(Args::from(&["-a", "10"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "Expected -b B, pass --help for usage information");
+    assert_eq!(
+        r,
+        "Expected -b B, got \"-a\". Pass --help for usage information"
+    );
 }
diff --git a/tests/anywhere.rs b/tests/anywhere.rs
index 7ebef38..91de4a7 100644
--- a/tests/anywhere.rs
+++ b/tests/anywhere.rs
@@ -12,7 +12,10 @@ fn parse_anywhere_no_catch() {
         .run_inner(Args::from(&["-a"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "Expected <x>, pass --help for usage information");
+    assert_eq!(
+        r,
+        "Expected <x>, got \"-a\". Pass --help for usage information"
+    );
 
     let r = parser
         .run_inner(Args::from(&["-a", "221b"]))
@@ -24,7 +27,10 @@ fn parse_anywhere_no_catch() {
         .run_inner(Args::from(&["-c", "-a"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "Expected <x>, pass --help for usage information");
+    assert_eq!(
+        r,
+        "Expected <x>, got \"-c\". Pass --help for usage information"
+    );
 
     let r = parser
         .run_inner(Args::from(&["-c", "-a", "221b"]))
@@ -48,7 +54,10 @@ fn parse_anywhere_no_catch() {
         .run_inner(Args::from(&["3", "-a"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "No such command: `3`, did you mean `-c`?");
+    assert_eq!(
+        r,
+        "Expected <x>, got \"3\". Pass --help for usage information"
+    );
 }
 
 #[test]
@@ -63,43 +72,64 @@ fn parse_anywhere_catch_required() {
         .run_inner(Args::from(&["-a"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "Expected <x>, pass --help for usage information");
+    assert_eq!(
+        r,
+        "Expected <x>, got \"-a\". Pass --help for usage information"
+    );
 
     let r = parser
         .run_inner(Args::from(&["-a", "221b"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "Expected -a x, pass --help for usage information");
+    assert_eq!(
+        r,
+        "Expected -a x, got \"-a\". Pass --help for usage information"
+    );
 
     let r = parser
         .run_inner(Args::from(&["-c", "-a"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "Expected <x>, pass --help for usage information");
+    assert_eq!(
+        r,
+        "Expected <x>, got \"-c\". Pass --help for usage information"
+    );
 
     let r = parser
         .run_inner(Args::from(&["-c", "-a", "221b"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "Expected -a x, pass --help for usage information");
+    assert_eq!(
+        r,
+        "Expected -a x, got \"-c\". Pass --help for usage information"
+    );
 
     let r = parser
         .run_inner(Args::from(&["-a", "-c"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "Expected -a x, pass --help for usage information");
+    assert_eq!(
+        r,
+        "Expected -a x, got \"-a\". Pass --help for usage information"
+    );
 
     let r = parser
         .run_inner(Args::from(&["-a", "221b", "-c"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "Expected -a x, pass --help for usage information");
+    assert_eq!(
+        r,
+        "Expected -a x, got \"-a\". Pass --help for usage information"
+    );
 
     let r = parser
         .run_inner(Args::from(&["3", "-a"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "No such command: `3`, did you mean `-c`?");
+    assert_eq!(
+        r,
+        "Expected <x>, got \"3\". Pass --help for usage information"
+    );
 }
 
 #[test]
@@ -150,5 +180,5 @@ fn parse_anywhere_catch_optional() {
         .run_inner(Args::from(&["3", "-a"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "No such command: `3`, did you mean `-c`?");
+    assert_eq!(r, "3 is not expected in this context");
 }
diff --git a/tests/errors.rs b/tests/errors.rs
index ad7f55c..37111f5 100644
--- a/tests/errors.rs
+++ b/tests/errors.rs
@@ -14,31 +14,25 @@ fn better_error_with_enum() {
         .run_inner(Args::from(&["--alpha", "--beta"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(
-        res,
-        "--beta is not expected in this context: --beta cannot be used at the same time as --alpha"
-    );
+    assert_eq!(res, "--beta cannot be used at the same time as --alpha");
 
     let res = foo()
         .run_inner(Args::from(&["--alpha", "--gamma"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(res, "--gamma is not expected in this context: --gamma cannot be used at the same time as --alpha");
+    assert_eq!(res, "--gamma cannot be used at the same time as --alpha");
 
     let res = foo()
         .run_inner(Args::from(&["--beta", "--gamma"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(res, "--gamma is not expected in this context: --gamma cannot be used at the same time as --beta");
+    assert_eq!(res, "--gamma cannot be used at the same time as --beta");
 
     let res = foo()
         .run_inner(Args::from(&["--alpha", "--beta", "--gamma"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(
-        res,
-        "--beta is not expected in this context: --beta cannot be used at the same time as --alpha"
-    );
+    assert_eq!(res, "--beta cannot be used at the same time as --alpha");
 }
 
 #[test]
diff --git a/tests/meta_youmean.rs b/tests/meta_youmean.rs
index 36d3741..9a7e117 100644
--- a/tests/meta_youmean.rs
+++ b/tests/meta_youmean.rs
@@ -1,3 +1,5 @@
+#![allow(dead_code)]
+
 use bpaf::*;
 
 #[test]
@@ -41,7 +43,10 @@ fn short_cmd() {
         .unwrap_err()
         .unwrap_stderr();
 
-    assert_eq!(r, "No such command: `c`, did you mean `b`?");
+    assert_eq!(
+        r,
+        "No such command or positional: `c`, did you mean `beta`?"
+    );
 }
 
 #[test]
@@ -60,10 +65,9 @@ fn double_dashes_no_fallback() {
         .unwrap_err()
         .unwrap_stderr();
 
-    // TODO: can we point out at -llvm here?
     assert_eq!(
         r,
-        "Expected (--llvm | --att), pass --help for usage information"
+        "No such flag: -llvm (with one dash), did you mean `--llvm`?"
     );
 }
 
@@ -114,8 +118,6 @@ fn double_dash_with_optional_positional() {
 
 #[test]
 fn inside_out_command_parser() {
-    #![allow(dead_code)]
-
     #[derive(Debug, Bpaf, Clone, PartialEq)]
     #[bpaf(options)]
     enum Cmd {
@@ -137,3 +139,85 @@ fn inside_out_command_parser() {
         .unwrap_stderr();
     assert_eq!(r, "flag: `--oneline` is not valid in this context, did you mean to pass it to command \"log\"?");
 }
+
+#[test]
+fn flag_specified_twice() {
+    let parser = long("flag").switch().to_options();
+
+    let r = parser
+        .run_inner(Args::from(&["--flag", "--flag"]))
+        .unwrap_err()
+        .unwrap_stderr();
+    assert_eq!(r, "--flag is not expected in this context");
+}
+
+#[test]
+fn ux_discussion() {
+    #[derive(Debug, Clone, Bpaf)]
+    #[bpaf(adjacent)]
+    pub struct ConfigSetBool {
+        /// Set <key> to <bool>
+        #[bpaf(long("setBool"))]
+        set_bool: (),
+        /// Configuration key
+        #[bpaf(positional("key"))]
+        key: String,
+        /// Configuration Value (bool)
+        #[bpaf(positional("bool"))]
+        value: bool,
+    }
+
+    let aa = long("bool-flag").switch();
+    let parser = construct!(config_set_bool(), aa).to_options();
+
+    let r = parser
+        .run_inner(Args::from(&["--setBool", "key", "tru"]))
+        .unwrap_err()
+        .unwrap_stderr();
+    assert_eq!(
+        r,
+        // everything before ":" comes from bpaf, after ":" - it's an error specific
+        // to FromStr instance.
+        "Couldn't parse \"tru\": provided string was not `true` or `false`"
+    );
+
+    let r = parser
+        .run_inner(Args::from(&["--bool-fla"]))
+        .unwrap_err()
+        .unwrap_stderr();
+
+    assert_eq!(r, "No such flag: `--bool-fla`, did you mean `--bool-flag`?");
+
+    let r = parser
+        .run_inner(Args::from(&["--bool-flag", "--bool-flag"]))
+        .unwrap_err()
+        .unwrap_stderr();
+
+    assert_eq!(
+        r,
+        "Expected --setBool, got \"--bool-flag\". Pass --help for usage information"
+    );
+}
+
+#[test]
+fn suggest_typo_fix() {
+    let p = long("flag").switch().to_options();
+
+    let r = p
+        .run_inner(Args::from(&["--fla"]))
+        .unwrap_err()
+        .unwrap_stderr();
+    assert_eq!(r, "No such flag: `--fla`, did you mean `--flag`?");
+
+    let r = p
+        .run_inner(Args::from(&["--fla", "--fla"]))
+        .unwrap_err()
+        .unwrap_stderr();
+    assert_eq!(r, "No such flag: `--fla`, did you mean `--flag`?");
+
+    let r = p
+        .run_inner(Args::from(&["--flag", "--flag"]))
+        .unwrap_err()
+        .unwrap_stderr();
+    assert_eq!(r, "--flag is not expected in this context");
+}
diff --git a/tests/tests.rs b/tests/tests.rs
index 53696a0..6e07934 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -337,7 +337,7 @@ fn parse_errors() {
         .run_inner(Args::from(&["-b", "123x"]))
         .unwrap_err()
         .unwrap_stderr();
-    let expected_err = "Expected -a ARG, pass --help for usage information";
+    let expected_err = "No such flag: `-b`, did you mean `-a`?";
     assert_eq!(expected_err, err);
 
     let err = decorated
@@ -502,46 +502,31 @@ fn from_several_alternatives_pick_more_meaningful() {
         .run_inner(Args::from(&["-a", "-b"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(
-        err1,
-        "-b is not expected in this context: -b cannot be used at the same time as -a"
-    );
+    assert_eq!(err1, "-b cannot be used at the same time as -a");
 
     let err2 = parser
         .run_inner(Args::from(&["-b", "-a"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(
-        err2,
-        "-a is not expected in this context: -a cannot be used at the same time as -b"
-    );
+    assert_eq!(err2, "-a cannot be used at the same time as -b");
 
     let err3 = parser
         .run_inner(Args::from(&["-c", "-a"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(
-        err3,
-        "-a is not expected in this context: -a cannot be used at the same time as -c"
-    );
+    assert_eq!(err3, "-a cannot be used at the same time as -c");
 
     let err4 = parser
         .run_inner(Args::from(&["-a", "-c"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(
-        err4,
-        "-c is not expected in this context: -c cannot be used at the same time as -a"
-    );
+    assert_eq!(err4, "-c cannot be used at the same time as -a");
 
     let err5 = parser
         .run_inner(Args::from(&["-c", "-b", "-a"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(
-        err5,
-        "-b is not expected in this context: -b cannot be used at the same time as -c"
-    );
+    assert_eq!(err5, "-b cannot be used at the same time as -c");
 }
 
 #[test]
@@ -1254,7 +1239,10 @@ fn did_you_mean_switch() {
         .run_inner(Args::from(&["flag"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(res, "No such command: `flag`, did you mean `--flag`?");
+    assert_eq!(
+        res,
+        "No such command or positional: `flag`, did you mean `--flag`?"
+    );
 
     let res = parser
         .run_inner(Args::from(&["--pla"]))
@@ -1276,7 +1264,7 @@ fn did_you_mean_req_flag() {
         .run_inner(Args::from(&["--fla"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(res, "Expected --flag, pass --help for usage information");
+    assert_eq!(res, "No such flag: `--fla`, did you mean `--flag`?");
 }
 
 #[test]
@@ -1287,19 +1275,13 @@ fn did_you_mean_argument() {
         .run_inner(Args::from(&["--fla"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(
-        res,
-        "Expected --flag VAL, pass --help for usage information"
-    );
+    assert_eq!(res, "No such flag: `--fla`, did you mean `--flag`?");
 
     let res = parser
         .run_inner(Args::from(&["--flg=hellop"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(
-        res,
-        "Expected --flag VAL, pass --help for usage information"
-    );
+    assert_eq!(res, "No such flag: `--flg`, did you mean `--flag`?");
 }
 
 #[test]
@@ -1314,7 +1296,10 @@ fn did_you_mean_command() {
         .run_inner(Args::from(&["comman"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(res, "No such command: `comman`, did you mean `command`?");
+    assert_eq!(
+        res,
+        "No such command or positional: `comman`, did you mean `command`?"
+    );
 
     let res = parser
         .run_inner(Args::from(&["--comman"]))
@@ -1382,13 +1367,19 @@ fn did_you_mean_two_or_arguments() {
         .run_inner(Args::from(&["--parameter", "--flag"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "--flag is not expected in this context: [--flag] cannot be used at the same time as [--parameter]");
+    assert_eq!(
+        r,
+        "[--flag] cannot be used at the same time as [--parameter]"
+    );
 
     let r = parser
         .run_inner(Args::from(&["--flag", "--parameter"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "--parameter is not expected in this context: [--parameter] cannot be used at the same time as [--flag]");
+    assert_eq!(
+        r,
+        "[--parameter] cannot be used at the same time as [--flag]"
+    );
 }
 
 // problematic steps look something like this:
@@ -1499,7 +1490,10 @@ fn did_you_mean_inside_command() {
         .run_inner(Args::from(&["--fla"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "No such flag: `--fla`, did you mean `cmd`?");
+    assert_eq!(
+        r,
+        "Expected COMMAND ..., got \"--fla\". Pass --help for usage information"
+    );
 
     let r = parser
         .run_inner(Args::from(&["cmd", "--fla"]))
@@ -1523,13 +1517,19 @@ fn did_you_mean_inside_command() {
         .run_inner(Args::from(&["cmd", "--parameter", "--flag"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "--flag is not expected in this context: [--flag] cannot be used at the same time as [--parameter]");
+    assert_eq!(
+        r,
+        "[--flag] cannot be used at the same time as [--parameter]"
+    );
 
     let r = parser
         .run_inner(Args::from(&["cmd", "--flag", "--parameter"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "--parameter is not expected in this context: [--parameter] cannot be used at the same time as [--flag]");
+    assert_eq!(
+        r,
+        "[--parameter] cannot be used at the same time as [--flag]"
+    );
 }
 
 #[test]
@@ -1638,7 +1638,7 @@ fn reject_fbar() {
         .run_inner(Args::from(&["-fbar", "baz"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "No such command: `baz`, did you mean `-f`?");
+    assert_eq!(r, "baz is not expected in this context");
 
     let r = parser.run_inner(Args::from(&["-fbar"])).unwrap();
     assert_eq!(r, "bar");
@@ -1908,7 +1908,11 @@ fn option_requires_other_option1() {
         .run_inner(Args::from(&["-a"]))
         .unwrap_err()
         .unwrap_stderr();
-    assert_eq!(r, "Expected -b B, pass --help for usage information");
+    // error message sucks...
+    assert_eq!(
+        r,
+        "Expected -b B, got \"-a\". Pass --help for usage information"
+    );
 }
 
 #[test]