pacak / bpaf

Command line parser with applicative interface
Apache License 2.0
354 stars 17 forks source link

Strict and regular many positionals conflict when parsing #373

Closed ozwaldorf closed 3 months ago

ozwaldorf commented 3 months ago

When a regular and strict many positional items are used at the same time, they conflict when parsing from the cli, and guards from a non-strict arg are used on the strict args, despite separation being used

For example this custom parser:

    fn extra_colors() -> impl Parser<Vec<Color>> {
        positional::<String>("COLORS")
            .help("Custom colors to use. Combines with a palette if provided.")
            .strict()
            .complete(|s| {
                let hex = s.trim_start_matches('#').to_string();
                if hex.len() == 3 {
                    vec![(
                        "#".chars()
                            .chain(hex.chars().flat_map(|a| [a, a]))
                            .collect::<String>(),
                        None,
                    )]
                } else {
                    vec![(s.clone(), None)]
                }
            })
            .parse(|s| Color::from_str(&s))
            .many()
    }

Used after a non-strict some positional with a guard:

        /// Images to correct, using the generated or provided hald clut.
        #[bpaf(
            positional("IMAGES"),
            guard(|v| v.exists(), "No such file or directory"),
            some("At least one image is needed to apply"),
        )]
        input: Vec<PathBuf>,
        #[bpaf(external(Color::extra_colors))]
        extra_colors: Vec<Color>,

With a command like so:

lutgen apply image.png -- dea  dbe  ef0
             ‾‾‾‾‾‾‾‾‾    ‾‾‾  ‾‾‾  ‾‾‾
               input         colors

The colors fail from the non-strict arg's guard:

error: dea: No such file or directory
pacak commented 3 months ago

Looks like it successfully parses everything as PathBuf - strict requires that items are prefixed with --, but regular positional also accepts those items. You have guard which performs the check - the only missing bit is to add a catch right after some to catch the error and stop trying to parse files.

        /// Images to correct, using the generated or provided hald clut.
        #[bpaf(
            positional("IMAGES"),
            guard(|v| v.exists(), "No such file or directory"),
            some("At least one image is needed to apply"),
            catch // <- this one
        )]
        input: Vec<PathBuf>,
        #[bpaf(external(Color::extra_colors))]
        extra_colors: Vec<Color>,
ozwaldorf commented 3 months ago

This does fix the error and allows the strict args to parse correctly, though when invalid non-strict items are passed (with or without the strict ones), the error gets lost, which somewhat defeats the purpose of the guard:

lutgen apply invalid/path.png
Error: At least one image is needed to apply

Could there be a way to explicitly say the first positional should stop parsing when it reaches a --, to be able to preserve the guard?

pacak commented 3 months ago

Not right now, but it should be easy to add - just a modifier that does the opposite of strict.