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

feat(cli.rs) Shell completions #4537

Merged
merged 26 commits into from
May 25, 2023
Merged

feat(cli.rs) Shell completions #4537

merged 26 commits into from
May 25, 2023

Conversation

JonasKruckenberg
Copy link
Contributor

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Docs
  • New Binding issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change?

  • Yes, and the changes were approved in issue #___
  • No

Checklist

  • When resolving issues, they are referenced in the PR's title (e.g fix: remove a typo, closes #___, #___)
  • A change file is added if any packages will require a version bump due to this PR per the instructions in the readme.
  • I have added a convincing reason for adding this feature, if necessary

Other information

This adds a subcommand that can generate shell completions for the following shells:

  • bash
  • zsh
  • powershell
  • fish
  • elvish

@JonasKruckenberg JonasKruckenberg requested a review from a team as a code owner June 30, 2022 12:07
@lucasfernog
Copy link
Member

🕺

@JonasKruckenberg
Copy link
Contributor Author

There's a fix in that PR for a panick the the signer sign command, but I cherry picked that to #4538 so we can have the fix even without any new features being merged into v1

@lucasfernog
Copy link
Member

Ahh but I wanted to merge this :(((((((((((((( it's a DX feature and the developer can check the script before enabling it.

@lucasfernog
Copy link
Member

I only tested this on Linux so far.

@JonasKruckenberg
Copy link
Contributor Author

I tested on macOS too, maybe we can can signoff from secdude and @nothingismagick regardless. I think this would be pretty uncontroversial. Even though it adds another dependency sooo idk :/

@lucasfernog
Copy link
Member

I couldn't get it to work on macOS, specially we need it to work when installing with cargo and running with cargo tauri

@JonasKruckenberg
Copy link
Contributor Author

True yeah there are many situations where this doesn't work. Let me look into it

@simonhyll
Copy link
Sponsor Contributor

Bit late to the party but for me it works just fine in bash and I'd love to see this merged even if it doesn't work for all shells. I feel it's better to merge a feature that works for some (especially after having sat on the shelf this long) and we can worry about fixing it for non-bash shells later.

simonhyll
simonhyll previously approved these changes May 5, 2023
Copy link
Sponsor Contributor

@simonhyll simonhyll left a comment

Choose a reason for hiding this comment

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

Works on Bash just fine and I feel we can fix other shells later.

@simonhyll
Copy link
Sponsor Contributor

simonhyll commented May 6, 2023

To use it just run tauri completions --shell bash > tauri.sh, inspect the file for malicious code, move the file to the right place sudo mv tauri.sh /etc/bash_completion.d/tauri

Or, if you're brave, sudo tauri completions --shell bash > /etc/bash_completion.d/tauri

@lucasfernog
Copy link
Member

For reference here's what I did to use it on ZSH:

Append the following to .zshrc:

fpath=(~/.completions $fpath)
autoload -U compinit

Run the following:

tauri completions -s zsh > completions.zsh
mv completions.zsh $HOME/.completions/_tauri

@lucasfernog
Copy link
Member

Found a way to fix PowerShell. Note that to use the completions subcommand redirecting output to a file, you must use yarn -s (same flag for other package managers) to omit the yarn output.

@lucasfernog
Copy link
Member

lucasfernog commented May 25, 2023

@JonasKruckenberg @simonhyll I've pushed some changes here to make it work on Bash/Zsh/PowerShell. I know you tested Bash already, but I had to push the underscore thing to fix it on my end. Let me know if you spot any issues.

Currently the command outputs the completion script for all package managers (cargo, yarn, npm, pnpm), but we could make it only generate the completion for the current manager too (reduces a lot of the script for non-bash shells).

EDIT: actually I don't need all that replace hack, that's better.

lucasfernog
lucasfernog previously approved these changes May 25, 2023
Copy link
Member

@lucasfernog lucasfernog left a comment

Choose a reason for hiding this comment

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

I looooooooooooooooooove this

@simonhyll
Copy link
Sponsor Contributor

I looooooooooooooooooove this

Ikr?? It's amazing :D

Personally I don't think it matters that the output is slightly longer, the default behavior outputting all of them I think is desirable at least. If we want to limit the output to only specific package managers I'd prefer it if we made an option for manually specifying which to output.

So something like tauri completions --shell bash --managers npm,cargo,yarn,custom-pm (we dont have to verify the entered managers, just loop them, makes it more future proof when new package managers come out)
Maybe with an extra tauri completions --shell bash --current-manager

@lucasfernog lucasfernog merged commit 6c5ade0 into dev May 25, 2023
9 checks passed
@lucasfernog lucasfernog deleted the shell-completions branch May 25, 2023 14:34
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.

None yet

3 participants