pacak / bpaf

Command line parser with applicative interface
337 stars 17 forks source link

Generate `help` subcommand #323

Open ysndr opened 8 months ago

ysndr commented 8 months ago

Just a quality of life thing. Nearly everyone using our tool tires to run prog help and prog help subcommand. I added a subcommand myself like this:

#[derive(Debug, Bpaf, Clone)]
struct Help {
    /// Command to show help for
    #[bpaf(positional("cmd"))]
    cmd: Option<String>,
}
impl Help {
    fn handle(self) {
        let mut args = Vec::from_iter(self.cmd.as_deref());
        args.push("--help");

        match flox_cli().run_inner(&*args) {
            Ok(_) => unreachable!(),
            Err(ParseFailure::Completion(comp)) => print!("{comp}"),
            Err(ParseFailure::Stdout(doc, _)) => info!("{doc}"),
            Err(ParseFailure::Stderr(err)) => error!("{err}"),
        }
    }
}

It works, but I'd expect prog help doesnotexist to print an error. I can't enumerate the available subcommands or anything alike so and --help permits unknowns https://github.com/pacak/bpaf/issues/288 :/

Btw, are the completion and Stderr variants reachable at all if i pass just a command --help in the parser given #288?

pacak commented 8 months ago

I can't enumerate the available subcommands

It should be possible to enumerate available commands with completion mechanism, take a look at static_complete_test_2 in tests/completion.rs. As long as you don't have anything that expands to an arbitrary text you should be able to tell what is a command by looking for items that don't start with a dash. Completion output contains lines - one per completion item with tab separated fields. Fields are: substitution, pretty substitution, group description and item help.

I need to think a bit more on making a helper for that, but it should be possible.

Btw, are the completion and Stderr variants reachable at all if i pass just a command --help in the parser given https://github.com/pacak/bpaf/issues/288?

They shouldn't be produced.

pacak commented 8 months ago

Current thoughts.

So there's two ways of solving it - adding more functionality to derive macro (in combinatorinc API it's not needed) or stabilizing some bits of the internal API.

More stuff in derive macro

Adding help subcommand is not a problem in combinatoric: you define your commands as OptionParser, then put them in a slice along with names, generate help since you have all the names, then transform all the parsers into boxed ones and combine them with choice. This should be as efficient as it can be in current implementations. A bit of allocations, nothing major. Same as using construct!([cmd1, cmd2]).

For derive - I can access command names here

#[derive(Bpaf, Debug, Clone)]
enum Opt {
    #[bpaf(command)]
    Fetch {
        dry_run: bool,
        all: bool,
        repository: String,
    },
    #[bpaf(command)]
    Add {
        interactive: bool,
        all: bool,
        files: Vec<PathBuf>,
    },
}

But not here, since external means impl Parser which can contain whatever.

#[derive(Bpaf, Debug, Clone)]
enum Opt {
    #[bpaf(external)]
    Fetch(Fetch),
    #[bpaf(external)]
    Add(Add),
}

In case where command names are visible I can use some kind of parameter that takes a way to print stuff.... Seems messy.

publishing bits of internal API

Alternatively impl Parser already gives access to public, but currently hidden function .meta() which gives access to whole parser meta info. You can tell what is optional, required, what arguments it takes, all the commands with all the help messages, etc. I can probably stabilize this along with some helper functions which means you'll have an easy way to tell what commands are there.

I think second approach is cleaner overall, need to think about this a bit longer.

bzm3r commented 7 months ago

@pacak Even the meta method implicated by Parser is not stabilized, would it be okay to make it public with a doc comment warning against unstability?

Alternatively, Parser can provide a help method which is based on meta, but is much more targeted in scope. This way, it can be stabilized, and made public, while the function it relies on (meta) can remain unstable?

pacak commented 7 months ago

Yea, I think publishing them makes sense. It's mostly about going over the available constructors and figuring out how to document them.