-
-
Notifications
You must be signed in to change notification settings - Fork 12.5k
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
mise: fix shell completion #201741
base: master
Are you sure you want to change the base?
mise: fix shell completion #201741
Conversation
64b21fd
to
e70853e
Compare
mise shell completion requires the `--include-bash-completion-lib` option, as documented [here](https://mise.jdx.dev/cli/completion.html#include-bash-completion-lib)
e70853e
to
50778e2
Compare
@@ -42,7 +42,7 @@ def install | |||
|
|||
system "cargo", "install", *std_cargo_args | |||
man1.install "man/man1/mise.1" | |||
generate_completions_from_executable(bin/"mise", "completion") | |||
generate_completions_from_executable(bin/"mise", "completion", "--include-bash-completion-lib") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will break for zsh and fish, only required for bash right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just tested this and it only adds the extra libs if bash is specified.
So no, it doesn't break for other shells.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this something that belongs in the user's .bash.rc? Looks like it's optional so the same library isn't sourced over-and-over at shell initialization. Maybe some clarification on what @jdx's (upstream) intention was with this flag would be helpful?
Additionally, does this work for both bash 3 and 5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought homebrew had this as a separate package. How do other CLI's include it? I wouldn't think you'd want my vendored version
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears there are two bash-completion recipes, both sourced from https://github.com/scop/bash-completion
- bash-completion (v1.3, for bash versions < 4.2)
- bash-completion@2 (v2.15.0 for bash versions 4.2+)
I imagine it may be difficult to optionally install one of these packages when mise is installed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, I can confirm that installing bash-completion@2
fixed mise completion for me (I'm on macos, but use bash 5 as my shell, installed from Homebrew).
HOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>
, where<formula>
is the name of the formula you're submitting?brew test <formula>
, where<formula>
is the name of the formula you're submitting?brew audit --strict <formula>
(after doingHOMEBREW_NO_INSTALL_FROM_API=1 brew install --build-from-source <formula>
)? If this is a new formula, does it passbrew audit --new <formula>
?