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

Shell completion script #133

Closed
MitchellBerend opened this issue Oct 9, 2022 · 11 comments · Fixed by #134
Closed

Shell completion script #133

MitchellBerend opened this issue Oct 9, 2022 · 11 comments · Fixed by #134
Labels
enhancement New feature or request question Further information is requested
Milestone

Comments

@MitchellBerend
Copy link
Collaborator

MitchellBerend commented Oct 9, 2022

The clap documentation points to clap_completion as a related project. Just like tool-sync can generate a default .tool.toml I think it's a good idea for it to be able to generate shell completion scipts for commonly used shells (Bash, fish, zsh for example). clap_complete supports these out of the box.

It would add a new dependency to the project, but I think for what it provides that is more than worth it. I think implementing this should be straight forward, but the help text for this command should also contain instructions as to how this completion should be used.

Edit: blocked by #136 Unblocked by @SanchithHegde

@chshersh chshersh added enhancement New feature or request question Further information is requested labels Oct 9, 2022
@chshersh
Copy link
Owner

chshersh commented Oct 9, 2022

I don't mind adding a separate dependency. Shell completion is a nice feature 👍🏻

I haven't used this crate though. How exactly does it work? Does it produce separate artifacts?


On a similar note, tool-sync itself can't configure shell completion of tools upon installation which is a bit sad 😞 And I currently don't know the best way to do this.

@MitchellBerend
Copy link
Collaborator Author

Does it produce separate artifacts?

The way I use it right now is that it just prints the script to stdout. I added the like eval "$(tool completion)" to my ~/.bashrc so it is persistent. Im not sure if this is the standard way for cli auto completion but the github cli has the user add the completion script to their ~/.bashprofile.

$ gh completion --help
Generate shell completion scripts for GitHub CLI commands.

When installing GitHub CLI through a package manager, it's possible that
no additional shell configuration is necessary to gain completion support. For
Homebrew, see <https://docs.brew.sh/Shell-Completion>

If you need to set up completions manually, follow the instructions below. The exact
config file locations might vary based on your system. Make sure to restart your
shell before testing whether completions are working.

### bash

First, ensure that you install `bash-completion` using your package manager.

After, add this to your `~/.bash_profile`:

	eval "$(gh completion -s bash)"
...

I think clap_completion allows for a selection of options like the actual commands, flags or files in the cwd. Im still playing around with this myself so I can't give a definitive answer on how polished suggestions can be.

@MitchellBerend
Copy link
Collaborator Author

There are some follow up questions that need to be addressed before this is actually implemented all the way.

clap_complete assumes the name of the tool will be the same as the name defined in the Cargo.toml, this is currently tool-sync but the binary is called tool.

I personally rename the binary by hand so this will work for me as is, but for other users that might not be the case. A decision should be made on this naming convention before more work is done.

I also did not find a way to create a custom list for suggestions for the install command, this could be solved by making a new enum that contains all the hardcoded tools. This would accrue a decent amount of maintenance load though since this enum, besides the actual hardcoded tools and config template, also has to be kept up to date.

I found this issue that talks about a similar feature so I don't think this is currently possible.

@MitchellBerend MitchellBerend added the blocked This issue is blocked by another issue label Oct 10, 2022
@chshersh
Copy link
Owner

The way I use it right now is that it just prints the script to stdout. I added the like eval "$(tool completion)" to my ~/.bashrc

That's great to hear! 👏🏻 This is how it works in Haskell as well and how I enable completion on my own too 🙂

clap_complete assumes the name of the tool will be the same as the name defined in the Cargo.toml, this is currently tool-sync but the binary is called tool.

This is unfortunate 😞
Not sure what to do with this now. So, I guess we'll wait for the upstream issue resolution.

@MitchellBerend
Copy link
Collaborator Author

MitchellBerend commented Oct 11, 2022

For transparency's sake, it is possible to set a custom binary name for clap_complete by passing a String or &str to the generate function.

@MitchellBerend
Copy link
Collaborator Author

Since #136 is resolved thanks to @SanchithHegde, this issue can also be implemented.

There is still the outstanding issue of how the naming convention should be for referring to tool-sync. I think it would be better if we used tool-sync as the name this would be less ambiguous. This would mean a change to the ci configuration and also updating the README.md.

We can also keep tool as the name but, if that is the path forward, the Cargo.toml name should also reflect this. That would also mean a minor change in the src/main.rs file since that refers to the library name.

Other naming conventions are also welcome of course but these came to mind quickly.

Changing the name later would also mean a breaking change.

@MitchellBerend MitchellBerend removed the blocked This issue is blocked by another issue label Oct 14, 2022
@chshersh
Copy link
Owner

@MitchellBerend The conversation about naming conventions is spread across multiple places so I'm going to reply here.

I've replied about the convention for the environment variables names in the corresponding issue:

