Re: [PATCH v2/RFC] Make git-completion Bash 4 compatible.
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Oct 27, 2010, 17:31 UTC
- Message-ID
- <20101027173132.GA15657@burratino>
- In-Reply-To
- <20101027131506.4da06c6d@MonteCarlo>
Hi Peter,
Peter van der Does wrote:
> The completion script does not work as expected under Bash 4.
Thanks for your work fixing this. That's awesome.
It would be ideal if someone could write or find a nice summary of the problem and the chosen solution, for inclusion in the commit message.
Could some zsh user perhaps test that the new zsh support is not broken?
> 1 files changed, 355 insertions(+), 62 deletions(-)
Kind of unfortunate. There are a lot of comments, but still...
Show 7 quoted lines
> --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -76,12 +76,251 @@ > # > # git@vger.kernel.org > # > +# Updated for Bash 4.0
I don't think this comment will be so important for posterity (e.g., once bash 5 comes around ;-)).
[...]
> +# If the function _get_comp_words_by_ref does not exists, we can assume the > +# bash_completion 1.2 script isn't loaded and therefor we're defining the > +# necessary functions ourselves.
Probably this explanation belongs in the commit message? A comment could provide a brief reminder, like:
if ! type _get_comp_words_by_ref &>/dev/null ; then # The bash_completion 1.2 library was not loaded, # so we have to define some functions from it ourselves.
Are the implementations taken from bash_completion? If so, that would be very useful information for the log message: future readers may want to know where to look for a more recent version.
> + # Assign variable one scope above the caller
[... I'm assuming this is all written correctly, etc ...]
Show 7 quoted lines
> @@ -331,7 +570,8 @@ __gitcomp_1 ()
> # generates completion reply with compgen
> __gitcomp ()
> {
> - local cur="${COMP_WORDS[COMP_CWORD]}"
> + local cur
> + _get_comp_words_by_ref -n "=" cur[...]
The rest looks sane. Maybe it would make sense to split this into two patches for readability:
- one to introduce the _get_comp_words_by_ref function - one to use it
?