Shell completion command

[MitchellBerend]

Resolves #331

This pr aims to add a shell completion command so users can add a command to their shell start up script. It currently supports bash, elvish, fish, powershell and zsh.

This pr is initially opened as a draft, so there are still a number of tasks that need to be done.

• suggest command to shell start up script
  • bash documentation
    - [ ] elvish documentation
  • include a link to the elvish not implemented issue
  • fish documentation
  • powershell documentation
  • fish zsh documentation
    - [ ] add a rename flag so the binary can be renamed by the user
• add docstring for main completion command
• automatically add the completion function to the correct shell init script
  - [ ] Implement elvish installation
  - [ ] Implement elvish uninstallation
  - [ ] Implement powershell installation
  - [ ] Implement powershell uninstallation

[MitchellBerend]

@cnpryer On a different project I also added a --rename flag that takes a string so the shell completion script can get generated with a different binary name. Is this something that's also a nice to have on this feature? I added the task initially, but on further inspection the binary name is currently not super ambiguous. The other project had its binary named tool and I also rename rg to ripgrep for instance.

[MitchellBerend]

Another question, do you have any experience with the elvish shell? I don't actually know what the steps for adding the shell completion are for this.

Besides these 2 questions, I think this pr is almost done.

[cnpryer]

I also added a --rename flag that takes a string so the shell completion script can get generated with a different binary name

I'm not sure this is something I'd like to add. Couldn't the user just create an alias?

Another question, do you have any experience with the elvish shell?

I do not.

Thanks for the PR! I'll give this a review later today.

[MitchellBerend]

I'm not sure this is something I'd like to add. Couldn't the user just create an alias?
They can actually, TIL.

As for the elvish part, I think I will also put a placeholder text in the suggestion. Maybe to a specific issue even.

[MitchellBerend]

There probably also needs to be a more clear separation between the script and the suggestion on how to add the script to the users rc file.

I was thinking something along the lines of

some shell script stuff that makes the completion work
############################################
add the following to your bashrc file

`eval "$(huak completion bash)"`
############################################

since it currently looks like this

$ cargo run -- completion bash
...
        huak__version)
            opts="-h --help"
            if [[ ${cur} == -* || ${COMP_CWORD} -eq 2 ]] ; then
                COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
                return 0
            fi
            case "${prev}" in
                *)
                    COMPREPLY=()
                    ;;
            esac
            COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") )
            return 0
            ;;
    esac
}

complete -F _huak -o bashdefault -o default huak
"First, ensure that you install `bash-completion` using your package manager.
After, add this to your `~/.bash_profile`:

`eval "$(huak completion bash --rename huak)"`

What do you think?

[cnpryer]

Do you mean adding it as a suggestion style behavior or documentation for it? I'd think documenting it somewhere would suffice, but I haven't thought much about it yet.

[MitchellBerend]

This would be styling, imo it's not super clear where the script stops and the suggestion starts. Unless you redirect stdout to /dev/null you will see some part of the script as well.

[cnpryer]

I'm a fan of less chatty CLI. Do you think we could get away with just adding usage docs to docs.rs in our lib.rs?

I'll try giving this a spin later today to see if I'm missing something.

[MitchellBerend]

Thats definitely possible, is the README.md a mirror of documentation in src/huak/lib.rs? If so I'll also update that.

[cnpryer]

is the README.md a mirror of documentation in src/huak/lib.rs?

No. The readme is meant to be high level. I would like for everyone to read the readme, so adding too much there I think disincentivizes that.

Good question though.

[MitchellBerend]

Ah okay, I'll remove that section tomorrow then.

[cnpryer]

Hmm I think I understand better. I think I originally thought this command would setup the configuration, but it looks like we're just returning the configuration for the user to the add manually.

I'm not familiar with clap_complete. Just thinking out loud maybe it'd be cool if we can just add the configuration using the command.

Also, this seems like more of an app-level command for huak. So I'm wondering if we should nest commands like this under something like self similar to how poetry does it. I'm open to alternatives. Never been too much of a fan of self specifically.

Otherwise thanks again for the PR. Looking great!

[MitchellBerend]

