Clap: Alias into subcomand

[CAD97]

As far as I can tell, it's currently possible to alias command arguments and subcommands. For things such as https://github.com/rust-lang/rustup/issues/2148#issue-534509269, it'd be really nice if it's somehow possible to alias both "across" and "into" subcommands, e.g. treat rustup install the same as rustup toolchain install, i.e. including all arguments and such configured for the target subcommand.

[Stupremee]

I would like to work on this issue

[Dylan-DPC]

@Stupremee sure. you can go ahead and submit a PR. If you have any questions, you can post them here :)

[pksunkara]

I have come up with 2 ways to implement this:

1. Have a new subcommand type thing whose job is just replacing things.

   App::new("rustup")
      .replacer("install", "toolchain install")
   
   // Or alternatively
   App::new("rustup")
      .subcommand(SubCommand::with_name("install")
          .replace_with("toolchain install")
      )
   

   PROS: Can also use flags and options in the replace target string
   CONS: Parsing logic might become a bit more complex?

2. Have a alias_up and visible_alias_up which takes number argument on how many levels it has go up.

   App::new("rustup")
      .subcommand(SubCommand::with_name("toolchain")
          .subcommand(SubCommand::with_name("install")
              .visible_alias_up(2, "install")
          )
      )
   

   PROS: Works like alias api
   CONS: Sometimes might not know how many levels it has go up. Also, what about going down?

[kinnison]

An idea I had was:

App::new("rustup")
    .mirror("install", "toolchain-install")
    ...
    .subcommand(SubCommand::with_name("toolchain")
        .subcommand(SubCommand::with_name("install")
            .as_mirror("toolchain-install")
            ...
        )
    )
)

Where the .mirror() and .as_mirror() names would have to match up by the time you called .get_matches() or somesuch perhaps?

[CreepySkeleton]

I like visible_alias_up but mirror is more general.

Again, I have yet to familiarize myself with clap internals (except for some) so I can't tell which way would be easier to implement. Let's postpone it until we dealt with clap_derive (I'll get to it in ~30 mins)

[Dylan-DPC]

I was thinking more of .mirror("toolchain") in app and then you can add .delegate() to each subcommand you want to send to toolchain (I'm assuming they'll be multiple at some point)

[kinnison]

Are you imagining .mirror("label") sets up a subcommand which can be referenced and then .delegate("name", "label") is like .subcommand(with_name("name").....) matching the subcommand which had the .mirror() on it?

[Dylan-DPC]

Yeah I guess you got it

[pksunkara]

So, I was looking to implement this, and as of now, I can simply do delegation to sibling commands. You can't delegate up. Is that okay?

App::new("rustup")
    .subcommand(App::new("install")
        .delegate(["toolchain", "install"]))
    .subcommand(App::new("toolchain")
        .subcommand(App::new("install")))

[kinnison]

That would be sufficient for rustup's use case, if others in clap are OK with that as a limitation.

[pksunkara]

Actually, we can do something like this:

App::new("rustup")
    .delegate(["install"], ["toolchain", "install"])
    .subcommand(App::new("install"))
    .subcommand(App::new("toolchain")
        .subcommand(App::new("install")))

Which would be the same thing, but it actually allows us to delegate up too. Would require more checks though.

// These runs `rustup install` logic if you run `rustup toolchain install`
App::new("rustup")
    .delegate(["toolchain", "install"], ["install"])
    .subcommand(App::new("install"))
    .subcommand(App::new("toolchain")
        .subcommand(App::new("install")))

[Dylan-DPC]

Looks good to me. I don't see much of a usecase of delegating upwords.

[kinnison]

From rustup's perspective the canonical form is rustup toolchain install and we want to support silently rustup install as an alias for those who have it stuck in scripting etc. We don't want it showing in help etc, but simply to be internally converted by clap.

[CreepySkeleton]

I would like propagating things only down (this delegation is a form of propagation too), like it has been for all this time; global args and global settings has been propagating down. It's being done in one, up-bottom propagation step. Making an exception for .delegate would essentially add another, bottom-up propagation step which may affect performance for other cases. It's hard to say for sure without concrete numbers, but I would still like to keep things simple.

@kinnison You're able to hide subcommands from help messages via AppSettings::Hidden.

[kinnison]

I'm aware of Hidden yes -- my point was that whatever the scheme, I'd need to be able to do that. That's all :D

[pksunkara]

I have been looking at this the past few days and I have realized that doing this requires large changes in parsing logic. I would like to instead throw an alternative.

let install = App::new("install"); // The install command

App::new("rustup")
    .subcommand(App::new("toolchain")
        .subcommand(install.clone()))
    .subcommand(install.settings(AppSettings::Hidden))

Doesn't this basically solve your use case? Also, this is more correct since if we are just aliasing to a different path, then it might have issues with args on parent subcommands (toolchain args here).

[CAD97]

@pksunkara the issue is that, while the arguments are correctly parsed in this case, the actual code still has to handle the fact that there are two entry points to the functionality. The advantage of alias over just mounting the same App at two places is being able to handle it in one place and have it automatically apply to the other, aliased location.

In short, we want to let the user say rustup install <args> and have that automatically expanded to rustup toolchain install <args>, replacing "install" with "toolchain install". The lack of arguments between toolchain and install is not an issue; the alias is, in effect, an alias to the zero argument (before the subcommand) form.

[pksunkara]

the actual code still has to handle the fact that there are two entry points to the functionality

Could you explain why you do not want this?

The advantage of alias over just mounting the same App at two places is being able to handle it in one place and have it automatically apply to the other, aliased location.

You are still writing just one install App. All you are doing is adding 2 lines to your root App.

[CAD97]

The issue comes after doing get_matches. Once you have done the matching, you need to test to see if subcommand_matches("install") and then somehow jump into the middle of your code handling subcommand_matches("toolchain") to where it handles subcommand_matches("install").

The point of having the alias at the App layer is enforcing, at the argument parsing level, that the two are identical functionality, rather than two distinct endpoints that happen to have the same exact syntax.

Or are you saying that if I use your code, then matches.subcommand_matches("install") and mathces.subcommand_matches("toolchain").subcommand_matches("install") are actually equivalent? We want the processing code to not know (because it doesn't care and _shouldn't_ know) that the alias was used. It _should not be possible_ for the behavior to diverge; that's the exact issue we have with rustup install currently that led to the maintenance team wanting to remove it, because they couldn't just say "treat it as if the user invoked the canonical command, rustup toolchain install".

[pksunkara]

My bad, I was thinking of a different architecture for the CLI.

I was thinking Apps have run() and when you get matches, you just iterate over them to get the App and then just run() on it. (clap_derive/structopt makes this even easier).

[pksunkara]

@CAD97 I have the PR ready, please give it a look.