threads / patch / 27195

patchgit-completion: fix zsh support

Subject: [PATCH] git-completion: fix zsh support

## tl;dr

28 messages between Apr 27, 2011 and May 8, 2011. Diffs are folded; open one to read it.

replies: 27people: 5as markdown or json

Felipe Contreras· Apr 27, 2011, 01:26 UTC · lore

It turns out 'words' is a special variable used by zsh completion, and it has some strange behavior as we can see.

Better avoid it.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |   66 ++++++++++++++++----------------
 1 files changed, 33 insertions(+), 33 deletions(-)
Show changes to contrib/completion/git-completion.bash +33 −33
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b94ff3c..9aae484 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -447,8 +447,8 @@ _get_comp_words_by_ref ()
 		prev)
 			prev=${words_[$cword_-1]}
 			;;
-		words)
-			words=("${words_[@]}")
+		cwords)
+			cwords=("${words_[@]}")
 			;;
 		cword)
 			cword=$cword_
@@ -468,8 +468,8 @@ _get_comp_words_by_ref ()
 		prev)
 			prev=${COMP_WORDS[COMP_CWORD-1]}
 			;;
-		words)
-			words=("${COMP_WORDS[@]}")
+		cwords)
+			cwords=("${COMP_WORDS[@]}")
 			;;
 		cword)
 			cword=$COMP_CWORD
@@ -739,12 +739,12 @@ __git_complete_revlist ()
 
 __git_complete_remote_or_refspec ()
 {
-	local cur words cword
-	_get_comp_words_by_ref -n =: cur words cword
-	local cmd="${words[1]}"
+	local cur cwords cword
+	_get_comp_words_by_ref -n =: cur cwords cword
+	local cmd="${cwords[1]}"
 	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
 	while [ $c -lt $cword ]; do
-		i="${words[c]}"
+		i="${cwords[c]}"
 		case "$i" in
 		--mirror) [ "$cmd" = "push" ] && no_complete_refspec=1 ;;
 		--all)
@@ -991,10 +991,10 @@ __git_aliased_command ()
 # __git_find_on_cmdline requires 1 argument
 __git_find_on_cmdline ()
 {
-	local word subcommand c=1 words cword
-	_get_comp_words_by_ref -n =: words cword
+	local word subcommand c=1 cwords cword
+	_get_comp_words_by_ref -n =: cwords cword
 	while [ $c -lt $cword ]; do
-		word="${words[c]}"
+		word="${cwords[c]}"
 		for subcommand in $1; do
 			if [ "$subcommand" = "$word" ]; then
 				echo "$subcommand"
@@ -1007,10 +1007,10 @@ __git_find_on_cmdline ()
 
 __git_has_doubledash ()
 {
-	local c=1 words cword
-	_get_comp_words_by_ref -n =: words cword
+	local c=1 cwords cword
+	_get_comp_words_by_ref -n =: cwords cword
 	while [ $c -lt $cword ]; do
-		if [ "--" = "${words[c]}" ]; then
+		if [ "--" = "${cwords[c]}" ]; then
 			return 0
 		fi
 		c=$((++c))
@@ -1135,11 +1135,11 @@ _git_bisect ()
 
 _git_branch ()
 {
-	local i c=1 only_local_ref="n" has_r="n" cur words cword
+	local i c=1 only_local_ref="n" has_r="n" cur cwords cword
 
-	_get_comp_words_by_ref -n =: cur words cword
+	_get_comp_words_by_ref -n =: cur cwords cword
 	while [ $c -lt $cword ]; do
-		i="${words[c]}"
+		i="${cwords[c]}"
 		case "$i" in
 		-d|-m)	only_local_ref="y" ;;
 		-r)	has_r="y" ;;
@@ -1167,9 +1167,9 @@ _git_branch ()
 
 _git_bundle ()
 {
-	local words cword
-	_get_comp_words_by_ref -n =: words cword
-	local cmd="${words[2]}"
+	local cwords cword
+	_get_comp_words_by_ref -n =: cwords cword
+	local cmd="${cwords[2]}"
 	case "$cword" in
 	2)
 		__gitcomp "create list-heads verify unbundle"
@@ -1713,15 +1713,15 @@ _git_notes ()
 {
 	local subcommands='add append copy edit list prune remove show'
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
-	local cur words cword
-	_get_comp_words_by_ref -n =: cur words cword
+	local cur cwords cword
+	_get_comp_words_by_ref -n =: cur cwords cword
 
 	case "$subcommand,$cur" in
 	,--*)
 		__gitcomp '--ref'
 		;;
 	,*)
-		case "${words[cword-1]}" in
+		case "${cwords[cword-1]}" in
 		--ref)
 			__gitcomp "$(__git_refs)"
 			;;
@@ -1749,7 +1749,7 @@ _git_notes ()
 	prune,*)
 		;;
 	*)
-		case "${words[cword-1]}" in
+		case "${cwords[cword-1]}" in
 		-m|-F)
 			;;
 		*)
@@ -1893,11 +1893,11 @@ _git_stage ()
 
 __git_config_get_set_variables ()
 {
-	local words cword
-	_get_comp_words_by_ref -n =: words cword
+	local cwords cword
+	_get_comp_words_by_ref -n =: cwords cword
 	local prevword word config_file= c=$cword
 	while [ $c -gt 1 ]; do
-		word="${words[c]}"
+		word="${cwords[c]}"
 		case "$word" in
 		--global|--system|--file=*)
 			config_file="$word"
@@ -2665,10 +2665,10 @@ _git_svn ()
 _git_tag ()
 {
 	local i c=1 f=0
-	local words cword prev
-	_get_comp_words_by_ref -n =: words cword prev
+	local cwords cword prev
+	_get_comp_words_by_ref -n =: cwords cword prev
 	while [ $c -lt $cword ]; do
-		i="${words[c]}"
+		i="${cwords[c]}"
 		case "$i" in
 		-d|-v)
 			__gitcomp "$(__git_tags)"
@@ -2712,10 +2712,10 @@ _git ()
 		setopt KSH_TYPESET
 	fi
 
-	local cur words cword
-	_get_comp_words_by_ref -n =: cur words cword
+	local cur cwords cword
+	_get_comp_words_by_ref -n =: cur cwords cword
 	while [ $c -lt $cword ]; do
-		i="${words[c]}"
+		i="${cwords[c]}"
 		case "$i" in
 		--git-dir=*) __git_dir="${i#--git-dir=}" ;;
 		--bare)      __git_dir="." ;;
-- 
1.7.5
Jonathan Nieder· Apr 27, 2011, 01:35 UTC · re: Felipe Contreras · lore

Re: [PATCH] git-completion: fix zsh support

Felipe Contreras wrote:
> It turns out 'words' is a special variable used by zsh completion, and
> it has some strange behavior as we can see.
> 
> Better avoid it.

Hoorah! I imagine this fixes a regression introduced by v1.7.4-rc0~11^2~2 (bash: get --pretty=m<tab> completion to work with bash v4, 2010-12-02).

 Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Show 5 quoted lines
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  contrib/completion/git-completion.bash |   66 ++++++++++++++++----------------
>  1 files changed, 33 insertions(+), 33 deletions(-)

Stefan and Mark, if you'd like to try this out, the patch is at http://download.gmane.org/gmane.comp.version-control.git/172142/172143

Happily, Jonathan

Felipe Contreras· Apr 27, 2011, 01:42 UTC · re: Jonathan Nieder · lore

Re: [PATCH] git-completion: fix zsh support

On Wed, Apr 27, 2011 at 4:35 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 12 quoted lines
> Felipe Contreras wrote:
>
>> It turns out 'words' is a special variable used by zsh completion, and
>> it has some strange behavior as we can see.
>>
>> Better avoid it.
>
> Hoorah!  I imagine this fixes a regression introduced by
> v1.7.4-rc0~11^2~2 (bash: get --pretty=m<tab> completion to work with
> bash v4, 2010-12-02).
>
>  Acked-by: Jonathan Nieder <jrnieder@gmail.com>
Yeap... The completion wasn't working at all after that :(
-- 
Felipe Contreras
Junio C Hamano· Apr 27, 2011, 04:55 UTC · re: Jonathan Nieder · lore

Re: [PATCH] git-completion: fix zsh support

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 12 quoted lines
> Felipe Contreras wrote:
>
>> It turns out 'words' is a special variable used by zsh completion, and
>> it has some strange behavior as we can see.
>> 
>> Better avoid it.
>
> Hoorah!  I imagine this fixes a regression introduced by
> v1.7.4-rc0~11^2~2 (bash: get --pretty=m<tab> completion to work with
> bash v4, 2010-12-02).
>
>  Acked-by: Jonathan Nieder <jrnieder@gmail.com>

I'd love to share the enthusiasm, but find that "as we can see" needs a much more clarification.

