diff --git a/PLAN.md b/PLAN.md index 801a1593..4365bf2e 100644 --- a/PLAN.md +++ b/PLAN.md @@ -265,14 +265,14 @@ tasks --usage"`, so task names are meant to come from running that. usage-argv d ### Then: what a CLI framework has to have -- [x] **Help rendering** — `-h` and `--help` both match usage-lib byte for byte across all 211 - of mise's commands: the usage line, the descriptions, commands, arguments and flags grouped - by heading, examples, the surrounding text, hidden-item filtering, and for the long form the - column alignment and wrapping to `COLUMNS`. What is left is the wiring — `--help`/`-h` on - every command, `Error::Help`, and a real `help` subcommand. Holding it to parity found ten - things a spec could say that the derive could not, plus two bugs in usage-lib's own - renderer; the last of them was `after_long_help`, which 115 of mise's commands use to carry - their Examples section, so a page without it was missing what a reader came for. +- [x] **Help rendering** — `-h` and `--help` both match usage-lib byte for byte across all 211 of + mise's commands, and both are wired: the parser recognises them itself, after a command's own + flags so a CLI that declares its own keeps it, and a request comes back as `Error::Help` + carrying the command it was asked about. `parse()` renders and exits; `parse_from` returns + it, so a library embedding this decides for itself. Costs 111 instructions, since the check + is only reached by a flag that matched nothing. What is left is the `help` _subcommand_, + which every CLI with subcommands should have. Holding the rendering to parity found ten + things a spec could say that the derive could not, and two bugs in usage-lib's own renderer. - [ ] **Completions, self-contained** — ` completion ` emits the script; a hidden ` complete-word` serves requests from the binary's own embedded spec. Same dispatch shape usage-cli uses today, without requiring diff --git a/argv/src/help.rs b/argv/src/help.rs index ab014bc4..159c5b5d 100644 --- a/argv/src/help.rs +++ b/argv/src/help.rs @@ -20,6 +20,7 @@ use core::fmt::Write as _; use crate::spec::{ArgMeta, CommandMeta, Example, FlagMeta, Spec}; +use crate::Command; use crate::DoubleDash; /// How many flags or arguments are listed individually before collapsing to a placeholder. @@ -668,3 +669,49 @@ fn long_commands_section(out: &mut String, path: &[&str], meta: &CommandMeta<'_> " help\n Print this message or the help of the given subcommand(s)" ); } + +/// The path and metadata for a command, found by identity within a spec. +/// +/// [`Error::Help`](crate::Error::Help) carries the `Command` the request was about, because the +/// parse tables are what a parse walks and the metadata is behind a feature. Rendering needs the +/// metadata and the path a user typed to reach it, and both are in the tree — so this walks it, +/// comparing addresses rather than names, which two commands can share. +/// +/// `None` when the command is not in this spec, which means the two came from different CLIs. +pub fn find<'a>( + spec: &'a Spec<'a>, + cmd: &Command<'_>, +) -> Option<(Vec<&'a str>, &'a CommandMeta<'a>)> { + fn walk<'a>( + path: &mut Vec<&'a str>, + meta: &'a CommandMeta<'a>, + cmd: &Command<'_>, + ) -> Option<&'a CommandMeta<'a>> { + if core::ptr::eq(meta.cmd, cmd) { + return Some(meta); + } + for sub in meta.subcommands { + path.push(sub.cmd.name); + if let Some(found) = walk(path, sub, cmd) { + return Some(found); + } + path.pop(); + } + None + } + + let mut path = vec![spec.bin.unwrap_or(spec.name)]; + walk(&mut path, spec.root, cmd).map(|meta| (path, meta)) +} + +/// The page a help request asks for, ready to print. +/// +/// The two forms differ as clap has them: `-h` is the short one and `--help` the long one. +pub fn render(spec: &Spec<'_>, cmd: &Command<'_>, long: bool) -> Option { + let (path, meta) = find(spec, cmd)?; + Some(if long { + long_help(spec, &path, meta) + } else { + short_help(spec, &path, meta) + }) +} diff --git a/argv/src/lib.rs b/argv/src/lib.rs index 7a7a6027..85ebda00 100644 --- a/argv/src/lib.rs +++ b/argv/src/lib.rs @@ -387,6 +387,16 @@ pub enum Error<'t, 'v> { InvalidValue(::std::boxed::Box>), /// A subcommand was required, and none was given. MissingSubcommand, + /// `--help` or `-h` was given, and `cmd` is what it was asked about. + /// + /// Not a failure, and returned as one anyway: a parse that stops to print help has not + /// produced a value, and every caller already handles the "no value" shape. clap does the + /// same thing for the same reason. + /// + /// `long` distinguishes the two: `-h` prints the short form and `--help` the long one, as + /// clap has them. The caller renders — this crate does not print, because a library that + /// writes to stdout on its own is one an adopter cannot embed. + Help { cmd: &'t Command<'t>, long: bool }, } /// The high half of every key one declaration's items get. @@ -547,6 +557,40 @@ pub const fn concat_args( out } +/// The key `--help` answers to, and the one `-h` does. +/// +/// Reserved rather than generated: a derive builds keys from a hash of the type they came from +/// in the high half and an index in the low half, so the top of the range belongs to nobody. +/// Generated code compares against these to tell a help request from a flag of its own. +pub const HELP_LONG_KEY: u64 = u64::MAX; +/// See [`HELP_LONG_KEY`]. +pub const HELP_SHORT_KEY: u64 = u64::MAX - 1; + +/// `--help`, which every command answers to. +/// +/// In the parse table and *not* in the metadata, which is the whole trick: the parser has to +/// recognise the flag, and help output must not list it — a spec does not declare `--help`, so +/// showing one would make the rendered page disagree with the spec it came from. +pub static HELP_LONG: Flag<'static> = Flag { + key: HELP_LONG_KEY, + name: "help", + longs: &["help"], + ..Flag::BOOL +}; + +/// `-h`, which prints the shorter form. +pub static HELP_SHORT: Flag<'static> = Flag { + key: HELP_SHORT_KEY, + name: "help", + shorts: b"h", + ..Flag::BOOL +}; + +/// Whether a flag is one of the two the parser supplies rather than the CLI declaring it. +pub fn is_help_flag(flag: &Flag<'_>) -> bool { + flag.key == HELP_LONG_KEY || flag.key == HELP_SHORT_KEY +} + /// Resolve a subcommand by name or alias, at compile time. /// /// For [`Command::default_subcommand`], which names a command that a derive cannot see: the @@ -885,6 +929,16 @@ impl<'t, 'v> Parser<'t, 'v> { }); } + // Every CLI answers to `--help`, and none of them declares it. Asked *after* the + // command's own flags, so a CLI that declares its own `--help` keeps it. + if name == b"help" { + return Ok(Event::Flag { + flag: &HELP_LONG, + value: None, + negated: false, + }); + } + if self.cmd.unknown_flags == UnknownFlags::Error { return Err(Error::UnknownFlag { token }); } @@ -1092,7 +1146,15 @@ impl<'t, 'v> Parser<'t, 'v> { } fn find_short(&self, byte: u8) -> Option<&'t Flag<'t>> { - self.in_scope().find(|f| f.shorts.contains(&byte)) + self.in_scope() + .find(|f| f.shorts.contains(&byte)) + // As for `--help`: supplied by the parser, and only where the command has not + // declared a `-h` of its own. + .or(if byte == b'h' { + Some(&HELP_SHORT) + } else { + None + }) } fn find_subcommand(&self, name: &[u8]) -> Option<&'t Command<'t>> { @@ -1908,6 +1970,79 @@ mod tests { ); } + #[test] + fn a_wrapper_still_forwards_a_help_flag() { + // Supplying `--help` must not take the two forwarding mechanisms away from a wrapper, + // which is the one place a CLI means to hand the token on rather than answer it. + static ARGS: Arg = Arg { + key: 24, + name: "args", + ..Arg::VAR + }; + static WRAP: Command = Command { + name: "wrap", + args: &[&ARGS], + ..Command::EMPTY + }; + + // A typed separator: everything after it is a value, `--help` included. + let a = argv(["--", "--help", "-h"]); + assert_eq!( + parse(&WRAP, &a).unwrap(), + vec![ + Event::Arg { + arg: &ARGS, + value: b"--help" + }, + Event::Arg { + arg: &ARGS, + value: b"-h" + }, + ] + ); + + // And `automatic`, for the wrapper whose caller should not have to type one: the + // first value stops flag interpretation, so the flags after it forward. + static AUTO_ARGS: Arg = Arg { + key: 25, + name: "args", + double_dash: DoubleDash::Automatic, + ..Arg::VAR + }; + static AUTO_WRAP: Command = Command { + name: "wrap", + args: &[&AUTO_ARGS], + ..Command::EMPTY + }; + + let a = argv(["node", "--help"]); + assert_eq!( + parse(&AUTO_WRAP, &a).unwrap(), + vec![ + Event::Arg { + arg: &AUTO_ARGS, + value: b"node" + }, + Event::Arg { + arg: &AUTO_ARGS, + value: b"--help" + }, + ] + ); + + // Before either takes effect, though, the wrapper's own help is what `--help` asks + // for — `mise run --help` is a question about `run`, not a value for it. + let a = argv(["--help"]); + assert_eq!( + parse(&AUTO_WRAP, &a).unwrap(), + vec![Event::Flag { + flag: &HELP_LONG, + value: None, + negated: false + }] + ); + } + #[test] fn double_dash_required_arg() { static CMD: Arg = Arg { diff --git a/conformance/tests/help_request.rs b/conformance/tests/help_request.rs new file mode 100644 index 00000000..4ab83171 --- /dev/null +++ b/conformance/tests/help_request.rs @@ -0,0 +1,160 @@ +//! Asking for help, and getting the page the reference would print. +//! +//! `--help` and `-h` are supplied by the parser rather than declared by a CLI, because no spec +//! declares them and one that did would render a `--help` in its own help output. They are +//! recognised after the command's own flags, so a CLI that *does* declare one keeps it. +//! +//! A request comes back as `Error::Help` rather than being printed: a parse that stops to show +//! help has produced no value, which is the shape every caller already handles, and a library +//! that writes to stdout on its own is one an adopter cannot embed. `parse()` — the convenience +//! that reads the process's own arguments — is the one place that prints and exits. + +use std::ffi::OsStr; + +use usage_argv::Error; +use usage_derive::{Args, Cli, Subcommands}; + +/// A command with its own flags, to be asked about. +#[derive(Args)] +struct Ls { + /// Do not print a header + #[usage(long)] + no_header: bool, +} + +#[derive(Subcommands)] +enum Commands { + /// List things + Ls(Box), +} + +/// A tool whose help is worth asking for +#[derive(Cli)] +#[usage(bin = "ex")] +struct Ex { + /// Be loud + #[usage(long, short = 'v')] + verbose: bool, + #[usage(subcommand)] + command: Option, +} + +fn ask(tokens: &[&str]) -> (bool, String) { + let argv: Vec<&OsStr> = tokens.iter().map(OsStr::new).collect(); + match Ex::parse_from(&argv) { + Err(Error::Help { cmd, long }) => ( + long, + usage_argv::help::render(Ex::spec(), cmd, long).expect("the command is this CLI's"), + ), + Err(other) => panic!("expected a help request, got {other:?}"), + Ok(_) => panic!("expected a help request, not a parse"), + } +} + +#[test] +fn the_long_and_short_forms_ask_for_different_pages() { + let (long, page) = ask(&["--help"]); + assert!(long, "`--help` asks for the long form, as clap has it"); + assert!(page.contains("\nUsage: ex"), "{page}"); + + let (long, short_page) = ask(&["-h"]); + assert!(!long, "`-h` asks for the short one"); + assert_ne!( + page, short_page, + "the two forms differ, or there was no reason to tell them apart" + ); +} + +#[test] +fn help_is_asked_about_the_command_the_words_reached() { + // `ex ls --help` is a question about `ls`, not about `ex` — which is why the request carries + // the command in scope rather than the root. + let (_, page) = ask(&["ls", "--help"]); + assert!(page.contains("\nUsage: ex ls"), "{page}"); + assert!(page.contains("--no-header"), "{page}"); + + // The root lists it too, inside `ls`'s own usage line in the commands section — so what + // says the flag is not the *root's* is that it has no entry in the root's flags. + let (_, root) = ask(&["--help"]); + assert!( + !root.contains("\n --no-header"), + "the root has no such flag of its own: {root}" + ); +} + +#[test] +fn asking_for_help_does_not_stop_the_cli_working() { + // The fixture parses as any CLI does when nobody asks for help, which is also what keeps its + // fields read: a test CLI nobody parses is dead code, and CI denies warnings. + let argv = [ + OsStr::new("-v"), + OsStr::new("ls"), + OsStr::new("--no-header"), + ]; + let parsed = Ex::parse_from(&argv).expect("no help was asked for"); + assert!(parsed.verbose); + let Some(Commands::Ls(ls)) = parsed.command else { + panic!("expected ls") + }; + assert!(ls.no_header); +} + +#[test] +fn a_help_flag_is_not_in_the_spec_it_renders() { + // The two are supplied by the parser and belong to no command's metadata, so they cannot + // appear in help output or in the emitted spec. A page advertising a `--help` that the spec + // it came from does not declare is a page that disagrees with its own CLI. + let (_, page) = ask(&["--help"]); + assert!(!page.contains("--help"), "{page}"); + assert!(!page.contains("-h "), "{page}"); + + let kdl = Ex::to_kdl(); + assert!(!kdl.contains("help\""), "{kdl}"); +} + +#[test] +fn a_cli_that_declares_its_own_help_keeps_it() { + // Recognised *after* the command's own flags, so declaring one is still possible — and then + // it binds as any other flag does rather than stopping the parse. + #[derive(Cli)] + #[usage(bin = "own")] + struct Own { + /// A help of its own + #[usage(long = "help", short = 'h')] + help: bool, + } + + let argv = [OsStr::new("--help")]; + let parsed = Own::parse_from(&argv).expect("the CLI's own flag, not a help request"); + assert!(parsed.help); + + let argv = [OsStr::new("-h")]; + let parsed = Own::parse_from(&argv).expect("the short form too"); + assert!(parsed.help); +} + +#[test] +fn help_wins_over_what_would_otherwise_be_an_error() { + // `ex --help` with a required argument missing still prints help: the request is answered + // while parsing, before anything is judged. Anything else would make `--help` useless for + // the person who needs it most — someone who does not yet know what to type. + #[derive(Cli)] + #[usage(bin = "strict")] + struct Strict { + /// Required + #[usage(long)] + file: String, + } + + let argv = [OsStr::new("--help")]; + assert!(matches!( + Strict::parse_from(&argv), + Err(Error::Help { long: true, .. }) + )); + + // And with the argument given, the same CLI parses — which is what says the help request + // was answered early rather than the requirement never having been there. + let argv = [OsStr::new("--file"), OsStr::new("mise.toml")]; + let parsed = Strict::parse_from(&argv).expect("nothing missing"); + assert_eq!(parsed.file, "mise.toml"); +} diff --git a/derive/src/codegen.rs b/derive/src/codegen.rs index c8c4375e..ef230e35 100644 --- a/derive/src/codegen.rs +++ b/derive/src/codegen.rs @@ -224,10 +224,24 @@ pub fn emit(cli: &Cli) -> TokenStream { while let ::std::option::Option::Some(__usage_event) = __usage_parser.next_event() { + let __usage_event = __usage_event?; + // Asked *before* the event is applied, and answered with the command in + // scope: `mise config --help` is a question about `config`, and the parser + // is what knows how deep the words reached. + if let ::usage_argv::Event::Flag { flag, .. } = &__usage_event { + if flag.key == ::usage_argv::HELP_LONG_KEY + || flag.key == ::usage_argv::HELP_SHORT_KEY + { + return ::std::result::Result::Err(::usage_argv::Error::Help { + cmd: __usage_parser.command(), + long: flag.key == ::usage_argv::HELP_LONG_KEY, + }); + } + } // `apply` handles this command's own fields and routes anything // else into its subcommands, which is why a nested command needs // nothing extra here. - #module::apply(&mut partial, &__usage_event?); + #module::apply(&mut partial, &__usage_event); } #module::check(&mut partial)?; @@ -247,7 +261,28 @@ pub fn emit(cli: &Cli) -> TokenStream { // The error borrows argv, so it cannot outlive this function; // rendering it here is what makes the signature usable. Better // diagnostics are a separate piece of work. - Self::parse_from(&__usage_argv).map_err(|e| ::std::format!("{e:?}")) + match Self::parse_from(&__usage_argv) { + ::std::result::Result::Ok(parsed) => ::std::result::Result::Ok(parsed), + // A help request is not a failure, and this is the one place that knows + // the process is the caller: print the page and leave successfully, which + // is what a user typing `--help` asked for. `parse_from` returns it + // instead, so a library embedding this decides for itself. + ::std::result::Result::Err(::usage_argv::Error::Help { cmd, long }) => { + match ::usage_argv::help::render(Self::spec(), cmd, long) { + ::std::option::Option::Some(page) => { + ::std::print!("{page}"); + ::std::process::exit(0); + } + // Only reachable if the command came from another CLI's tables. + ::std::option::Option::None => ::std::result::Result::Err( + "help was asked for a command this program does not have".into(), + ), + } + } + ::std::result::Result::Err(e) => { + ::std::result::Result::Err(::std::format!("{e:?}")) + } + } } } }