Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Complete subcommand to list all subcommands for a given argument. #134

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

naudzghebre
Copy link

Adding a subcommand for all commands, similar to help, which is responsible for generating all the subcommands for a given command.

This function can be used to assist with auto-completion in the shell
by providing the set of possible subcommands available at the cursor.

responsible for generating all the subcommands for a given command.

This function can be used to assist with auto-completion in the shell
by providing the set of possible subcommands available at the cursor.
Copy link
Collaborator

@erickt erickt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you go into some more detail about why building this functionality into argh is advantageous, vs the clients implementing it on their own? Extending from my example, it wouldn't be hard to do:

let cmd: MyCmd = argh::from_env();
if cmd.completions {
    for subcommand in MyCmd::COMMANDS.iter().chain(MyCmd::dynamic_commands()) {
        println!("{:?}", cmd.name);
    }
    return;
}

The main downside I can see is that it's not recursive. However, if we land #119 then we'd have a way to walk through the full recursive argument structure.

--

The other thing I think worth exploring is how do you expect this to be used for argument completion? As best as I can tell by things like https://kbknapp.dev/shell-completions/ and https://docs.rs/clap_complete/latest/clap_complete/, it sounds like the typical workflow is to generate shell-specific helpers. Would it make more sense to support a workflow where we could have a build-time option to generate the completion shell scripts without needing to claim flags like --complete or the subcommand name complete?

@@ -817,6 +820,11 @@ pub fn parse_struct_args(
continue;
}

if (next_arg == "--complete" || next_arg == "complete") && !options_ended {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claiming --complete or complete is a backwards incompatible change, since it's possible for users to be actively using these as options or subcommands. Can we avoid this? For example, we could land support for complete_func, but still leave it up to the user to implement it, with something like:

#[derive(FromArgs)]
struct MyCmd {
    #[argh(switch)]
    complete: bool,

    ...
}

fn main() {
    let cmd: MyCmd = argh::from_env();
    if cmd.complete {
        return MyCmd::complete();
    }
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And good point about the backwards incompatibility. I think this is going to need iterating.

As mentioned in the other comment, this would require going back and adding this switch to every command/subcommand in the ffx tool (which is what I'm targeting to test this stuff) ?

let subcommand_ty = subcommand.ty_without_wrapper;
subcommand_format_arg = quote! { subcommands = subcommands };
subcommand_calculation = quote! {
let subcommands = argh::print_subcommand_list(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also include options in the output?

@naudzghebre
Copy link
Author

Could you go into some more detail about why building this functionality into argh is advantageous, vs the clients implementing it on their own? Extending from my example, it wouldn't be hard to do:

let cmd: MyCmd = argh::from_env();
if cmd.completions {
    for subcommand in MyCmd::COMMANDS.iter().chain(MyCmd::dynamic_commands()) {
        println!("{:?}", cmd.name);
    }
    return;
}

My thinking was implementing it in argh would be most straightforward/simplest, since it seems that's where relative structure between commands and subcommands is accessible (e.g. help() gathering all of the info about a specific command). Truth be told, I didn't consider client side in this way. This approach seems like a large amount of changes would have to be made to each of the existing command structs. And would we have to enforce any new command implementations to have a complete switch added in order to get an accurate representation?

The main downside I can see is that it's not recursive. However, if we land #119 then we'd have a way to walk through the full recursive argument structure.

If #119 is landed, does that mean we'd have a way to traverse the entire topology of the argument parser's final output? This is actually an approach I wanted to take. Something to the effect of building out and writing the entire structure to some intermediate schema that can be parsed by shell-specific scripts implemented by the user.

One level beyond this would be to spit out the shell specific scripts from within argh, just as you suggested. But I think we'd need this ability of exposing all of the possible command/subcommands/options to do this.

And good point about the backwards incompatibility. I think this is going to need iterating.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants