pacak / bpaf

Command line parser with applicative interface
337 stars 17 forks source link

Missing hidden argument causes panic instead of error #345

Closed vallentin closed 5 months ago

vallentin commented 5 months ago

As the title says:

use bpaf::Bpaf;

fn main() {
    let args = args().run();
    println!("{:#?}", args);
}

#[derive(Bpaf, Clone, Debug)]
#[bpaf(options)]
pub struct Args {
    #[bpaf(long)]
    foo: String,
}

Executing cargo run results in:

Error: expected `--foo=ARG`, pass `--help` for usage information
error: process didn't exit successfully: `target\debug\issue.exe` (exit code: 1)

However, changing #[bpaf(long)] to #[bpaf(hide, long)] and then executing cargo run results in:

thread 'main' panicked at C:\Users\vallentin\.cargo\registry\src\index.crates.io-6f17d22bba15001f\bpaf-0.9.9\src\error.rs:601:10:
called `Option::unwrap()` on a `None` value
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
error: process didn't exit successfully: `target\debug\issue.exe` (exit code: 101)

I still expected an error, but not a panic.

I think a hidden argument, should be hidden from the help text, but included in error messages. Maybe rephrasing a bit, and saying expected hidden `--foo=ARG` ... or something similar.

pacak commented 5 months ago

Ouch. Fixed locally, new error message is going to say "parser requires an extra flag, argument or parameter, but its name is hidden by the author". I might add an error into invariant check - required but hidden argument seems very bad UX.

vallentin commented 5 months ago

I agree, I actually ran into it because of a "typo" in relation to the previous issue #344 I posted. I was doing something along the lines of #[bpaf(hide, env)], but then accidentally used field: T instead of field: Option<T>

pacak commented 5 months ago

Fixed in #349