Jonathan Nieder· Apr 27, 2011, 06:40 UTC · re: Junio C Hamano · lore

[RFC/PATCH] completion: avoid "words" as variable name for zsh portability

The "_get_comp_words_by_ref -n := words" command from the bash_completion library reassembles a modified version of COMP_WORDS with ':' and '=' no longer treated as word separators and stores it in the ${words[@]} array. Git's programmable tab completion script uses this to abstract away the difference between bash v3's and bash v4's definitions of COMP_WORDS (bash v3 used shell words, while bash v4 breaks at separator characters); see v1.7.4-rc0~11^2~2 (bash: get --pretty=m<tab> completion to work with bash v4, 2010-12-02).

zsh has (or rather its completion functions have) another idea about what ${words[@]} should contain: the array is prepopulated with the words from the command it is completing. For reasons that are not well understood, when git-completion.bash reserves its own "words" variable with "local words", the variable becomes empty and cannot be changed from then on. So the completion script neglects the arguments it has seen, and words complete like git subcommand names. For example, typing "git log origi<TAB>" gives no completions because there are no "git origi..." commands.

Work around this by using a different variable (comp_words) that is not special to zsh. So now commands that completed correctly before v1.7.4-rc0~11^2~2 on zsh should be able to complete correctly again.

Reported-by: Stefan Haller <lists@haller-berlin.de>
Suggested-by: Felipe Contreras <felipe.contreras@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:
> I'd love to share the enthusiasm, but find that "as we can see" needs a
> much more clarification.

Sorry, I got carried away (I am happy to see someone has made some headway in investigating this old bug). How about this?

There is still a "for unknown reasons" in the above explanation.
 contrib/completion/git-completion.bash |   68 ++++++++++++++++---------------
 1 files changed, 35 insertions(+), 33 deletions(-)
Show changes to contrib/completion/git-completion.bash +35 −33
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9150ea6..ce6b3e4 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -447,8 +447,9 @@ _get_comp_words_by_ref ()
 		prev)
 			prev=${words_[$cword_-1]}
 			;;
-		words)
-			words=("${words_[@]}")
+		-w)
+			eval $2'=("${words_[@]}")'
+			shift
 			;;
 		cword)
 			cword=$cword_
@@ -468,8 +469,9 @@ _get_comp_words_by_ref ()
 		prev)
 			prev=${COMP_WORDS[COMP_CWORD-1]}
 			;;
-		words)
-			words=("${COMP_WORDS[@]}")
+		-w)
+			eval $2='("${COMP_WORDS[@]}")'
+			shift
 			;;
 		cword)
 			cword=$COMP_CWORD
@@ -739,12 +741,12 @@ __git_complete_revlist ()
 
 __git_complete_remote_or_refspec ()
 {
-	local cur words cword
-	_get_comp_words_by_ref -n =: cur words cword
-	local cmd="${words[1]}"
+	local cur comp_words cword
+	_get_comp_words_by_ref -n =: -w comp_words cur cword
+	local cmd="${comp_words[1]}"
 	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
 	while [ $c -lt $cword ]; do
-		i="${words[c]}"
+		i="${comp_words[c]}"
 		case "$i" in
 		--mirror) [ "$cmd" = "push" ] && no_complete_refspec=1 ;;
 		--all)
@@ -991,10 +993,10 @@ __git_aliased_command ()
 # __git_find_on_cmdline requires 1 argument
 __git_find_on_cmdline ()
 {
-	local word subcommand c=1 words cword
-	_get_comp_words_by_ref -n =: words cword
+	local word subcommand c=1 comp_words cword
+	_get_comp_words_by_ref -n =: -w comp_words cword
 	while [ $c -lt $cword ]; do
-		word="${words[c]}"
+		word="${comp_words[c]}"
 		for subcommand in $1; do
 			if [ "$subcommand" = "$word" ]; then
 				echo "$subcommand"
@@ -1007,10 +1009,10 @@ __git_find_on_cmdline ()
 
 __git_has_doubledash ()
 {
-	local c=1 words cword
-	_get_comp_words_by_ref -n =: words cword
+	local c=1 comp_words cword
+	_get_comp_words_by_ref -n =: -w comp_words cword
 	while [ $c -lt $cword ]; do
-		if [ "--" = "${words[c]}" ]; then
+		if [ "--" = "${comp_words[c]}" ]; then
 			return 0
 		fi
 		c=$((++c))
@@ -1135,11 +1137,11 @@ _git_bisect ()
 
 _git_branch ()
 {
-	local i c=1 only_local_ref="n" has_r="n" cur words cword
+	local i c=1 only_local_ref="n" has_r="n" cur comp_words cword
 
-	_get_comp_words_by_ref -n =: cur words cword
+	_get_comp_words_by_ref -n =: -w comp_words cur cword
 	while [ $c -lt $cword ]; do
-		i="${words[c]}"
+		i="${comp_words[c]}"
 		case "$i" in
 		-d|-m)	only_local_ref="y" ;;
 		-r)	has_r="y" ;;
@@ -1167,9 +1169,9 @@ _git_branch ()
 
 _git_bundle ()
 {
-	local words cword
-	_get_comp_words_by_ref -n =: words cword
-	local cmd="${words[2]}"
+	local comp_words cword
+	_get_comp_words_by_ref -n =: -w comp_words cword
+	local cmd="${comp_words[2]}"
 	case "$cword" in
 	2)
 		__gitcomp "create list-heads verify unbundle"
@@ -1713,15 +1715,15 @@ _git_notes ()
 {
 	local subcommands='add append copy edit list prune remove show'
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
-	local cur words cword
-	_get_comp_words_by_ref -n =: cur words cword
+	local cur comp_words cword
+	_get_comp_words_by_ref -n =: -w comp_words cur cword
 
 	case "$subcommand,$cur" in
 	,--*)
 		__gitcomp '--ref'
 		;;
 	,*)
-		case "${words[cword-1]}" in
+		case "${comp_words[cword-1]}" in
 		--ref)
 			__gitcomp "$(__git_refs)"
 			;;
@@ -1749,7 +1751,7 @@ _git_notes ()
 	prune,*)
 		;;
 	*)
-		case "${words[cword-1]}" in
+		case "${comp_words[cword-1]}" in
 		-m|-F)
 			;;
 		*)
@@ -1893,11 +1895,11 @@ _git_stage ()
 
 __git_config_get_set_variables ()
 {
-	local words cword
-	_get_comp_words_by_ref -n =: words cword
+	local comp_words cword
+	_get_comp_words_by_ref -n =: -w comp_words cword
 	local prevword word config_file= c=$cword
 	while [ $c -gt 1 ]; do
-		word="${words[c]}"
+		word="${comp_words[c]}"
 		case "$word" in
 		--global|--system|--file=*)
 			config_file="$word"
@@ -2665,10 +2667,10 @@ _git_svn ()
 _git_tag ()
 {
 	local i c=1 f=0
-	local words cword prev
-	_get_comp_words_by_ref -n =: words cword prev
+	local comp_words cword prev
+	_get_comp_words_by_ref -n =: -w comp_words cword prev
 	while [ $c -lt $cword ]; do
-		i="${words[c]}"
+		i="${comp_words[c]}"
 		case "$i" in
 		-d|-v)
 			__gitcomp "$(__git_tags)"
@@ -2712,10 +2714,10 @@ _git ()
 		setopt KSH_TYPESET
 	fi
 
-	local cur words cword
-	_get_comp_words_by_ref -n =: cur words cword
+	local cur comp_words cword
+	_get_comp_words_by_ref -n =: -w comp_words cur cword
 	while [ $c -lt $cword ]; do
-		i="${words[c]}"
+		i="${comp_words[c]}"
 		case "$i" in
 		--git-dir=*) __git_dir="${i#--git-dir=}" ;;
 		--bare)      __git_dir="." ;;
-- 
1.7.5
Felipe Contreras· Apr 27, 2011, 08:42 UTC · re: Jonathan Nieder · lore

Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability

On Wed, Apr 27, 2011 at 9:40 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 5 quoted lines
>> I'd love to share the enthusiasm, but find that "as we can see" needs a
>> much more clarification.
>
> Sorry, I got carried away (I am happy to see someone has made some
> headway in investigating this old bug).  How about this?
What's wrong with my patch?
> There is still a "for unknown reasons" in the above explanation.

I'm asking zsh guys: http://www.zsh.org/mla/workers/2011/msg00515.html

-- 
Felipe Contreras
Jonathan Nieder· Apr 27, 2011, 09:11 UTC · re: Felipe Contreras · lore

Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability

Felipe Contreras wrote:
> On Wed, Apr 27, 2011 at 9:40 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Sorry, I got carried away (I am happy to see someone has made some
>> headway in investigating this old bug).  How about this?
>
> What's wrong with my patch?

