diff --git a/apps/figcraft-cli/src/args.rs b/apps/figcraft-cli/src/args.rs index c224f9a7..3fe17e80 100644 --- a/apps/figcraft-cli/src/args.rs +++ b/apps/figcraft-cli/src/args.rs @@ -16,6 +16,8 @@ pub struct Args { impl Args { pub fn parse(argv: &[String], valued: &[&str], flags: &[&str]) -> CliResult { let mut a = Args::default(); + // The first bad option, reported once the rest of the line is checked for `--help`. + let mut bad: Option = None; let mut it = argv.iter(); while let Some(arg) = it.next() { let name = if let Some(n) = arg.strip_prefix("--") { @@ -35,18 +37,22 @@ impl Args { return Err(CliError::Help); } if valued.contains(&name) { - let v = match inline { - Some(v) => v, - None => it.next().cloned().ok_or_else(|| CliError::Usage(format!("`{arg}` needs a value")))?, - }; - a.values.entry(name.to_string()).or_default().push(v); + match inline.or_else(|| it.next().cloned()) { + Some(v) => a.values.entry(name.to_string()).or_default().push(v), + None => { + bad.get_or_insert(CliError::Usage(format!("`{arg}` needs a value"))); + } + } } else if flags.contains(&name) && inline.is_none() { a.flags.push(name.to_string()); } else { - return Err(CliError::Usage(format!("unknown option `{arg}`"))); + bad.get_or_insert(CliError::Usage(format!("unknown option `{arg}`"))); } } - Ok(a) + match bad { + Some(e) => Err(e), + None => Ok(a), + } } pub fn value(&self, k: &str) -> Option { @@ -142,7 +148,10 @@ mod tests { #[test] fn help_is_asked_for_anywhere_but_as_a_value() { - for argv in [&["--help"][..], &["-h"], &["in.figcraft", "-o", "out", "--help"], &["--json", "-h"]] { + // Also after a bad option: help wins over the usage error (#1161 review). + for argv in + [&["--help"][..], &["-h"], &["in.figcraft", "-o", "out", "--help"], &["--json", "-h"], &["--bogus", "--help"], &["--json=1", "-h"]] + { assert!(matches!(Args::parse(&v(argv), &["o"], &["json"]), Err(CliError::Help)), "{argv:?}"); } let a = Args::parse(&v(&["--filter", "--help", "-o", "-h"]), &["filter", "o"], &[]).unwrap(); diff --git a/apps/figcraft-cli/tests/cli.rs b/apps/figcraft-cli/tests/cli.rs index 0d6fbe79..23580016 100644 --- a/apps/figcraft-cli/tests/cli.rs +++ b/apps/figcraft-cli/tests/cli.rs @@ -60,6 +60,7 @@ fn help_after_a_subcommand_prints_the_usage() { &["plugin", "publish", "--help"], &["perf", "--help"], &["mcp", "--attach", "notaport", "--help"], + &["mcp", "--bogus", "--help"], ] { let out = bin().args(args).stdin(Stdio::null()).output().unwrap(); assert_eq!(out.status.code(), Some(0), "{args:?}: {}", String::from_utf8_lossy(&out.stderr));