threads / patch / 27314

v5git-completion: fix regression in zsh support

Subject: [PATCH v5 (for maint)] git-completion: fix regression in zsh support

## tl;dr

6 messages between May 10, 2011 and May 10, 2011. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Felipe Contreras· May 10, 2011, 12:20 UTC · lore

Right now zsh is quite broken; the completion doesn't notice when there's a subcommand. For example: "git log origi<TAB>" gives no completions because it would try to find a "git origi..." command. The issue would be gone in zsh 4.3.12, but for now we can workaround it by porting the same fix.

The problem started after commit v1.7.4-rc0~11^2~2 (bash: get --pretty=m<tab> completion to work with bash v4), which introduced _get_comp_words_by_ref() that comes from bash-completion[1] scripts, and relies on the 'words' variable. However, it turns out 'words' is a special variable used by zsh completion. From zshcompwid(1):

  [...] the parameters are reset on each function exit (including nested
  function calls from within the completion widget) to the values they
  had when the function was entered.
As a result, subcommand words are lost.

This is now fixed in the latest master branch of zsh[2] by simply defining 'words' as hidden (typeset -h), which removes the special meaning inside the emulated bash function. So let's do the same.

Jonathan Nieder helped on the commit message.

[1] http://bash-completion.alioth.debian.org/ [2] http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=commitdiff;h=e880604f029088f32fb1ecc39213d720ae526aaa

Reported-by: Stefan Haller <lists@haller-berlin.de>
Comments-by: SZEDER Gábor <szeder@ira.uka.de>
Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
v2: fix _gitk() too as Szeder suggested.
v3: improve commit message as Jonathan Nieder suggested.
    Also, improve comments.
v4: more commit message improvements
v5: even more commit message improvements from Jonathan's version
Show changes to contrib/completion/git-completion.bash +8 −0
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 840ae38..a7d20df 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2710,6 +2710,10 @@ _git ()
 	if [[ -n ${ZSH_VERSION-} ]]; then
 		emulate -L bash
 		setopt KSH_TYPESET
+
+		# workaround zsh's bug that leaves 'words' as a special
+		# variable in versions < 4.3.12
+		typeset -h words
 	fi
 
 	local cur words cword
@@ -2761,6 +2765,10 @@ _gitk ()
 	if [[ -n ${ZSH_VERSION-} ]]; then
 		emulate -L bash
 		setopt KSH_TYPESET
+
+		# workaround zsh's bug that leaves 'words' as a special
+		# variable in versions < 4.3.12
+		typeset -h words
 	fi
 
 	__git_has_doubledash && return
-- 
1.7.5.1
Junio C Hamano· May 10, 2011, 17:17 UTC · re: Felipe Contreras · lore

Re: [PATCH v5 (for maint)] git-completion: fix regression in zsh support

Felipe Contreras <felipe.contreras@gmail.com> writes:
> Right now zsh is quite broken;...

Yuck--sounds like zsh as a whole is broken, but certainly you didn't mean that ;-) Could you also clarify "Right now" a bit?

Is "The git-completion script in contrib/ in 1.7.5 is broken for versions of zsh before 4.3.12" a good rewrite of the above?

No need for resend if that is the only change required.
Felipe Contreras· May 10, 2011, 17:36 UTC · re: Junio C Hamano · lore

Re: [PATCH v5 (for maint)] git-completion: fix regression in zsh support

On Tue, May 10, 2011 at 8:17 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 6 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Right now zsh is quite broken;...
>
> Yuck--sounds like zsh as a whole is broken, but certainly you didn't mean
> that ;-)
Right: s/zsh/zsh support/
> Could you also clarify "Right now" a bit?
>
> Is "The git-completion script in contrib/ in 1.7.5 is broken for versions
> of zsh before 4.3.12" a good rewrite of the above?

Well, "right now" all versions of zsh are before 4.3.12. I also don't see the point mentioning of 1.7.5. Maybe "The git-completion script in contrib/ is quite broken".

> of zsh before 4.3.12"
> No need for resend if that is the only change required.
All right. Hopefully there won't be other changes.
-- 
Felipe Contreras
Junio C Hamano· May 10, 2011, 18:27 UTC · re: Felipe Contreras · lore

Re: [PATCH v5 (for maint)] git-completion: fix regression in zsh support

Felipe Contreras <felipe.contreras@gmail.com> writes:
> Well, "right now" all versions of zsh are before 4.3.12.

I was trying to help people who read the log in 12 months. They can certainly see the date of the commit, but then they have to correlate that date with the then-current version of zsh if you do not mention it.

> I also don't
> see the point mentioning of 1.7.5. Maybe "The git-completion script in
> contrib/ is quite broken".

That is Ok by me, if you mean this zsh breakages is not something our recent change has exposed. Mentioning 1.7.5 only helps the users if git-completion.bash in 1.7.4 did not have an issue even on the current zsh and 1.7.5 had an issue. I thought you may know which without me having to dig that information out.

Jonathan Nieder· May 10, 2011, 22:18 UTC · re: Junio C Hamano · lore

Re: [PATCH v5 (for maint)] git-completion: fix regression in zsh support

Junio C Hamano wrote:
> Mentioning 1.7.5 only helps the users if
> git-completion.bash in 1.7.4 did not have an issue even on the current zsh
> and 1.7.5 had an issue. I thought you may know which without me having to
> dig that information out.

The zsh support was introduced in 1.7.4-rc0[1] and the breakage was introduced in 1.7.4-rc0[2], for what it's worth.

[1] v1.7.4-rc0~169^2 (completion: make compatible with zsh, 2010-09-06) [2] v1.7.4-rc0~11^2~2 (bash: get --pretty=m<tab> completion to work with bash v4, 2010-12-02)

Jonathan Nieder· May 10, 2011, 22:16 UTC · re: Felipe Contreras · lore

Re: [PATCH v5 (for maint)] git-completion: fix regression in zsh support

Felipe Contreras wrote:
Show 10 quoted lines
> Right now zsh is quite broken; the completion doesn't notice when
> there's a subcommand. For example: "git log origi<TAB>" gives no
> completions because it would try to find a "git origi..." command. The
> issue would be gone in zsh 4.3.12, but for now we can workaround it by
> porting the same fix.
>
> The problem started after commit v1.7.4-rc0~11^2~2 (bash: get
> --pretty=m<tab> completion to work with bash v4), which introduced
> _get_comp_words_by_ref() that comes from bash-completion[1] scripts, and
> relies on the 'words' variable. However, it turns out 'words' is a

The _get_comp_words_by_ref function does not rely on but writes to the 'words' variable. Aside from that, this is a nicer summary.

> special variable used by zsh completion. From zshcompwid(1):
[...]
> [2] http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=commitdiff;h=e880604f029088f32fb1ecc39213d720ae526aaa

I doubt this URL will last forever. Why not just say commit e880604f029088f32fb1ecc, or '"29140: hide the "words" special variable so that it may be used as an ordinary variable by bash completions", 2011-05-04' which conveniently includes a mailing list reference?

Thanks for the fix and your patience with me. To answer the sincere questions I could find among your replies to v4:

 - I included the "completion: move private shopt shim for zsh to
   __git_ namespace" patch in the series as a reminder to Junio,
   since he had requested one.
 - "in no part of [compgen] does the zsh emulation make use of the
   'words' special variable" is just plain false.  compgen -F uses the
   'words' variable, and we are lucky to have not needed to use it
   yet.
 - I changed the comment because when I read the original, putting
   myself in the shoes of someone who didn't know the context, I found
   it confusing.  "work around bug in old versions of zsh" just leaves
   me wondering "What bug?  Is it a bug in the behavior of 'local',
   and are we working around it by using an alternate spelling?"  
   That said, I suppose the main point of a comment is to say
   "something important happens here", and a person can use 
	git log -S"workaround zsh's bug" -- contrib/completion/git-completion.bash
   to find out what exactly it is, so no harm done.

← back to recent threads