As mentioned at http://thread.gmane.org/gmane.comp.version-control.git/172142/focus=172157 it breaks the tab completion in the common case that the user uses the standard bash completion library (usually provided at /etc/bash_completion) and git uses its _get_comp_words_by_ref function. You can test like so:

	% bash
	$ . /etc/bash_completion
	$ . contrib/completion/git-completion.bash
	$ git fetch origin <TAB>

I also made a small cosmetic change which is less important (sorry, I should have mentioned it before): the patch I sent spells out comp_words instead of writing cwords to avoid a false analogy between the array of all completion words (cwords) and the current word index (cword).

>> There is still a "for unknown reasons" in the above explanation.
>
> I'm asking zsh guys:
> http://www.zsh.org/mla/workers/2011/msg00515.html

Thanks. It looks like to get the semantics I expect from "local" in zsh, one needs to use "typeset -h" (which bash does not support, unfortunately). Probably it is best to steer clear of zsh's special variables anyway.

Felipe Contreras· Apr 27, 2011, 09:49 UTC · re: Jonathan Nieder · lore

Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability

On Wed, Apr 27, 2011 at 12:11 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 19 quoted lines
> Felipe Contreras wrote:
>> On Wed, Apr 27, 2011 at 9:40 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>
>>> Sorry, I got carried away (I am happy to see someone has made some
>>> headway in investigating this old bug).  How about this?
>>
>> What's wrong with my patch?
>
> As mentioned at
> http://thread.gmane.org/gmane.comp.version-control.git/172142/focus=172157
> it breaks the tab completion in the common case that the user uses
> the standard bash completion library (usually provided at
> /etc/bash_completion) and git uses its _get_comp_words_by_ref
> function.  You can test like so:
>
>        % bash
>        $ . /etc/bash_completion
>        $ . contrib/completion/git-completion.bash
>        $ git fetch origin <TAB>

Where does that 'bash_completion' file comes from? I don't have it on my system. Do we really need to use _get_comp_words_by_ref from there? Can't we have our own _get_comp_words_by_ref?

Show 5 quoted lines
> I also made a small cosmetic change which is less important (sorry, I
> should have mentioned it before): the patch I sent spells out
> comp_words instead of writing cwords to avoid a false analogy between
> the array of all completion words (cwords) and the current word index
> (cword).

I don't see how cword can be confused with cwords. Besides, I prefer uniformity, so if you use comp_words, cword should be cur_word, or word_index, or something like that.

Show 9 quoted lines
>>> There is still a "for unknown reasons" in the above explanation.
>>
>> I'm asking zsh guys:
>> http://www.zsh.org/mla/workers/2011/msg00515.html
>
> Thanks.  It looks like to get the semantics I expect from "local"
> in zsh, one needs to use "typeset -h" (which bash does not support,
> unfortunately).  Probably it is best to steer clear of zsh's special
> variables anyway.

Hmm, interesting, maybe we should try to find a way to replace those 'local' with 'typeset -h'.

-- 
Felipe Contreras
John Szakmeister· Apr 27, 2011, 09:59 UTC · re: Felipe Contreras · lore

Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability

On Wed, Apr 27, 2011 at 5:49 AM, Felipe Contreras <felipe.contreras@gmail.com> wrote: [snip]

> Where does that 'bash_completion' file comes from? I don't have it on
> my system. Do we really need to use _get_comp_words_by_ref from there?
> Can't we have our own _get_comp_words_by_ref?
It typically comes from the bash-completion package for your system.
Depending on your distro, it may or may not be there by default.  You
can get the source from here though:
   <http://bash-completion.alioth.debian.org/>
/me goes back to lurking.
-John
Felipe Contreras· Apr 27, 2011, 10:09 UTC · re: Felipe Contreras · lore

Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability

On Wed, Apr 27, 2011 at 12:49 PM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 8 quoted lines
> On Wed, Apr 27, 2011 at 12:11 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
>> Thanks.  It looks like to get the semantics I expect from "local"
>> in zsh, one needs to use "typeset -h" (which bash does not support,
>> unfortunately).  Probably it is best to steer clear of zsh's special
>> variables anyway.
>
> Hmm, interesting, maybe we should try to find a way to replace those
> 'local' with 'typeset -h'.
This seems to do it:
Show changes to contrib/completion/git-completion.bash +4 −0
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -75,6 +75,10 @@

 if [[ -n ${ZSH_VERSION-} ]]; then
        autoload -U +X bashcompinit && bashcompinit
+
+       # 'words' has special meaning in zsh, and only typeset -h seems to
+       # override that
+       alias local="typeset -h"
 fi

 case "$COMP_WORDBREAKS" in
-- 
Felipe Contreras
Jonathan Nieder· Apr 27, 2011, 21:27 UTC · re: Felipe Contreras · lore

[PATCH] completion: move private shopt shim for zsh to __git_ namespace

Most zsh users probably probably do not expect a custom shopt function to enter their environment just because they ran "source ~/.git-completion.sh".

Such namespace pollution makes development of other scripts confusing (because it makes the bash-specific shopt utility seem to be available in zsh) and makes git's tab completion script brittle (since any other shell snippet implementing some other subset of shopt will break it). Rename the shopt shim to the more innocuous __git_shopt to be a good citizen (with two underscores to avoid confusion with completion rules for a hypothetical "git shopt" command).

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Hi,
Felipe Contreras wrote:
Show 12 quoted lines
> +++ b/contrib/completion/git-completion.bash
> @@ -75,6 +75,10 @@
> 
>  if [[ -n ${ZSH_VERSION-} ]]; then
>         autoload -U +X bashcompinit && bashcompinit
> +
> +       # 'words' has special meaning in zsh, and only typeset -h seems to
> +       # override that
> +       alias local="typeset -h"
>  fi
> 
>  case "$COMP_WORDBREAKS" in

The above would change the meaning of "local" in the user's environment and in all shell snippets she sources later. Are you sure that's intended?

Actually the completion script already has a problem along the same lines. Thanks for a reminder.

 contrib/completion/git-completion.bash |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)
Show changes to contrib/completion/git-completion.bash +8 −4
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9150ea6..ab95690 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -629,12 +629,12 @@ __git_refs_remotes ()
 __git_remotes ()
 {
 	local i ngoff IFS=$'\n' d="$(__gitdir)"
-	shopt -q nullglob || ngoff=1
-	shopt -s nullglob
+	__git_shopt -q nullglob || ngoff=1
+	__git_shopt -s nullglob
 	for i in "$d/remotes"/*; do
 		echo ${i#$d/remotes/}
 	done
-	[ "$ngoff" ] && shopt -u nullglob
+	[ "$ngoff" ] && __git_shopt -u nullglob
 	for i in $(git --git-dir="$d" config --get-regexp 'remote\..*\.url' 2>/dev/null); do
 		i="${i#remote.}"
 		echo "${i/.url*/}"
@@ -2800,7 +2800,7 @@ complete -o bashdefault -o default -o nospace -F _git git.exe 2>/dev/null \
 fi
 
 if [[ -n ${ZSH_VERSION-} ]]; then
-	shopt () {
+	__git_shopt () {
 		local option
 		if [ $# -ne 2 ]; then
 			echo "USAGE: $0 (-q|-s|-u) <option>" >&2
@@ -2823,4 +2823,8 @@ if [[ -n ${ZSH_VERSION-} ]]; then
 			return 1
 		esac
 	}
+else
+	__git_shopt () {
+		shopt "$@"
+	}
 fi
-- 
1.7.5
Felipe Contreras· Apr 27, 2011, 22:48 UTC · re: Jonathan Nieder · lore

Re: [PATCH] completion: move private shopt shim for zsh to __git_ namespace

On Thu, Apr 28, 2011 at 12:27 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 17 quoted lines
> Felipe Contreras wrote:
>> +++ b/contrib/completion/git-completion.bash
>> @@ -75,6 +75,10 @@
>>
>>  if [[ -n ${ZSH_VERSION-} ]]; then
>>         autoload -U +X bashcompinit && bashcompinit
>> +
>> +       # 'words' has special meaning in zsh, and only typeset -h seems to
>> +       # override that
>> +       alias local="typeset -h"
>>  fi
>>
>>  case "$COMP_WORDBREAKS" in
>
> The above would change the meaning of "local" in the user's
> environment and in all shell snippets she sources later.  Are you sure
> that's intended?
Crap. I didn't realize that :(

I have been looking for a way to have local aliases or functions, but I can't find any.

-- 
Felipe Contreras
Jonathan Nieder· Apr 27, 2011, 23:00 UTC · re: Felipe Contreras · lore

Re: [PATCH] completion: move private shopt shim for zsh to __git_ namespace

Felipe Contreras wrote:
> I have been looking for a way to have local aliases or functions, but
> I can't find any.
It is possible to do
	if [[ -n ${ZSH_VERSION-} ]]; then
		alias __git_local="typeset -h"
	else
		alias __git_local=local
	fi
but let's consider that for a moment.
 1. It's ugly (it means completion code would use a dialect where the
    ordinary "local" keyword has to be spelled differently).
 2. It's ugly (use of aliases in scripts sets off alarm bells.  As
    Almquist's sh manual says:
	Aliases provide a convenient way for naïve users to create
	shorthands for commands without having to learn how to create
	functions with arguments.  They can also be used to create
	lexically obscure code.  This use is discouraged.)
 3. It's fragile (maybe some day a function from zsh's completion
    library that we call will look at $words and get utterly confused).

