Merge #1161: M0.9: --help and -h after a figcraft-cli subcommand print the usage (#1155) (@switch87)

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<Self> {
         let mut a = Args::default();
+        // The first bad option, reported once the rest of the line is checked for `--help`.
+        let mut bad: Option<CliError> = 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<String> {
@@ -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(_))));
+    }
 }

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 <folder> --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<String> = 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.

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");

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