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...
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -76,12 +76,251 @@
> #
> # git@xxxxxxxxxxxxxxx
> #
> +# 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 ...]
> @@ -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
?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html