I don't mean to sound so negative; actually I am very happy to see us getting closer to a full understanding of the problem and relevant constraints.

Jonathan Nieder· May 6, 2011, 05:46 UTC · re: Jonathan Nieder · lore

Re: [PATCH] completion: move private shopt shim for zsh to __git_ namespace

(culling cc list of quiet people :)) Jonathan Nieder wrote:

Show 11 quoted lines
> Most zsh users probably probably do not expect a custom shopt function
> to enter their environment just because they ran "source
> ~/.git-completion.sh".
>
> Such namespace pollution makes development of other scripts confusing
> (because it makes the bash-specific shopt utility seem to be available
> in zsh) and makes git's tab completion script brittle (since any other
> shell snippet implementing some other subset of shopt will break it).
> Rename the shopt shim to the more innocuous __git_shopt to be a good
> citizen (with two underscores to avoid confusion with completion rules
> for a hypothetical "git shopt" command).

By the way, I meant the above[1] as a genuine patch submission. Thoughts? Bugs? Improvements?

[1] http://thread.gmane.org/gmane.comp.version-control.git/172142/focus=172275
Felipe Contreras· May 6, 2011, 08:35 UTC · re: Jonathan Nieder · lore

Re: [PATCH] completion: move private shopt shim for zsh to __git_ namespace

On Fri, May 6, 2011 at 8:46 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
Show 17 quoted lines
> (culling cc list of quiet people :))
> Jonathan Nieder wrote:
>
>> Most zsh users probably probably do not expect a custom shopt function
>> to enter their environment just because they ran "source
>> ~/.git-completion.sh".
>>
>> Such namespace pollution makes development of other scripts confusing
>> (because it makes the bash-specific shopt utility seem to be available
>> in zsh) and makes git's tab completion script brittle (since any other
>> shell snippet implementing some other subset of shopt will break it).
>> Rename the shopt shim to the more innocuous __git_shopt to be a good
>> citizen (with two underscores to avoid confusion with completion rules
>> for a hypothetical "git shopt" command).
>
> By the way, I meant the above[1] as a genuine patch submission.
> Thoughts?  Bugs?  Improvements?
Looks fine to me.
-- 
Felipe Contreras
SZEDER Gábor· May 8, 2011, 10:48 UTC · re: Jonathan Nieder · lore

Re: [PATCH] completion: move private shopt shim for zsh to __git_ namespace

On Fri, May 06, 2011 at 12:46:04AM -0500, Jonathan Nieder wrote:
Show 19 quoted lines
> (culling cc list of quiet people :))
> Jonathan Nieder wrote:
> 
> > Most zsh users probably probably do not expect a custom shopt function
> > to enter their environment just because they ran "source
> > ~/.git-completion.sh".
> >
> > Such namespace pollution makes development of other scripts confusing
> > (because it makes the bash-specific shopt utility seem to be available
> > in zsh) and makes git's tab completion script brittle (since any other
> > shell snippet implementing some other subset of shopt will break it).
> > Rename the shopt shim to the more innocuous __git_shopt to be a good
> > citizen (with two underscores to avoid confusion with completion rules
> > for a hypothetical "git shopt" command).
> 
> By the way, I meant the above[1] as a genuine patch submission.
> Thoughts?  Bugs?  Improvements?
> 
> [1] http://thread.gmane.org/gmane.comp.version-control.git/172142/focus=172275
It surely won't be fun to debug such breakages, so:
Acked-by: SZEDER Gábor <szeder@ira.uka.de>
SZEDER Gábor· Apr 28, 2011, 16:01 UTC · re: Jonathan Nieder · lore

Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability

Hi,
On Wed, Apr 27, 2011 at 01:40:34AM -0500, Jonathan Nieder wrote:
Show 22 quoted lines
> The "_get_comp_words_by_ref -n := words" command from the
> bash_completion library reassembles a modified version of COMP_WORDS
> with ':' and '=' no longer treated as word separators and stores it in
> the ${words[@]} array.  Git's programmable tab completion script uses
> this to abstract away the difference between bash v3's and bash v4's
> definitions of COMP_WORDS (bash v3 used shell words, while bash v4
> breaks at separator characters); see v1.7.4-rc0~11^2~2 (bash: get
> --pretty=m<tab> completion to work with bash v4, 2010-12-02).
> 
> zsh has (or rather its completion functions have) another idea about
> what ${words[@]} should contain: the array is prepopulated with the
> words from the command it is completing.  For reasons that are not
> well understood, when git-completion.bash reserves its own "words"
> variable with "local words", the variable becomes empty and cannot be
> changed from then on.  So the completion script neglects the arguments
> it has seen, and words complete like git subcommand names.  For
> example, typing "git log origi<TAB>" gives no completions because
> there are no "git origi..." commands.
> 
> Work around this by using a different variable (comp_words) that is
> not special to zsh.  So now commands that completed correctly before
> v1.7.4-rc0~11^2~2 on zsh should be able to complete correctly again.

Thanks for this explanation. I tried to fix this some time ago, but got only as far as to indentify that something is amiss with returning $words from _get_comp_words_by_ref(), and during tracing I saw so much weird and (for me) unexplicable zsh behavior that I simply gave up.

But this patch heavily conflicted with one of my long-forgotten cleanup patch series, and that series together with the above explanation gave me alternative ideas for fixing this issue with zsh.

So, here it is. The first two patches are independent cleanups, but they make the actual fix in the third patch so short.

It works well as far as I tested it with both bash and zsh, but I would really appreciate a few extra sets of eyeballs for the cleanups and sanity-checking and testing of the bugfix.

SZEDER Gábor (3):
  bash: don't modify the $cur variable in completion functions
  bash: remove unnecessary _get_comp_words_by_ref() invocations
  bash: don't declare 'local words' to make zsh happy
 contrib/completion/git-completion.bash |  225 +++++++++-----------------------
 1 files changed, 64 insertions(+), 161 deletions(-)
SZEDER Gábor· Apr 28, 2011, 16:01 UTC · re: SZEDER Gábor · lore

[PATCH 1/3] bash: don't modify the $cur variable in completion functions

Since v1.7.4-rc0~11^2~2 (bash: get --pretty=m<tab> completion to work with bash v4, 2010-12-02) we use _get_comp_words_by_ref() to access completion-related variables, and the $cur variable holds the word containing the current cursor position in all completion functions. This $cur variable is left unchanged in most completion functions; there are only four functions modifying its value, namely __gitcomp(), __git_complete_revlist_file(), __git_complete_remote_or_refspec(), and _git_config().

If this variable were never modified, then it would allow us a nice optimisation and cleanup. Therefore, this patch assigns $cur to an other local variable and uses that for later modifications in those four functions.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |  107 +++++++++++++++-----------------
 1 files changed, 50 insertions(+), 57 deletions(-)
Show changes to contrib/completion/git-completion.bash +50 −57
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 840ae38..a594b40 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -491,10 +491,12 @@ __gitcomp ()
 {
 	local cur
 	_get_comp_words_by_ref -n =: cur
+	local cur_="$cur"
+
 	if [ $# -gt 2 ]; then
-		cur="$3"
+		cur_="$3"
 	fi
-	case "$cur" in
+	case "$cur_" in
 	--*=)
 		COMPREPLY=()
 		;;
@@ -502,7 +504,7 @@ __gitcomp ()
 		local IFS=$'\n'
 		COMPREPLY=($(compgen -P "${2-}" \
 			-W "$(__gitcomp_1 "${1-}" "${4-}")" \
-			-- "$cur"))
+			-- "$cur_"))
 		;;
 	esac
 }
@@ -668,17 +670,18 @@ __git_complete_revlist_file ()
 {
 	local pfx ls ref cur
 	_get_comp_words_by_ref -n =: cur
-	case "$cur" in
+	local cur_="$cur"
+	case "$cur_" in
 	*..?*:*)
 		return
 		;;
 	?*:*)
-		ref="${cur%%:*}"
-		cur="${cur#*:}"
-		case "$cur" in
+		ref="${cur_%%:*}"
+		cur_="${cur_#*:}"
+		case "$cur_" in
 		?*/*)
-			pfx="${cur%/*}"
-			cur="${cur##*/}"
+			pfx="${cur_%/*}"
+			cur_="${cur_##*/}"
 			ls="$ref:$pfx"
 			pfx="$pfx/"
 			;;
