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

fix Bash completion on macOS #2074

Merged
merged 4 commits into from
Feb 24, 2022

Conversation

joshpencheon
Copy link
Contributor

Issue #2066 reports that bash completion is broken on macOS. This is because the version of bash-completion that's often packed on macOS via Home-brew is v1.3, which doesn't include the missing _init_completion function.

This PR adopts a pattern seen across many other projects (e.g. tmux/ gh / kubectl / helm, some of which use the Cobra Go library to autogenerate their completion) to add a shim function with the minimal required functionality.

Completion now works for me on both macOS 10.15.7 and Ubuntu 22.04.

The Homebrew-provided bash-completion is version 1.x,
which doesn't provide _init_completion. We add a standard
shim instead.
@sharkdp
Copy link
Owner

sharkdp commented Feb 13, 2022

Thank you very much for your contribution!

@scop Would you mind having a short look at this?

Copy link
Contributor

@scop scop left a comment

Choose a reason for hiding this comment

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

Looks quite concise and ok to me, if support for such old versions is desired. Not tested as I don't have easy access to them. (An alternative would be to be more accurate that we require bash-completion >= 2, but that implies bash >= 4 and causes some extra complications for macOS users as noted in my comment about the changelog entry.)

I haven't examined the shim in depth, but based on the references it appears to be quite battle tested.

__bat_init_completion -n "=" || return 0
fi

_split_longopt && split=true
Copy link
Contributor

Choose a reason for hiding this comment

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

This should go in the above __bat_init_completion block or be done within its body, it's unnecessary with bash-completion 2+ and its _init_completion.

CHANGELOG.md Outdated Show resolved Hide resolved
joshpencheon and others added 2 commits February 13, 2022 17:36
Co-authored-by: Ville Skyttä <ville.skytta@iki.fi>
@joshpencheon
Copy link
Contributor Author

Thanks for taking a look @scop! I've made the suggestions you gave.

Agreed this is supporting older setups, but I think it's probably relatively common amongst e.g. macOS users that haven't switched to using ZSH - a crude indicator, but bash-completion@1 is getting installed from Homebrew at 4x the rate of bash-completion@2.

@sharkdp
Copy link
Owner

sharkdp commented Feb 13, 2022

Thanks! Both of you!

@joshpencheon
Copy link
Contributor Author

@sharkdp are you happy for me to merge this PR, or do you follow a different process? Thanks!

@Enselic
Copy link
Collaborator

Enselic commented Feb 24, 2022

I am personally not well-versed enough in bash completion to make a judgement call, but I fully trust the rest of you.

Merging, so that this will be included in the upcoming 0.19.1 release.

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.

4 participants