cli: allow alias after global args

[martinvonz]

Our support for aliases is very naively implemented; it assumes the
alias is the first argument in argv. It therefore fails to resolve
aliases after global arguments such as --at-op.

This patch fixes that by using doing an intial parse of the arguments
with a modified definition of the CLI passed to clap. The modified
definition has only global arguments, plus an "external
subcommand". That way we can convince clap to parse the global
arguments for us and give us the alias (or real command) and further
arguments back. Thanks to @epage for the suggestion on in
clap-rs/clap#3672. The only minor problem is that it doesn't seem
possible to tell clap to parse global arguments that come after the
external subcommand. To work around that, we manually merge any parsed
global argument before the alias with any parsed global arguments
after it.

Closes #292.

Checklist

• I have made relevant updates to CHANGELOG.md

[spectral54]

Nit: description says "That make clap", should be "makes"; also says 'suggestion on in' should just be 'suggestion in'.

Thoughts on implementation from offline discussion: Assuming that alias_args is a subvec (the tail elements of string_args), let's do something where each loop pass concatenates the first N arguments from string_args, the arguments from the alias resolution, and all of the arguments from alias_args[1..] (where N = string_args.length() - alias_args.length()).

[martinvonz]

Nit: description says "That make clap", should be "makes"; also says 'suggestion on in' should just be 'suggestion in'.

Done.

Thoughts on implementation from offline discussion: Assuming that alias_args is a subvec (the tail elements of string_args), let's do something where each loop pass concatenates the first N arguments from string_args, the arguments from the alias resolution, and all of the arguments from alias_args[1..] (where N = string_args.length() - alias_args.length()).

That turned out much simpler, and obviously more reliable because we don't need to manually merge earlier global arguments with later ones. @epage, can you confirm if that assumption is correct (i.e. that the list of strings passed to an external_subcommand exactly matches the end of the full list of arguments passed to parse_from())?

[epage]

That turned out much simpler, and obviously more reliable because we don't need to manually merge earlier global arguments with later ones. @epage, can you confirm if that assumption is correct (i.e. that the list of strings passed to an external_subcommand exactly matches the end of the full list of arguments passed to parse_from())?

clap does no post-processing on the external subcommand arguments; they are exactly what clap received.

If I'm understanding the suggestion, its to manually extract the global args and subcommand and merge the global args in with the external subcommand arguments. I personally wouldn't do this as I'd be concerned about it being brittle; it having corner cases to deal with.

[martinvonz]

That turned out much simpler, and obviously more reliable because we don't need to manually merge earlier global arguments with later ones. @epage, can you confirm if that assumption is correct (i.e. that the list of strings passed to an external_subcommand exactly matches the end of the full list of arguments passed to parse_from())?

clap does no post-processing on the external subcommand arguments; they are exactly what clap received.

Okay, that's good, and what we assumed.

If I'm understanding the suggestion, its to manually extract the global args and subcommand and merge the global args in with the external subcommand arguments. I personally wouldn't do this as I'd be concerned about it being brittle; it having corner cases to deal with.

Yes, that's correct. It actually seems much simpler and less brittle to me. See how much simpler it got.

[epage]

Yes, that's correct. It actually seems much simpler and less brittle to me. See how much simpler it got.

Fewer lines of code is not the sample thing as simplicity.

There are more assumptions on behavior with the new approach which is where my concern with brittleness comes from.

If it works for you, go for it. Like I said, for myself, I wouldn't but there are times when people come to different conclusions and that is fine.

[martinvonz]

Yes, that's correct. It actually seems much simpler and less brittle to me. See how much simpler it got.

Fewer lines of code is not the sample thing as simplicity.

There are more assumptions on behavior with the new approach which is where my concern with brittleness comes from.

If it works for you, go for it. Like I said, for myself, I wouldn't but there are times when people come to different conclusions and that is fine.

In what way do you think it's brittle? What should we watch out for if we do it this way? Is the concern that clap might do some parsing of the later arguments in the future (making that assertion I added fail)?

My biggest concern with the way we did it before was that we had to replicate clap's override behavior.

[epage]

I'm more going more from gut feel and do not have the time or interest in litigating this.

[yuja]

I think new implementation is better.
It's clearer that alias expansion works at lower args: &[str] layer (e.g. aliased_diff = ["diff", "-r"] is valid even though -r takes value), not at the layer where the structs parsed by clap should live (e.g. where the alias would be described as ["diff", "-r", "$1"].)