@@ -708,17 +711,17 @@ __git_complete_revlist_file ()
 				           s,$,/,
 				       }
 				       s/^.*	//')" \
-			-- "$cur"))
+			-- "$cur_"))
 		;;
 	*...*)
-		pfx="${cur%...*}..."
-		cur="${cur#*...}"
-		__gitcomp "$(__git_refs)" "$pfx" "$cur"
+		pfx="${cur_%...*}..."
+		cur_="${cur_#*...}"
+		__gitcomp "$(__git_refs)" "$pfx" "$cur_"
 		;;
 	*..*)
-		pfx="${cur%..*}.."
-		cur="${cur#*..}"
-		__gitcomp "$(__git_refs)" "$pfx" "$cur"
+		pfx="${cur_%..*}.."
+		cur_="${cur_#*..}"
+		__gitcomp "$(__git_refs)" "$pfx" "$cur_"
 		;;
 	*)
 		__gitcomp "$(__git_refs)"
@@ -741,7 +744,7 @@ __git_complete_remote_or_refspec ()
 {
 	local cur words cword
 	_get_comp_words_by_ref -n =: cur words cword
-	local cmd="${words[1]}"
+	local cur_="$cur" cmd="${words[1]}"
 	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"
@@ -771,40 +774,40 @@ __git_complete_remote_or_refspec ()
 		return
 	fi
 	[ "$remote" = "." ] && remote=
-	case "$cur" in
+	case "$cur_" in
 	*:*)
 		case "$COMP_WORDBREAKS" in
 		*:*) : great ;;
-		*)   pfx="${cur%%:*}:" ;;
+		*)   pfx="${cur_%%:*}:" ;;
 		esac
-		cur="${cur#*:}"
+		cur_="${cur_#*:}"
 		lhs=0
 		;;
 	+*)
 		pfx="+"
-		cur="${cur#+}"
+		cur_="${cur_#+}"
 		;;
 	esac
 	case "$cmd" in
 	fetch)
 		if [ $lhs = 1 ]; then
-			__gitcomp "$(__git_refs2 "$remote")" "$pfx" "$cur"
+			__gitcomp "$(__git_refs2 "$remote")" "$pfx" "$cur_"
 		else
-			__gitcomp "$(__git_refs)" "$pfx" "$cur"
+			__gitcomp "$(__git_refs)" "$pfx" "$cur_"
 		fi
 		;;
 	pull)
 		if [ $lhs = 1 ]; then
-			__gitcomp "$(__git_refs "$remote")" "$pfx" "$cur"
+			__gitcomp "$(__git_refs "$remote")" "$pfx" "$cur_"
 		else
-			__gitcomp "$(__git_refs)" "$pfx" "$cur"
+			__gitcomp "$(__git_refs)" "$pfx" "$cur_"
 		fi
 		;;
 	push)
 		if [ $lhs = 1 ]; then
-			__gitcomp "$(__git_refs)" "$pfx" "$cur"
+			__gitcomp "$(__git_refs)" "$pfx" "$cur_"
 		else
-			__gitcomp "$(__git_refs "$remote")" "$pfx" "$cur"
+			__gitcomp "$(__git_refs "$remote")" "$pfx" "$cur_"
 		fi
 		;;
 	esac
@@ -2012,70 +2015,60 @@ _git_config ()
 		return
 		;;
 	branch.*.*)
-		local pfx="${cur%.*}."
-		cur="${cur##*.}"
-		__gitcomp "remote merge mergeoptions rebase" "$pfx" "$cur"
+		local pfx="${cur%.*}." cur_="${cur##*.}"
+		__gitcomp "remote merge mergeoptions rebase" "$pfx" "$cur_"
 		return
 		;;
 	branch.*)
-		local pfx="${cur%.*}."
-		cur="${cur#*.}"
-		__gitcomp "$(__git_heads)" "$pfx" "$cur" "."
+		local pfx="${cur%.*}." cur_="${cur#*.}"
+		__gitcomp "$(__git_heads)" "$pfx" "$cur_" "."
 		return
 		;;
 	guitool.*.*)
-		local pfx="${cur%.*}."
-		cur="${cur##*.}"
+		local pfx="${cur%.*}." cur_="${cur##*.}"
 		__gitcomp "
 			argprompt cmd confirm needsfile noconsole norescan
 			prompt revprompt revunmerged title
-			" "$pfx" "$cur"
+			" "$pfx" "$cur_"
 		return
 		;;
 	difftool.*.*)
-		local pfx="${cur%.*}."
-		cur="${cur##*.}"
-		__gitcomp "cmd path" "$pfx" "$cur"
+		local pfx="${cur%.*}." cur_="${cur##*.}"
+		__gitcomp "cmd path" "$pfx" "$cur_"
 		return
 		;;
 	man.*.*)
-		local pfx="${cur%.*}."
-		cur="${cur##*.}"
-		__gitcomp "cmd path" "$pfx" "$cur"
+		local pfx="${cur%.*}." cur_="${cur##*.}"
+		__gitcomp "cmd path" "$pfx" "$cur_"
 		return
 		;;
 	mergetool.*.*)
-		local pfx="${cur%.*}."
-		cur="${cur##*.}"
-		__gitcomp "cmd path trustExitCode" "$pfx" "$cur"
+		local pfx="${cur%.*}." cur_="${cur##*.}"
+		__gitcomp "cmd path trustExitCode" "$pfx" "$cur_"
 		return
 		;;
 	pager.*)
-		local pfx="${cur%.*}."
-		cur="${cur#*.}"
+		local pfx="${cur%.*}." cur_="${cur#*.}"
 		__git_compute_all_commands
-		__gitcomp "$__git_all_commands" "$pfx" "$cur"
+		__gitcomp "$__git_all_commands" "$pfx" "$cur_"
 		return
 		;;
 	remote.*.*)
-		local pfx="${cur%.*}."
-		cur="${cur##*.}"
+		local pfx="${cur%.*}." cur_="${cur##*.}"
 		__gitcomp "
 			url proxy fetch push mirror skipDefaultUpdate
 			receivepack uploadpack tagopt pushurl
-			" "$pfx" "$cur"
+			" "$pfx" "$cur_"
 		return
 		;;
 	remote.*)
-		local pfx="${cur%.*}."
-		cur="${cur#*.}"
-		__gitcomp "$(__git_remotes)" "$pfx" "$cur" "."
+		local pfx="${cur%.*}." cur_="${cur#*.}"
+		__gitcomp "$(__git_remotes)" "$pfx" "$cur_" "."
 		return
 		;;
 	url.*.*)
-		local pfx="${cur%.*}."
-		cur="${cur##*.}"
-		__gitcomp "insteadOf pushInsteadOf" "$pfx" "$cur"
+		local pfx="${cur%.*}." cur_="${cur##*.}"
+		__gitcomp "insteadOf pushInsteadOf" "$pfx" "$cur_"
 		return
 		;;
 	esac
-- 
1.7.5.86.g799a6
SZEDER Gábor· Apr 28, 2011, 16:01 UTC · re: SZEDER Gábor · lore

[PATCH 2/3] bash: remove unnecessary _get_comp_words_by_ref() invocations

In v1.7.4-rc0~11^2~2 (bash: get --pretty=m<tab> completion to work with bash v4, 2010-12-02) we started to use _get_comp_words_by_ref() to access completion-related variables. That was large change, and to make it easily reviewable, we invoked _get_comp_words_by_ref() in each completion function and systematically replaced every occurance of bash's completion-related variables ($COMP_WORDS and $COMP_CWORD) with variables set by _get_comp_words_by_ref().

This has the downside that _get_comp_words_by_ref() is invoked several times during a single completion. The worst offender is perhaps 'git log mas<TAB>': during the completion of 'master' _get_comp_words_by_ref() is invoked no less than six times.

However, the variables $prev, $cword, and $words provided by _get_comp_words_by_ref() are not modified in any of the completion functions, and the previous commit ensures that the $cur variable is not modified as well. This makes it possible to invoke _get_comp_words_by_ref() to get those variables only once in our toplevel completion functions _git() and _gitk(), and all other completion functions will inherit them.

Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |  116 +++-----------------------------
 1 files changed, 11 insertions(+), 105 deletions(-)
