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

ockam completion --shell {bash|zsh|fish} command for shell completion #3220

Closed
mrinalwadhwa opened this issue Aug 13, 2022 · 8 comments
Closed

Comments

@mrinalwadhwa
Copy link
Member

mrinalwadhwa commented Aug 13, 2022

Current Behaviour

Users have to type the full ockam commands

Desired Behaviour

Generate shell completion scripts for various shells

ockam completion --shell {bash|zsh|fish}

This would require adding the completion command here:
https://github.com/build-trust/ockam/tree/develop/implementations/rust/ockam/ockam_command/src

Clap Complete seems to have everything we need for this.
https://github.com/clap-rs/clap/tree/master/clap_complete

After this command exists we can also look at adding it to our homebrew formula
https://github.com/build-trust/homebrew-ockam/blob/main/ockam.rb

Github's gh completion command seems good to model ockam completion behavior after
https://cli.github.com/manual/gh_completion


We love helping new contributors!
If you have questions or need help as you work on your first Ockam contribution, please leave a comment on this discussion.
If you're looking for other issues to contribute to, checkout this discussion and labels - good first issue or help wanted

@datdhruv
Copy link
Contributor

Hi!, Can this be assigned to me?

@mrinalwadhwa
Copy link
Member Author

mrinalwadhwa commented Aug 18, 2022

@datdhruv of course, all yours. Looking forward to the pull request. Let us know if you have questions along the way.

@mrinalwadhwa
Copy link
Member Author

mrinalwadhwa commented Aug 24, 2022

@datdhruv checking in to see if you still have time to work on this issue or do you have any questions.

If you're busy that's completely okay. We can open this issue for someone else to pick up and then whenever you're available again I'd be happy suggest other issues you could contribute to.

@datdhruv
Copy link
Contributor

Hey, should be ready with a commit today

@mrinalwadhwa
Copy link
Member Author

Awesome, looking forward to the PR 🐰

@datdhruv
Copy link
Contributor

datdhruv commented Aug 24, 2022

One question,

clap_complete has a method generate which expects the shell, a reference to a Clap::App, the name of the bin file and output stream.

generate(Shell::Bash, &mut ockam_command, "ockam", &mut io::stdout())

From what i gather we dont use a clap::App in the classical sense

(classic sense => something like this)

pub fn make_subcommand<'help>() -> App<'help> {
    App::new("init")
        .about("Creates the boilerplate structure and files for a new book")
        // the {n} denotes a newline which will properly aligned in all help messages
        .arg(arg!([dir]
            "Directory to create the book in{n}\
            (Defaults to the Current Directory when omitted)"
        ))
        .arg(arg!(--theme "Copies the default theme into your source folder"))
        .arg(arg!(--force "Skips confirmation prompts"))
        .arg(
            Arg::new("title")
                .long("title")
                .takes_value(true)
                .help("Sets the book title")
                .required(false),
        )
        .arg(
            Arg::new("ignore")
                .long("ignore")
                .takes_value(true)
                .possible_values(&["none", "git"])
                .help("Creates a VCS ignore file (i.e. .gitignore)")
                .required(false),
        )
}

from https://github.com/rust-lang/mdBook/blob/4c303c3b1dde1cbdf2f593b05687340915b93417/src/cmd/init.rs#L12

How would I create a reference to the clap::app? (right now ockam_command is of type struct:OckamCommand

@mrinalwadhwa
Copy link
Member Author

@datdhruv I found this on the Clap discussions group clap-rs/clap#3671 (comment)

Deriving Parser also derives the trait CommandFactory which has the CommandFactory::command function.

@datdhruv
Copy link
Contributor

#3326 and #3327 should take care of this, so we can close this

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

No branches or pull requests

2 participants