From 731d1e6ecbd7d2fcb5fe0921f689cb814241defe Mon Sep 17 00:00:00 2001 From: Michael Baikov Date: Fri, 11 Aug 2023 07:39:49 -0400 Subject: [PATCH] Fancier short squashing support squashing short flags as well as short arguments with their bodies inside the same flag. --- Changelog.md | 3 ++ src/args.rs | 120 ++++++++++++++++++++++++++++-------------------- tests/errors.rs | 26 +++++++---- tests/params.rs | 25 ++++++++++ 4 files changed, 115 insertions(+), 59 deletions(-) diff --git a/Changelog.md b/Changelog.md index 5cd32793..a70d74fa 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,5 +1,8 @@ # Change Log +## bpaf [0.9.5] - Unreleased +- fancier squashing: parse `-abfoo` as `-a -b=foo` if b is a short argument + ## bpaf [0.9.4] - 2023-08-08 - add `help` to `ParseFlag` and `ParseArgument` - stop deprecating `Parser::run` diff --git a/src/args.rs b/src/args.rs index 1f701ce4..91924e12 100644 --- a/src/args.rs +++ b/src/args.rs @@ -180,6 +180,66 @@ impl ItemState { } } +fn disambiguate_short( + mut os: OsString, + short: String, + short_flags: &[char], + short_args: &[char], + items: &mut Vec, +) -> Option { + // block can start with 0 or more short flags + // followed by zero or one short argument, possibly with a body + + // keep the old length around so we can trimp items to it and push a Arg::Word + // if we decide to give up + let original = items.len(); + + // first flag contains the original os string for error message and anywhere purposes + let mut first_flag = os.clone(); + + for (ix, c) in short.char_indices() { + let tail_ix = ix + c.len_utf8(); + let rest = &short[tail_ix..]; + + // shortcircuit single character short options + if ix == 0 && rest.is_empty() { + items.push(Arg::Short(c, false, std::mem::take(&mut first_flag))); + return None; + } + match (short_flags.contains(&c), short_args.contains(&c)) { + // short name that can be flag + (true, false) => { + items.push(Arg::Short(c, false, std::mem::take(&mut first_flag))); + } + + // short name that can be argument + (false, true) => { + let adjacent_body = !rest.is_empty(); + items.push(Arg::Short(c, adjacent_body, std::mem::take(&mut os))); + if adjacent_body { + items.push(Arg::Word(rest.into())); + } + return None; + } + + // neither is valid and there's more than one character. fallback to using it as a Word + (false, false) => { + items.truncate(original); + items.push(Arg::Word(os)); + return None; + } + + // ambiguity, this is bad + (true, true) => { + let msg = Message::Ambiguity(items.len(), short); + items.push(Arg::Word(std::mem::take(&mut os))); + return Some(msg); + } + } + } + None +} + pub use inner::State; /// Hides [`State`] internal implementation mod inner { @@ -266,7 +326,7 @@ mod inner { name: args.name.as_deref(), }; - for mut os in args.items { + for os in args.items { if pos_only { items.push(Arg::PosWord(os)); continue; @@ -280,55 +340,15 @@ mod inner { match split_os_argument(&os) { // -f and -fbar, but also -vvvvv Some((ArgType::Short, short, None)) => { - // this scenario can be ambiguous: -fbar can mean one of - // several one char: -f -b -a -r - // a single short flag -f with attached value "bar" - // bpaf applies following logic: - // - it can be a collection of separate flags if all the names - // are valid flag name - // - it can be a short flag with a value if first character is - // a valid argument name - // - // if both variants are correct - we complain, if just one is correct we go - // with that option, otherwise this is a strange positional. - - let mut can_be_arg = true; - let mut can_be_flags = true; - let mut total = 0; - for (ix, c) in short.chars().enumerate() { - can_be_flags &= short_flags.contains(&c); - if ix == 0 { - can_be_arg = short_args.contains(&c); - } - total = ix; - } - // there's no body so - if total == 0 { - items.push(Arg::Short(short.chars().next().unwrap(), false, os)); - continue; - } - - match (can_be_flags, can_be_arg) { - (true, true) => { - *err = Some(Message::Ambiguity(items.len(), short)); - items.push(Arg::Word(os)); - - break; - } - (true, false) => { - for c in short.chars() { - // first gets the os, rest gets the empty - items.push(Arg::Short(c, false, std::mem::take(&mut os))); - } - } - (false, true) => { - let mut chars = short.chars(); - let first = chars.next().unwrap(); - let rest = chars.as_str(); - items.push(Arg::Short(first, true, os)); - items.push(Arg::ArgWord(rest.into())); - } - (false, false) => items.push(Arg::Word(os)), + if let Some(msg) = super::disambiguate_short( + os, + short, + short_flags, + short_args, + &mut items, + ) { + *err = Some(msg); + break; } } Some((ArgType::Short, short, Some(arg))) => { diff --git a/tests/errors.rs b/tests/errors.rs index 4d73328e..d433f04f 100644 --- a/tests/errors.rs +++ b/tests/errors.rs @@ -159,6 +159,17 @@ fn cannot_be_used_twice() { #[test] fn should_not_split_adjacent_options() { + let a = short('a').req_flag(0); + let b = pure(()).to_options().command("hello"); + let parser = construct!(a, b).to_options(); + let r = parser.run_inner(&["-ahello"]).unwrap_err().unwrap_stderr(); + // can probably suggest splitting here too: `-a` `hello` + let expected = "no such flag: `-ahello`, did you mean `hello`?"; + assert_eq!(r, expected); +} + +#[test] +fn should_not_split_adjacent_ambig_options() { let a1 = short('a').req_flag(0); let a2 = short('a').argument::("x"); let a = construct!([a2, a1]); @@ -166,18 +177,15 @@ fn should_not_split_adjacent_options() { let parser = construct!(a, c).to_options(); let r = parser.run_inner(&["-a=hello"]).unwrap_err().unwrap_stderr(); - assert_eq!( - r, - "expected `COMMAND ...`, got `hello`. Pass `--help` for usage information" - ); + let expected = "expected `COMMAND ...`, got `hello`. Pass `--help` for usage information"; + assert_eq!(r, expected); let r = parser.run_inner(&["-ahello"]).unwrap_err().unwrap_stderr(); - assert_eq!( - r, - "expected `COMMAND ...`, got `hello`. Pass `--help` for usage information" - ); + let expected = "app supports `-a` as both an option and an option-argument, try to split `-ahello` into individual options\n(-a -h ..) or use `-a=hello` syntax to disambiguate"; + assert_eq!(r, expected); - // this one is okay + // this one is okay, try to parse -a as argument - it fails because "hello" is not a number, then + // try to parse -a as a flag - this works let r = parser.run_inner(&["-a", "hello"]).unwrap(); assert_eq!(r, (0, ())); } diff --git a/tests/params.rs b/tests/params.rs index fb8c3d52..008d54be 100644 --- a/tests/params.rs +++ b/tests/params.rs @@ -77,3 +77,28 @@ fn from_str_works_with_parse() { let r = parser.run_inner(&["42"]).unwrap(); assert_eq!(r, 42); } + +#[test] +fn squashed_short_names() { + let a = short('a').switch(); + let b = short('b').argument::("B"); + let parser = construct!(a, b).to_options(); + + // For now instead of supporting I parse "-Obits=2048" instead as "-O" with "bits=2048" body + // https://github.com/pacak/bpaf/issues/121 + // + // let r = parser.run_inner(&["-ab=foo"]).unwrap(); + // assert_eq!(r.1, "foo"); + + // an odd one, short flag a is squashed with short argument b + let r = parser.run_inner(&["-ab", "foo"]).unwrap(); + assert_eq!(r.1, "foo"); + + // same, but body for b is adjacent as well + let r = parser.run_inner(&["-abfoo"]).unwrap(); + assert_eq!(r.1, "foo"); + + // normal + let r = parser.run_inner(&["-a", "-bfoo"]).unwrap(); + assert_eq!(r.1, "foo"); +}