diff --git a/apps/figcraft-cli/src/args.rs b/apps/figcraft-cli/src/args.rs index 4c162dbb..3fe17e80 100644 --- a/apps/figcraft-cli/src/args.rs +++ b/apps/figcraft-cli/src/args.rs @@ -1,6 +1,6 @@ //! Minimal std-only argument parsing: positionals, `--key value` / `--key=value` / `-o value` //! options (given again: the last counts, or all with [`Args::values`]), and boolean `--flag`s. -//! Unknown options are usage errors. +//! Unknown options are usage errors; `--help` or `-h` is [`CliError::Help`]. use std::collections::BTreeMap; @@ -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("--") { @@ -30,19 +32,27 @@ impl Args { Some((n, v)) => (n, Some(v.to_string())), None => (name, None), }; + // `--help` / `-h` anywhere among the options (not as an option's value) asks for help. + if inline.is_none() && matches!(name, "help" | "h") { + 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 { @@ -135,4 +145,18 @@ mod tests { let a = Args::parse(&v(&["-5"]), &[], &[]).unwrap(); assert_eq!(a.positional, ["-5"]); } + + #[test] + fn help_is_asked_for_anywhere_but_as_a_value() { + // 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(); + assert_eq!((a.value("filter").as_deref(), a.value("o").as_deref()), (Some("--help"), Some("-h"))); + // `--help=x` is no request for help, just an unknown option. + assert!(matches!(Args::parse(&v(&["--help=x"]), &[], &[]), Err(CliError::Usage(_)))); + } } diff --git a/apps/figcraft-cli/src/main.rs b/apps/figcraft-cli/src/main.rs index badc5bc3..65fd3aa8 100644 --- a/apps/figcraft-cli/src/main.rs +++ b/apps/figcraft-cli/src/main.rs @@ -27,7 +27,7 @@ USAGE: figcraft-cli mcp [--attach PORT] [--automation-read-root DIR]... [--automation-write-root DIR]... figcraft-cli plugin publish --server ws://host:port/[?key=KEY] figcraft-cli perf [--layers N[,N...]] [--instances N[,N...]] - figcraft-cli help | --help | --version + figcraft-cli help | --help | --version (a subcommand followed by --help or -h prints this too) Every subcommand takes --no-system-fonts (or set FIGCRAFT_NO_SYSTEM_FONTS=1): text uses only the fonts built into FigCraft, for renders that match on every machine. Otherwise installed fonts are @@ -74,6 +74,10 @@ fn main() -> ExitCode { let argv: Vec = std::env::args().skip(1).collect(); match run(&argv) { Ok(code) => code, + Err(CliError::Help) => { + println!("{USAGE}"); + ExitCode::SUCCESS + } Err(CliError::Usage(msg)) => { eprintln!("error: {msg}\n\n{USAGE}"); ExitCode::from(2) @@ -87,6 +91,8 @@ fn main() -> ExitCode { #[derive(Debug)] pub enum CliError { + /// `--help` or `-h` after a subcommand: usage on stdout, exit code 0. + Help, /// Bad arguments: usage is printed, exit code 2. Usage(String), /// The operation failed: exit code 1. diff --git a/apps/figcraft-cli/tests/it/cli.rs b/apps/figcraft-cli/tests/it/cli.rs index 4af73d69..be19a091 100644 --- a/apps/figcraft-cli/tests/it/cli.rs +++ b/apps/figcraft-cli/tests/it/cli.rs @@ -39,6 +39,40 @@ fn bad_arguments_exit_2_without_panicking() { assert_eq!(out.status.code(), Some(1)); } +/// `--help` / `-h` after a subcommand prints the usage on stdout and exits 0, as it does first +/// (#1155), whatever else is on the line; nothing runs. +#[test] +fn help_after_a_subcommand_prints_the_usage() { + for args in [ + &["--help"][..], + &["-h"], + &["help"], + &["mcp", "--help"], + &["mcp", "-h"], + &["info", "--help"], + &["export", "--help"], + &["export", "x.figcraft", "-o", "out", "--help"], + &["convert", "-h"], + &["render", "--help"], + &["run", "--help"], + &["commands", "--help"], + &["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)); + let text = String::from_utf8_lossy(&out.stdout); + assert!(text.contains("USAGE") && text.contains("figcraft-cli mcp"), "{args:?}: {text}"); + assert!(out.stderr.is_empty(), "{args:?}: nothing on stderr, got {}", String::from_utf8_lossy(&out.stderr)); + } + // A value that happens to be `-h` is still a value, not a request for help. + let out = bin().args(["commands", "--filter", "--help"]).output().unwrap(); + assert_eq!(out.status.code(), Some(0)); + assert!(!String::from_utf8_lossy(&out.stdout).contains("USAGE"), "--filter took `--help` as its text"); +} + #[test] fn run_export_info_convert() { let d = tmp("run"); diff --git a/docs/cli.md b/docs/cli.md index 699b6ba2..0975f4fd 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -4,7 +4,9 @@ FigCraft without a window: inspect, export, convert and render documents, run co commands, serve MCP and measure performance. Build it with `cargo build --release -p figcraft-cli` (binary `target/release/figcraft-cli`). -Bad arguments print usage and exit with code 2; failures exit with 1. The CLI never panics on input. +`figcraft-cli --help` (or `-h`, or `help`) prints usage on stdout and exits with 0, and so does +`--help` or `-h` after a subcommand (`figcraft-cli mcp --help`). Bad arguments print usage and exit +with code 2; failures exit with 1. The CLI never panics on input. ## Fonts