Also, this seems like more of an app-level command for huak. So I'm wondering if we should nest commands like this under something like self similar to how poetry does it. I'm open to alternatives. Never been too much of a fan of self specifically.

Im not familiar with poetry actually. After reading the docs quickly the self command seems to look like a command that groups all commands related to the management of poetry itself?

Since there are already a lot of commands in huak that's definitely something that could help with an explosion of commands in the top level. I think this should be fairly straightforward to implement as well. 2 questions on this though.

Do you want this subcommand to also be called self?

Do you have other commands that could be moved under this "category"?

Its probably a good idea to open a separate issue for all of those as well if that's the case since those seem out of scope for this issue.

[cnpryer]

I personally don't like self but I don't have an alternative off the top of my head. Here are some ideas:

1. cli
2. config
3. settings

I guess if I'd have to choose I'd choose config, but I feel like cli is broader and can't think of any purpose other than management of "the cli application huak".

No other commands implemented so far should be put under that subcommand afiak. I can double check in a bit.

[MitchellBerend]

@cnpryer can you check the changes I made, I'm not sure if these tests suffice since they only test if the file gets created and cleaned up. Only the bash version checks for content.

[cnpryer]

Not sure if there was reasoning to throwing the Config command enum in completion.rs, but I'd think this would be more appropriate in mod.rs. Wydt?

Made a few other comments.

[cnpryer]

If you'd like for me to wrap this PR lmk. I hate to be so picky. Appreciate the help!

[MitchellBerend]

No I think the iteration is good, I'd rather have the pr take a little longer than it to be half baked. I'll be honest this feature did have some scope creep but all the feedback has been productive.

[MitchellBerend]

There are still the elvish and powershell implementations that need to be done. I'm not sure if these need to also be implemented in this pr since I have no way of testing these.

[cnpryer]

There are still the elvish and powershell implementations that need to be done. I'm not sure if these need to also be implemented in this pr since I have no way of testing these.

No big deal if you want to leave those unimplemented. Maybe just add that to usage feedback. So if someone tries using that mention it's currently unimplemented and if they'd like the feature to submit an issue.

[messense]

IMO huak should not worry about shell completion (un)installation, that should be done by end user or distro packaging. It should just provide a way to output shell completion to stdout.

Here are some distro packaging with shell completions examples:

• https://github.com/Homebrew/homebrew-core/blob/89ac56e4c8b0cee300629283fa04ddba27e96d24/Formula/maturin.rb#L24
• https://github.com/alpinelinux/aports/blob/d8a204d5101ae041c5b92bb68e3d2805072283a6/community/maturin/APKBUILD#L35-L37

[MitchellBerend]

The implementation currently distinguishes between supported shells but it always outputs the same script. The initial implementation just simply printed the script to stdout iirc. Maybe its a good idea to have a clean huak config completion and have the --install and --uninstall flags as optional with a required --shell flag if either of those are passed?

[cnpryer]

Well I don't want to add any behavior that wouldn't be standard.

I also don't want to make you rip this apart. So if you feel comfortable with the way this is currently implemented, let me know and I'll come in and finalize this later today.

Are you looking for the hacktoberfest credit?

[MitchellBerend]

Are you looking for the hacktoberfest credit?

No

As far as ripping it apart goes, I think these things can exist side by side. Have huak config completion print to stdout and have huak config completion -i -s bash install the completion for bash manually.

[cnpryer]

Yea personally I don't have a problem with that. If the default behavior is to print to stdout then we're good. My comment here #332 (comment) is wrong. So I just wanted to make sure we're on the same page with this feedback.

[MitchellBerend]

@cnpryer This is not a super important but currently if a user runs huak config completion -i there will be a small message printed to stderr like this. I think the rest of the pr is good to go though.

$ huak config completion -i
Please provide a shell

[cnpryer]

@cnpryer This is not a super important but currently if a user runs huak config completion -i there will be a small message printed to stderr like this. I think the rest of the pr is good to go though.

$ huak config completion -i
Please provide a shell

Would this be difficult to remove? Less chatty the better imo and iirc from the convo shell isn't always required. Correct me if I'm wrong.

[MitchellBerend]

Its only required if --install or --uninstall is passed and only outputs it then. I can still remove it though so it just passes silently.

[cnpryer]

Thanks for the PR!