Show changes to contrib/completion/git-completion.bash +11 −105
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a594b40..862b840 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -489,8 +489,6 @@ fi
 # generates completion reply with compgen
 __gitcomp ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	local cur_="$cur"
 
 	if [ $# -gt 2 ]; then
@@ -553,8 +551,7 @@ __git_tags ()
 __git_refs ()
 {
 	local i is_hash=y dir="$(__gitdir "${1-}")" track="${2-}"
-	local cur format refs
-	_get_comp_words_by_ref -n =: cur
+	local format refs
 	if [ -d "$dir" ]; then
 		case "$cur" in
 		refs|refs/*)
@@ -668,9 +665,7 @@ __git_compute_merge_strategies ()
 
 __git_complete_revlist_file ()
 {
-	local pfx ls ref cur
-	_get_comp_words_by_ref -n =: cur
-	local cur_="$cur"
+	local pfx ls ref cur_="$cur"
 	case "$cur_" in
 	*..?*:*)
 		return
@@ -742,8 +737,6 @@ __git_complete_revlist ()
 
 __git_complete_remote_or_refspec ()
 {
-	local cur words cword
-	_get_comp_words_by_ref -n =: cur words cword
 	local cur_="$cur" cmd="${words[1]}"
 	local i c=2 remote="" pfx="" lhs=1 no_complete_refspec=0
 	while [ $c -lt $cword ]; do
@@ -815,8 +808,6 @@ __git_complete_remote_or_refspec ()
 
 __git_complete_strategy ()
 {
-	local cur prev
-	_get_comp_words_by_ref -n =: cur prev
 	__git_compute_merge_strategies
 	case "$prev" in
 	-s|--strategy)
@@ -994,8 +985,7 @@ __git_aliased_command ()
 # __git_find_on_cmdline requires 1 argument
 __git_find_on_cmdline ()
 {
-	local word subcommand c=1 words cword
-	_get_comp_words_by_ref -n =: words cword
+	local word subcommand c=1
 	while [ $c -lt $cword ]; do
 		word="${words[c]}"
 		for subcommand in $1; do
@@ -1010,8 +1000,7 @@ __git_find_on_cmdline ()
 
 __git_has_doubledash ()
 {
-	local c=1 words cword
-	_get_comp_words_by_ref -n =: words cword
+	local c=1
 	while [ $c -lt $cword ]; do
 		if [ "--" = "${words[c]}" ]; then
 			return 0
@@ -1025,8 +1014,7 @@ __git_whitespacelist="nowarn warn error error-all fix"
 
 _git_am ()
 {
-	local cur dir="$(__gitdir)"
-	_get_comp_words_by_ref -n =: cur
+	local dir="$(__gitdir)"
 	if [ -d "$dir"/rebase-apply ]; then
 		__gitcomp "--skip --continue --resolved --abort"
 		return
@@ -1050,8 +1038,6 @@ _git_am ()
 
 _git_apply ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--whitespace=*)
 		__gitcomp "$__git_whitespacelist" "" "${cur##--whitespace=}"
@@ -1074,8 +1060,6 @@ _git_add ()
 {
 	__git_has_doubledash && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -1089,8 +1073,6 @@ _git_add ()
 
 _git_archive ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--format=*)
 		__gitcomp "$(git archive --list)" "" "${cur##--format=}"
@@ -1138,9 +1120,8 @@ _git_bisect ()
 
 _git_branch ()
 {
-	local i c=1 only_local_ref="n" has_r="n" cur words cword
+	local i c=1 only_local_ref="n" has_r="n"
 
-	_get_comp_words_by_ref -n =: cur words cword
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"
 		case "$i" in
@@ -1170,8 +1151,6 @@ _git_branch ()
 
 _git_bundle ()
 {
-	local words cword
-	_get_comp_words_by_ref -n =: words cword
 	local cmd="${words[2]}"
 	case "$cword" in
 	2)
@@ -1194,8 +1173,6 @@ _git_checkout ()
 {
 	__git_has_doubledash && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--conflict=*)
 		__gitcomp "diff3 merge" "" "${cur##--conflict=}"
@@ -1225,8 +1202,6 @@ _git_cherry ()
 
 _git_cherry_pick ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "--edit --no-commit"
@@ -1241,8 +1216,6 @@ _git_clean ()
 {
 	__git_has_doubledash && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "--dry-run --quiet"
@@ -1254,8 +1227,6 @@ _git_clean ()
 
 _git_clone ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -1282,8 +1253,6 @@ _git_commit ()
 {
 	__git_has_doubledash && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--cleanup=*)
 		__gitcomp "default strip verbatim whitespace
@@ -1318,8 +1287,6 @@ _git_commit ()
 
 _git_describe ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -1351,8 +1318,6 @@ _git_diff ()
 {
 	__git_has_doubledash && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "--cached --staged --pickaxe-all --pickaxe-regex
@@ -1373,8 +1338,6 @@ _git_difftool ()
 {
 	__git_has_doubledash && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--tool=*)
 		__gitcomp "$__git_mergetools_common kompare" "" "${cur##--tool=}"
@@ -1399,8 +1362,6 @@ __git_fetch_options="
 
 _git_fetch ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "$__git_fetch_options"
@@ -1412,8 +1373,6 @@ _git_fetch ()
 
 _git_format_patch ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--thread=*)
 		__gitcomp "
@@ -1445,8 +1404,6 @@ _git_format_patch ()
 
 _git_fsck ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -1461,8 +1418,6 @@ _git_fsck ()
 
 _git_gc ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "--prune --aggressive"
@@ -1481,8 +1436,6 @@ _git_grep ()
 {
 	__git_has_doubledash && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -1505,8 +1458,6 @@ _git_grep ()
 
 _git_help ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "--all --info --man --web"
@@ -1524,8 +1475,6 @@ _git_help ()
 
 _git_init ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--shared=*)
 		__gitcomp "
@@ -1545,8 +1494,6 @@ _git_ls_files ()
 {
 	__git_has_doubledash && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "--cached --deleted --modified --others --ignored
@@ -1607,8 +1554,6 @@ _git_log ()
 	if [ -f "$g/MERGE_HEAD" ]; then
 		merge="--merge"
 	fi
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--pretty=*)
 		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
@@ -1662,8 +1607,6 @@ _git_merge ()
 {
 	__git_complete_strategy && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "$__git_merge_options"
@@ -1674,8 +1617,6 @@ _git_merge ()
 
 _git_mergetool ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--tool=*)
 		__gitcomp "$__git_mergetools_common tortoisemerge" "" "${cur##--tool=}"
@@ -1696,8 +1637,6 @@ _git_merge_base ()
 
 _git_mv ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "--dry-run"
@@ -1716,8 +1655,6 @@ _git_notes ()
 {
 	local subcommands='add append copy edit list prune remove show'
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
-	local cur words cword
-	_get_comp_words_by_ref -n =: cur words cword
 
 	case "$subcommand,$cur" in
 	,--*)
@@ -1767,8 +1704,6 @@ _git_pull ()
 {
 	__git_complete_strategy && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -1784,8 +1719,6 @@ _git_pull ()
 
 _git_push ()
 {
-	local cur prev
-	_get_comp_words_by_ref -n =: cur prev
 	case "$prev" in
 	--repo)
 		__gitcomp "$(__git_remotes)"
@@ -1810,8 +1743,6 @@ _git_push ()
 _git_rebase ()
 {
 	local dir="$(__gitdir)"
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	if [ -d "$dir"/rebase-apply ] || [ -d "$dir"/rebase-merge ]; then
 		__gitcomp "--continue --skip --abort"
 		return
@@ -1853,8 +1784,6 @@ __git_send_email_suppresscc_options="author self cc bodycc sob cccmd body all"
 
 _git_send_email ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--confirm=*)
 		__gitcomp "
@@ -1896,8 +1825,6 @@ _git_stage ()
 
 __git_config_get_set_variables ()
 {
-	local words cword
-	_get_comp_words_by_ref -n =: words cword
 	local prevword word config_file= c=$cword
 	while [ $c -gt 1 ]; do
 		word="${words[c]}"
@@ -1928,8 +1855,6 @@ __git_config_get_set_variables ()
 
 _git_config ()
 {
-	local cur prev
-	_get_comp_words_by_ref -n =: cur prev
 	case "$prev" in
 	branch.*.remote)
 		__gitcomp "$(__git_remotes)"
@@ -2389,8 +2314,6 @@ _git_reset ()
 {
 	__git_has_doubledash && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "--merge --mixed --hard --soft --patch"
@@ -2402,8 +2325,6 @@ _git_reset ()
 
 _git_revert ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "--edit --mainline --no-edit --no-commit --signoff"
@@ -2417,8 +2338,6 @@ _git_rm ()
 {
 	__git_has_doubledash && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "--cached --dry-run --ignore-unmatch --quiet"
@@ -2432,8 +2351,6 @@ _git_shortlog ()
 {
 	__git_has_doubledash && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -2451,8 +2368,6 @@ _git_show ()
 {
 	__git_has_doubledash && return
 
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--pretty=*)
 		__gitcomp "$__git_log_pretty_formats $(__git_pretty_aliases)
@@ -2476,8 +2391,6 @@ _git_show ()
 
 _git_show_branch ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -2494,8 +2407,6 @@ _git_show_branch ()
 
 _git_stash ()
 {
-	local cur
-	_get_comp_words_by_ref -n =: cur
 	local save_opts='--keep-index --no-keep-index --quiet --patch'
 	local subcommands='save list show apply clear drop pop create branch'
 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
@@ -2540,8 +2451,6 @@ _git_submodule ()
 
 	local subcommands="add status init update summary foreach sync"
 	if [ -z "$(__git_find_on_cmdline "$subcommands")" ]; then
-		local cur
-		_get_comp_words_by_ref -n =: cur
 		case "$cur" in
 		--*)
 			__gitcomp "--quiet --cached"
@@ -2585,8 +2494,6 @@ _git_svn ()
 			--edit --rmdir --find-copies-harder --copy-similarity=
 			"
 
-		local cur
-		_get_comp_words_by_ref -n =: cur
 		case "$subcommand,$cur" in
 		fetch,--*)
 			__gitcomp "--revision= --fetch-all $fc_opts"
@@ -2658,8 +2565,6 @@ _git_svn ()
 _git_tag ()
 {
 	local i c=1 f=0
-	local words cword prev
-	_get_comp_words_by_ref -n =: words cword prev
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"
 		case "$i" in
@@ -2705,8 +2610,8 @@ _git ()
 		setopt KSH_TYPESET
 	fi
 
-	local cur words cword
-	_get_comp_words_by_ref -n =: cur words cword
+	local cur words cword prev
+	_get_comp_words_by_ref -n =: cur words cword prev
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"
 		case "$i" in
@@ -2756,15 +2661,16 @@ _gitk ()
 		setopt KSH_TYPESET
 	fi
 
+	local cur words cword prev
+	_get_comp_words_by_ref -n =: cur words cword prev
+
 	__git_has_doubledash && return
 
-	local cur
 	local g="$(__gitdir)"
 	local merge=""
 	if [ -f "$g/MERGE_HEAD" ]; then
 		merge="--merge"
 	fi
-	_get_comp_words_by_ref -n =: cur
 	case "$cur" in
 	--*)
 		__gitcomp "
-- 
1.7.5.86.g799a6
SZEDER Gábor· Apr 28, 2011, 16:01 UTC · re: SZEDER Gábor · lore

[PATCH 3/3] bash: don't declare 'local words' to make zsh happy

The "_get_comp_words_by_ref -n := words" command from the bash_completion library reassembles a modified version of COMP_WORDS with ':' and '=' no longer treated as word separators and stores it in the ${words[@]} array. Git's programmable tab completion script uses this to abstract away the difference between bash v3's and bash v4's definitions of COMP_WORDS (bash v3 used shell words, while bash v4 breaks at separator characters); see v1.7.4-rc0~11^2~2 (bash: get --pretty=m<tab> completion to work with bash v4, 2010-12-02).

zsh has (or rather its completion functions have) another idea about what ${words[@]} should contain: the array is prepopulated with the words from the command it is completing. For reasons that are not well understood, when git-completion.bash reserves its own "words" variable with "local words", the variable becomes empty and cannot be changed from then on. So the completion script neglects the arguments it has seen, and words complete like git subcommand names. For example, typing "git log origi<TAB>" gives no completions because there are no "git origi..." commands.

However, when this words variable is not declared as local but is just populated by _get_comp_words_by_ref() and then read in various completion functions, then zsh seems to be happy about it and our completion script works as expected.

So, to get our completion script working again under zsh and to prevent the words variable from leaking into the shell environment under bash, we will only declare words as local when using bash.

Reported-by: Stefan Haller <lists@haller-berlin.de>
Suggested-by: Felipe Contreras <felipe.contreras@gmail.com>
Explained-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
 contrib/completion/git-completion.bash |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)
Show changes to contrib/completion/git-completion.bash +6 −2
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 862b840..6869765 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2608,9 +2608,11 @@ _git ()
 	if [[ -n ${ZSH_VERSION-} ]]; then
 		emulate -L bash
 		setopt KSH_TYPESET
+	else
+		local words
 	fi
 
-	local cur words cword prev
+	local cur cword prev
 	_get_comp_words_by_ref -n =: cur words cword prev
 	while [ $c -lt $cword ]; do
 		i="${words[c]}"
@@ -2659,9 +2661,11 @@ _gitk ()
 	if [[ -n ${ZSH_VERSION-} ]]; then
 		emulate -L bash
 		setopt KSH_TYPESET
+	else
+		local words
 	fi
 
-	local cur words cword prev
+	local cur cword prev
 	_get_comp_words_by_ref -n =: cur words cword prev
 
 	__git_has_doubledash && return
-- 
1.7.5.86.g799a6
Felipe Contreras· May 3, 2011, 17:53 UTC · re: SZEDER Gábor · lore

Re: [PATCH 3/3] bash: don't declare 'local words' to make zsh happy

On Thu, Apr 28, 2011 at 7:01 PM, SZEDER Gábor <szeder@ira.uka.de> wrote:
Show 68 quoted lines
> The "_get_comp_words_by_ref -n := words" command from the
> bash_completion library reassembles a modified version of COMP_WORDS
> with ':' and '=' no longer treated as word separators and stores it in
> the ${words[@]} array.  Git's programmable tab completion script uses
> this to abstract away the difference between bash v3's and bash v4's
> definitions of COMP_WORDS (bash v3 used shell words, while bash v4
> breaks at separator characters); see v1.7.4-rc0~11^2~2 (bash: get
> --pretty=m<tab> completion to work with bash v4, 2010-12-02).
>
> zsh has (or rather its completion functions have) another idea about
> what ${words[@]} should contain: the array is prepopulated with the
> words from the command it is completing.  For reasons that are not
> well understood, when git-completion.bash reserves its own "words"
> variable with "local words", the variable becomes empty and cannot be
> changed from then on.  So the completion script neglects the arguments
> it has seen, and words complete like git subcommand names.  For
> example, typing "git log origi<TAB>" gives no completions because
> there are no "git origi..." commands.
>
> However, when this words variable is not declared as local but is just
> populated by _get_comp_words_by_ref() and then read in various
> completion functions, then zsh seems to be happy about it and our
> completion script works as expected.
>
> So, to get our completion script working again under zsh and to
> prevent the words variable from leaking into the shell environment
> under bash, we will only declare words as local when using bash.
>
> Reported-by: Stefan Haller <lists@haller-berlin.de>
> Suggested-by: Felipe Contreras <felipe.contreras@gmail.com>
> Explained-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> ---
>  contrib/completion/git-completion.bash |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 862b840..6869765 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -2608,9 +2608,11 @@ _git ()
>        if [[ -n ${ZSH_VERSION-} ]]; then
>                emulate -L bash
>                setopt KSH_TYPESET
> +       else
> +               local words
>        fi
>
> -       local cur words cword prev
> +       local cur cword prev
>        _get_comp_words_by_ref -n =: cur words cword prev
>        while [ $c -lt $cword ]; do
>                i="${words[c]}"
> @@ -2659,9 +2661,11 @@ _gitk ()
>        if [[ -n ${ZSH_VERSION-} ]]; then
>                emulate -L bash
>                setopt KSH_TYPESET
> +       else
> +               local words
>        fi
>
> -       local cur words cword prev
> +       local cur cword prev
>        _get_comp_words_by_ref -n =: cur words cword prev
>
>        __git_has_doubledash && return
> --
> 1.7.5.86.g799a6
Here's another option:
From 603e4db259283a4eb6bac2315a630480e3238f50 Mon Sep 17 00:00:00 2001
From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Tue, 3 May 2011 20:45:26 +0300
Subject: [PATCH] git-completion: fix zsh support
It turns out 'words' is a special variable used by zsh completion.

There's probably a bug in zsh's bashcompinit: http://article.gmane.org/gmane.comp.shells.zsh.devel/22546

But in the meantime we can workaround it this way.
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Show changes to contrib/completion/git-completion.bash +3 −0
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 00691fc..d32b1b8 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2608,6 +2608,9 @@ _git ()
 	if [[ -n ${ZSH_VERSION-} ]]; then
 		emulate -L bash
 		setopt KSH_TYPESET
+
+		# 'words' has special meaning in zsh; override that
+		typeset -h words
 	fi

 	local cur words cword prev
-- 
1.7.5

-- 
Felipe Contreras
Felipe Contreras· Apr 28, 2011, 20:24 UTC · re: SZEDER Gábor · lore

Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability

2011/4/28 SZEDER Gábor <szeder@ira.uka.de>:
Show 48 quoted lines
> On Wed, Apr 27, 2011 at 01:40:34AM -0500, Jonathan Nieder wrote:
>> The "_get_comp_words_by_ref -n := words" command from the
>> bash_completion library reassembles a modified version of COMP_WORDS
>> with ':' and '=' no longer treated as word separators and stores it in
>> the ${words[@]} array.  Git's programmable tab completion script uses
>> this to abstract away the difference between bash v3's and bash v4's
>> definitions of COMP_WORDS (bash v3 used shell words, while bash v4
>> breaks at separator characters); see v1.7.4-rc0~11^2~2 (bash: get
>> --pretty=m<tab> completion to work with bash v4, 2010-12-02).
>>
>> zsh has (or rather its completion functions have) another idea about
>> what ${words[@]} should contain: the array is prepopulated with the
>> words from the command it is completing.  For reasons that are not
>> well understood, when git-completion.bash reserves its own "words"
>> variable with "local words", the variable becomes empty and cannot be
>> changed from then on.  So the completion script neglects the arguments
>> it has seen, and words complete like git subcommand names.  For
>> example, typing "git log origi<TAB>" gives no completions because
>> there are no "git origi..." commands.
>>
>> Work around this by using a different variable (comp_words) that is
>> not special to zsh.  So now commands that completed correctly before
>> v1.7.4-rc0~11^2~2 on zsh should be able to complete correctly again.
>
> Thanks for this explanation.  I tried to fix this some time ago, but
> got only as far as to indentify that something is amiss with returning
> $words from _get_comp_words_by_ref(), and during tracing I saw so much
> weird and (for me) unexplicable zsh behavior that I simply gave up.
>
> But this patch heavily conflicted with one of my long-forgotten
> cleanup patch series, and that series together with the above
> explanation gave me alternative ideas for fixing this issue with zsh.
>
> So, here it is.  The first two patches are independent cleanups, but
> they make the actual fix in the third patch so short.
>
> It works well as far as I tested it with both bash and zsh, but I
> would really appreciate a few extra sets of eyeballs for the cleanups
> and sanity-checking and testing of the bugfix.
>
>
> SZEDER Gábor (3):
>  bash: don't modify the $cur variable in completion functions
>  bash: remove unnecessary _get_comp_words_by_ref() invocations
>  bash: don't declare 'local words' to make zsh happy
>
>  contrib/completion/git-completion.bash |  225 +++++++++-----------------------
>  1 files changed, 64 insertions(+), 161 deletions(-)
Nice! Much better approach.

I also noticed that behavior when not defining 'words' as local, but though modifying all those instances was overkill.

Acked-by: Felipe Contreras <felipe.contreras@gmail.com>
-- 
Felipe Contreras
Junio C Hamano· Apr 28, 2011, 20:52 UTC · re: Felipe Contreras · lore

Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 6 quoted lines
> Nice! Much better approach.
>
> I also noticed that behavior when not defining 'words' as local, but
> though modifying all those instances was overkill.
>
> Acked-by: Felipe Contreras <felipe.contreras@gmail.com>
Do you mean reviewed-by or even better tested-by?
Felipe Contreras· Apr 28, 2011, 21:27 UTC · re: Junio C Hamano · lore

Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability

On Thu, Apr 28, 2011 at 11:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 10 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Nice! Much better approach.
>>
>> I also noticed that behavior when not defining 'words' as local, but
>> though modifying all those instances was overkill.
>>
>> Acked-by: Felipe Contreras <felipe.contreras@gmail.com>
>
> Do you mean reviewed-by or even better tested-by?
Right. Reviewed-by.
-- 
Felipe Contreras
Felipe Contreras· Apr 27, 2011, 08:20 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-completion: fix zsh support

On Wed, Apr 27, 2011 at 7:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 17 quoted lines
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Felipe Contreras wrote:
>>
>>> It turns out 'words' is a special variable used by zsh completion, and
>>> it has some strange behavior as we can see.
>>>
>>> Better avoid it.
>>
>> Hoorah!  I imagine this fixes a regression introduced by
>> v1.7.4-rc0~11^2~2 (bash: get --pretty=m<tab> completion to work with
>> bash v4, 2010-12-02).
>>
>>  Acked-by: Jonathan Nieder <jrnieder@gmail.com>
>
> I'd love to share the enthusiasm, but find that "as we can see" needs a
> much more clarification.

Jonathan already described it: http://article.gmane.org/gmane.comp.version-control.git/170665

And this snipped demonstrates it:
---
set_vars ()
{
	cur="foo"
	words="foo"
	cwords="foo"
}
_foo ()
{
	local cur words cwords
	set_vars
	echo "cur=${cur} words=${words} cwords=${cwords}" >> /tmp/comp_test.txt
}

compdef _foo foo ---

When trying to auto-complete 'foo' the result would be:
  cur=foo words= cwords=foo

You can see it's special in the source code: http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=blob;f=Src/Zle/complete.c;h=6398fd3e77eff2ef819c10503d316b08421034ac;hb=HEAD#l1116

-- 
Felipe Contreras
Junio C Hamano· Apr 27, 2011, 16:56 UTC · re: Felipe Contreras · lore

Re: [PATCH] git-completion: fix zsh support

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 10 quoted lines
> On Wed, Apr 27, 2011 at 7:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
> ...
>> I'd love to share the enthusiasm, but find that "as we can see" needs a
>> much more clarification.
>
> Jonathan already described it:
> http://article.gmane.org/gmane.comp.version-control.git/170665
>
> And this snipped demonstrates it:
> ...

When I say "needs more clarification" during a review, I am not asking the contributor to explain it in the discussion thread to _me_ who happen to be asking at that moment. I am asking the contributor to explain it to people who will read "git log" output 6 months down the road.

You have been here long enough to know that "Jonathan already described it" that is not connected in the commit that is going to be recorded is not something we appreciate, no?

In any case, the message of Jonathan's
    Subject: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability
    Date: Wed, 27 Apr 2011 01:40:34 -0500
    Message-ID: <20110427064033.GB4226@elie>

seems to explain it better. The naming of variables and other details might need to be settled, but other than that is it correct to understand that we will see a final version along the line of that patch?

Thanks.
Felipe Contreras· Apr 27, 2011, 17:17 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-completion: fix zsh support

On Wed, Apr 27, 2011 at 7:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 21 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Wed, Apr 27, 2011 at 7:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> ...
>>> I'd love to share the enthusiasm, but find that "as we can see" needs a
>>> much more clarification.
>>
>> Jonathan already described it:
>> http://article.gmane.org/gmane.comp.version-control.git/170665
>>
>> And this snipped demonstrates it:
>> ...
>
> When I say "needs more clarification" during a review, I am not asking the
> contributor to explain it in the discussion thread to _me_ who happen to
> be asking at that moment.  I am asking the contributor to explain it to
> people who will read "git log" output 6 months down the road.
>
> You have been here long enough to know that "Jonathan already described
> it" that is not connected in the commit that is going to be recorded is
> not something we appreciate, no?

How is one supposed to know when the clarification is sufficient or not? Are you advocating communication through git patches?

I prefer to discuss in the mailing list, and when there's consensus fire 'git send-email', that's what they do in lkml.

Show 9 quoted lines
> In any case, the message of Jonathan's
>
>    Subject: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability
>    Date: Wed, 27 Apr 2011 01:40:34 -0500
>    Message-ID: <20110427064033.GB4226@elie>
>
> seems to explain it better.  The naming of variables and other details
> might need to be settled, but other than that is it correct to understand
> that we will see a final version along the line of that patch?
I prefer something short, and to the point:

--- git-completion: fix zsh support

Support for zsh was broken on commit da48616f1d[1], due to the fact that 'words' is a is a special variable used by zsh completion[2].

Jonathan Nieder found that 'typset -h' resets that special behavior, which is exactly what we want. So alias the currently used 'local' to that.

[1] http://article.gmane.org/gmane.comp.version-control.git/170665 [2] http://article.gmane.org/gmane.comp.shells.zsh.devel/22484 ---

-- 
Felipe Contreras
Jonathan Nieder· Apr 27, 2011, 02:21 UTC · re: Felipe Contreras · lore

Re: [PATCH] git-completion: fix zsh support

Felipe Contreras wrote:
> +++ b/contrib/completion/git-completion.bash
[...]
Show 9 quoted lines
> @@ -739,12 +739,12 @@ __git_complete_revlist ()
>  
>  __git_complete_remote_or_refspec ()
>  {
> -	local cur words cword
> -	_get_comp_words_by_ref -n =: cur words cword
> -	local cmd="${words[1]}"
> +	local cur cwords cword
> +	_get_comp_words_by_ref -n =: cur cwords cword
Hmm, on second thought, this will break the following case, in bash:
	. /etc/bash_completion;	# defines _get_comp_words_by_ref
	. contrib/completion/git-completion.bash

Not sure how to salvage that. Maybe we need a git-specific API that wraps _get_comp_words_by_ref when the latter is available.

	if type _get_comp_words_by_ref >/dev/null 2>&1; then
		_git_get_comp_words_by_ref () {
			_get_comp_words_by_ref "$@"
			if test "${words+set}"
			then
				cword="${words[@]}"
			fi
		}
	else
		_git_get_comp_words_by_ref () {
			...
		}
	fi

← back to recent threads