Import bash-completion slightly more carefully #1908
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NoahGorny
approved these changes
Aug 3, 2021
There was a problem hiding this comment.
Another nice idea- I will let @davidpfarrell take a look as well and merge if he also approves 😄
|
Anything else needed for this PR? 😃 |
Instead of using the profile.d version, just invoke the script. The profile.d script preemptively short-circuits if it thinks that bash-completions has already been loaded, which it does by using the $BASH_COMPLETION variable, which is expressly supported by upstream to specify the location of the script...so it will entirely be never loaded if this is set.
Bash-completion supports pre-defining $BASH_COMPLETION as the path to the main script, so use that if it's defined. Alsö, don't load homebrew's completion if we've successfully loaded one already.
gaelicWizard
force-pushed
the
bash_completion
branch
from
018252d to
893c8db
Compare
|
nope- well done! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Uses the
$BASH_COMPLETIONvariable to locate the script if defined, falling back to the existing waterfall. For the final case (Homebrew), use$OSTYPEinstead of running an external command, and then import the script itself rather than the wrapper in theprofile.dfolder.Motivation and Context
The bash-completion script itself allows for
$BASH_COMPLETION,$BASH_COMPLETION_COMPAT_DIR, and$BASH_COMPLETION_DIRto be set prior to invocation and in fact at least$BASH_COMPLETION_COMPAT_DIRneeds to be in order to be useful. This can allow for different versions to be used when, for example, Bash is v4+ or not.However, the wrapper script in
.../etc/profile.d/bash_completion.shon many distributions actually short-circuits if$BASH_COMPLETIONis defined. When theprofile.dwrapper is used while$BASH_COMPLETIONis already defined, bash-completion is simply never once loaded as the wrapper and the actual script disagree about this variable.How Has This Been Tested?
This is live on my branch on my computer currently.
Types of changes
Checklist:
clean_files.txtand formatted it usinglint_clean_files.sh.