Allow unrecognized subcommands

[sgrif]

It's a pretty common pattern for CLIs which take subcommands to attempt to subshell out to appname-subcommand if the command is unrecognized, to allow new commands to be easily added. This is impossible to do with clap today (at least not without writing the usage string by hand), as if you do appname foo it'll complain that it did not expect an argument foo.

I propose adding AppSettings::AllowUnrecognizedSubcommand, and also improving the error message to specify that it's an unrecognized subcommand and list the possible subcommands, as it'll likely require the same code changes

[kbknapp]

This should be an easy addition. Thanks for suggesting it! 👍

I should be able to knock this out either later today or early tomorrow. I'll post back here with the updates.

[sru]

Rather than AppSettings::AllowUnrecognizedSubcommand, how about AppSettings::AllowExternalSubcommand? AllowUnrecognizedSubcommand sounds like it will just allow passing a subcommand that is not recognized, but I think clap can do better because I also think appname-subcommand is pretty common pattern. If AppSettings::AllowExternalSubcommand is enabled and clap cannot match any subcommand, it should try finding external command named appname-subcommand.

Now, it can get ambiguous when positional argument is also allowed. Should clap pass the argument as positional argument if it cannot find any subcommands? Or should it give the subcommand not found error message?

[kbknapp]

I have a local branch of this and AllowExternalSubcommands and ExecuteExternalSubcommands is what I ended up going with.

The issue that ended up making this take way longer than I'd hoped was in the details. What happens if that external subcommand isn't found (when Execute is used), or exits with an error code? (I have since solved this). Or what about with simply Allow, how should it return the trailing arguments to pass to the external subcommand, along with the matches? (Again, solved.)

The solutions I ended up going with (which I'm just ironing out, and hope to have done shortly), are you can either AllowExternalSubcommands, which simply returns the subcommand name, and all trailing arguments within the typical ArgMatches struct. This allows you to verify (security), execute, and handle external subcommands in a totally custom way. Also defering the execution of subcommands until after you've processed anything else, or in whatever order you choose. Basically you have full control.

The second way of ExecuteExternalSubcommands will basically just take away the boilerplate. clap tries to execute bin_name-subcommand [trailing args] which must be somewhere in your $PATH. This is the least secure, and flexible way, but if you don't care about verifying locations, or execution order (because external subcommands will be executed before returning any ArgMatches struct), then it should be fine. This also comes with two additional error cases when using the App::get_matches_safe family of methods. ExternalSubcommandNotFound and ExternalSubcommandError for missing bin_name-subcommand binaries and error exit codes respectively.

Once I finish the testing, I'll upload the branch and push the PR.

[sru]

@kbknapp 👍

[sgrif]

I definitely like AllowExternalSubcommands, but Execute just seems like it's too much. When I ask for the subcommand, the library probably shouldn't be subshelling out and doing anything of the sort. Does the program exit in that case? If not what does subcommand return in that case? It seems like a much less magic solution would be to provide a run_external_subcommand(String, ArgMatches) function that can be called for the "simple" case.

Does the proposed solution also work for bin_name-subcommand_name-subsubcommand_name?

[kbknapp]

@sgrif I'm glad you said that, because I already had some reservations about the Execute portions. There's a fuzzy line between what a library provides for convenience, and what's the consumer's responsibility. My main reservation was the security aspect, of just blinding executing binaries. Granted, users wouldn't have to use the Execute, but still.

So I'm going to pull the Execute portion, and stick with only Allow.

Does the proposed solution also work for bin_name-subcommand_name-subsubcommand_name?

Do you mean only for Execute? Just want to make sure, since that's now a somewhat moot issue 😜

Status Update

Obviously this has taken much longer than I originally anticipated. Just to keep everyone in the loop, some of the changes required for this feature also melded nicely with some planned breaking changes for a 2x release. These breaking changes are minor, but big ergonomic wins. And as a bi-product, also making implementing this feature easier. So I'm trying to push out the base of the 2x (basically just breaking changes portion to speed up time to release), and this feature will be part of that. Now to quell any fears, 2x won't be a massive re-design like #259 proposes (that'll have to wait for a 3x).

It's nearly complete, so it shouldn't be too much longer depending on my free time over the next few days.

Here's what 2x looks like:

• App, Arg, SubCommand, ArgMatches, and ArgGroup all have reduced the number of lifetimes in the signature from 6-7 (App, and Arg) down to none, or one. (breaking change)
• Invalid unicode support for argument values (since Linux allows invalid unicode for files, etc.)
• Removal of all deprecated methods (breaking change)
• Removal of all App::get_matches_*_lossy methods, because that functionality has been moved into AppSettings (breaking change)
• Allowing support external subcommands

That's it. So you can see 2x isn't a massive change, but it's a chance to clean up some cruft.

[kbknapp]

Closed with #391 and will be available shortly upon v2 release. Here's an example of using the new feature:

Note, using get_matches_from only for the demo purposes....this works just fine with get_matches() ;)

extern crate clap;
use clap::{App, AppSettings};

fn main() {
    // Assume there is a third party subcommand named myprog-subcmd
    let m = App::new("myprog")
        .setting(AppSettings::AllowExternalSubcommands)
        .get_matches_from(vec!["myprog", "subcmd", "--option", "value", "-f"]);
    // All trailing arguments will be stored under the subcommands sub-matches under a value
    // of their runtime name (in this case "subcmd")
    match m.subcommand() {
        (external, Some(ext_m)) => {
             let ext_args: Vec<&str> = ext_m.values_of(external).unwrap().collect();
             assert_eq!(external, "subcmd");
             assert_eq!(ext_args, ["--option", "value", "-f"]);
        },
        _ => unreachable!()
    }
}