fix: report "requires an argument" instead of "no such flag" for missing values

[tonky]

When a flag that takes an argument is provided without its value (e.g. --name instead of --name foo), bpaf could produce a misleading "no such flag" error with a typo suggestion instead of the correct "requires an argument" message.

This happened in two scenarios:

1. or_else/enum variant chains: when one branch matched the flag but failed with NoArgument, and another branch with all-optional fields succeeded, the successful branch was picked leaving the flag unconsumed. The unconsumed flag then triggered a "no such flag" suggestion.

2. optional().catch(): catch swallowed the NoArgument error, leaving the flag unconsumed with the same misleading result.

Fix: in or_else, propagate NoArgument when the winning branch left the flag unconsumed and didn't consume more items overall. In parse_option, never let catch swallow NoArgument since the user explicitly provided the flag.

Ref: flox/flox#3411, related to #350

[pacak]

I appreciate the bug report and the fix looks reasonable. Let me poke around for a bit, I'll see if there are any cleaner solutions. If not - I'll merge this and release a new version in a day or two. And I guess it's time to bump older toolchain test to something more recent.

[tonky]

There was another fix suggested, with which these tests are passing, but i have neither context nor understanding of code to judge which one is more correct.

Maybe it'll be of some use to you:

diff --git a/src/structs.rs b/src/structs.rs
index 37a55b5..9885749 100644
--- a/src/structs.rs
+++ b/src/structs.rs
@@ -817,8 +817,9 @@ where
             // anything left unconsumed - this won't be lost.
 
             let missing = matches!(err, Message::Missing(_));
+            let no_argument = matches!(err, Message::NoArgument(_, _));
 
-            if catch || (missing && orig_args.len() == args.len()) || (!missing && err.can_catch())
+            if (catch && !no_argument) || (missing && orig_args.len() == args.len()) || (!missing && err.can_catch())
             {
                 std::mem::swap(&mut orig_args, args);
                 #[cfg(feature = "autocomplete")]

[pacak]

I have a slightly more general solution - looking not at this error specifically but treating this_or_that as a some sort of fallback and allowing suppressing only the errors that can be caught if the alternative is a branch that didn't consume anything. Ran out of time experimenting. Hopefully tomorrow.

[pacak]

So I made this: #444

Main differences are:

1. I simplified the tests a bit - .map() doesn't add much value
2. Rather than looking at one specific error I'm looking for a class of them in a specific scenario - when the alternative succeeds without consuming anything. This matches the logic of the unreleased 0.10 more closely.
3. In .catch() I documented what errors it cannot catch, expanded the errors from a single one to a class and simplified the code somewhat.

Still, I appreciate the bugreport, the reproducer and test cases and a solution prototype. It made my work easier.

Are there any smallish bugs you have in mind? If not - I'll release whatever the next 0.9 version is and will go back working on 0.10 :)

[tonky]

awesome!
nothing else to report, thank you for your work!