Could you clarify a bit more about the problem with the naming convention? I'm happy to call the binary tool (especially considering the fact that shell completion works great with that) and use tool-sync in all other places when we refer to the CLI tool / project to avoid ambiguity. Both crate and repository already have the name tool-sync so I would happily accept any changes to README.md to make the situation clearer 🙂

@MitchellBerend
Copy link
Collaborator Author

Could you clarify a bit more about the problem with the naming convention?

Because the completion script needs to actually be ran every time a new shell without a parent is spawned my question was if we are using the name tool or tool-sync. If the user renames the binary themselves this autocomplete feature breaks for them.

The way I'm reading your comment you want this to be tool?

@chshersh
Copy link
Owner

@MitchellBerend Thanks for clarifying! I see the problem now 🙂

I would like to keep the name tool indeed. I'm not aware of any other tool that is named just tool so this seems good to me 😅
We can put a warning in the README.md saying that autocompletion won't work if you rename the binary.

It would be better if clap supported several names for the autocompletion as potential common aliases for the tool.

It would be even better if users could configure the name for themselves so we don't need to support all possible use cases in our repo.

But looks like the two last options are not possible, if I'm reading your comments correctly.

@MitchellBerend
Copy link
Collaborator Author

MitchellBerend commented Oct 15, 2022

It would be better if clap supported several names for the autocompletion as potential common aliases for the tool.

I think we can support this by adding an optional flag possibly. There needs to be a special output to the stderr maybe since the command that gets run at first shell start up needs to hold only the completion script.

Something like

fn generate_custom_completion(bin_name: String, shell: clap_complete::Shell) {
	let cmd = Cli::commands();
	eprintln!(format!(r###"Add this to your ./bashrc:\n\teval "$({bin_name} completion --rename {bin_name})""###));

	clap_complete::generate(shell, &mut cmd, bin_name, std::io::stdout());
}

Edit: corrected eprintln suggestion

@MitchellBerend
Copy link
Collaborator Author

There needs to be a special output to the stderr maybe since the command that gets run at first shell start up needs to hold only the completion script.

This ended up being pretty easy to implement but there are still a few issues.

  1. Elvish shell does not give a suggestion as to how the completion script should be added.
  2. The match statement looks very messy to me, maybe the src/lib is not the right file to implement this.
mitchell@mitchell-workstation:~/rust/tool-sync$ cargo run -- completion bash --rename asdfasdf -- $1 > /dev/null 
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/tool completion bash --rename asdfasdf --`
First, ensure that you install `bash-completion` using your package manager.

After, add this to your `~/.bash_profile`:

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

----------------------------------------------------------------------------------------------------------------------------------------------------------------

mitchell@mitchell-workstation:~/rust/tool-sync$ cargo run -- completion fish --rename asdfasdf -- $1 > /dev/null 
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/tool completion fish --rename asdfasdf --`
Generate a `tool.fish` completion script:

`asdfasdf completion fish --rename asdfasdf > ~/.config/fish/completions/asdfasdf.fish`

----------------------------------------------------------------------------------------------------------------------------------------------------------------

mitchell@mitchell-workstation:~/rust/tool-sync$ cargo run -- completion zsh --rename asdfasdf -- $1 > /dev/null 
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/tool completion zsh --rename asdfasdf --`
Generate a `_asdfasdf` completion script and put it somewhere in your `$fpath`:
`asdfasdf completion zsh --rename asdfasdf > /usr/local/share/zsh/site-functions/_asdfasdf`

Ensure that the following is present in your `~/.zshrc`:

`autoload -U compinit`

`compinit -i`

----------------------------------------------------------------------------------------------------------------------------------------------------------------

mitchell@mitchell-workstation:~/rust/tool-sync$ cargo run -- completion powershell --rename asdfasdf -- $1 > /dev/null 
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/tool completion powershell --rename asdfasdf --`
Open your profile script with:

`mkdir -Path (Split-Path -Parent $profile) -ErrorAction SilentlyContinue`
`notepad $profile`

Add the line and save the file:

`Invoke-Expression -Command $(asdfasdf completion powershell --rename asdfasdf | Out-String)`

----------------------------------------------------------------------------------------------------------------------------------------------------------------

mitchell@mitchell-workstation:~/rust/tool-sync$ cargo run -- completion elvish --rename asdfasdf -- $1 > /dev/null 
    Finished dev [unoptimized + debuginfo] target(s) in 0.04s
     Running `target/debug/tool completion elvish --rename asdfasdf --`
This suggestion is missing, if you use this and know how to implement this please file an issue over at https://github.com/chshersh/tool-sync/issues

@chshersh chshersh added this to the v0.3.0: Auto milestone Oct 19, 2022
chshersh added a commit that referenced this issue Oct 20, 2022
Resolves #133 

This pr aims to add a tab completion script so users don't have to
remember every command if they want to experiment with config options.

### Additional tasks

- [x] Documentation for changes provided/changed
- [x] Tests added
- [x] Updated CHANGELOG.md

Co-authored-by: Dmitrii Kovanikov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants