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

[Felipe Contreras <felipe.contreras@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3511 bytes --]

On Fri, May 6, 2011 at 2:25 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Felipe Contreras wrote:
>> 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 by using 'typedef -h', which
>> gets rid of any special meaning.
>
> As I mentioned before (sorry to come in late; I assume you forgot to
> cc the previous participants in the discussion?), I do not think this
> is a good fix.
>
> The point here is that 'words' is a special variable used by zsh
> completion, and we are using facilities from zsh completion.  So
> if you set 'typeset -h', then the zsh completion functions will
> use _our_ copy of "words".

No, the scope remains local.

>From the manual:
---
Hide: only useful for special parameters (those marked `<S>' in the
table in zshparam(1)), and for local parameters with the same name as
a special parameter, though harmless for others. A special parameter
with this attribute will not retain its special effect when made
local. Thus after `typeset -h PATH', a function containing `typeset
PATH' will create an ordinary local parameter without the usual
behaviour of PATH. Alterna‐ tively, the local parameter may itself be
given this attribute; hence inside a function `typeset -h PATH'
creates an ordinary local parameter and the special PATH parameter is
not altered in any way. It is also possible to create a local
parameter using `typeset +h special', where the local copy of special
will retain its special properties regardless of having the -h
attribute. Global special parameters loaded from shell modules (cur‐
rently those in zsh/mapfile and zsh/parameter) are automatically given
the -h attribute to avoid name clashes.
---

> Now in practice our copy of words matches zsh's anyway, so nothing
> goes wrong.  But that could easily change in the future.

It doesn't matter if we change the value of 'words'. See the attached
test. The result is:

 complete: words=foo  (before)
 foo: cur=foo words=blah cwords=1
 foo bar: cur=foo words=blah cwords=1
 complete: words=foo  (after)

See? No change.

In fact, if you follow the link I posted, that's precisely the fix the
zsh guys were pushing for. And the it is already merged:
http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=commitdiff;h=e880604f029088f32fb1ecc39213d720ae526aaa

I also discussed the workaround with zsh guys:
http://article.gmane.org/gmane.comp.shells.zsh.devel/22557

>> Currently zsh is completely broken after commit da48616 (bash: get
>> --pretty=m<tab> completion to work with bash v4), which introduced
>> _get_comp_words_by_ref() that comes from debian's bash_completion
>> scripts
>
> The bash_completion project does not originate in Debian fwiw; it
> was originally from Ian Macdonald iirc and available from
>
>  http://www.caliban.org/bash/index.shtml
>
> and then it was abandoned.  The bash-completion project on alioth is
> not Debian-specific, either.

Well, it's hosted on debian.org, and I haven't seen it used anywhere
else. I just don't know how else to identify that project.

> Maybe simplest would be to use Szeder's fix + make the zsh version of
> _get_comp_words_by_ref not overwrite "words" at all?

I think the simplest fix is the one I'm proposing, which zsh guys agree with.

Cheers.

-- 
Felipe Contreras

[-- Attachment #2: zsh_words_2 --]
[-- Type: application/octet-stream, Size: 717 bytes --]

#!zsh

set_vars ()
{
	cur=${COMP_WORDS[COMP_CWORD]}
	# words=("${COMP_WORDS[@]}")
	words="blah"
	cwords=$COMP_CWORD
}

_foo_bar ()
{
	local cur words cwords
	set_vars

	echo "foo bar: cur=${cur} words=${words} cwords=${cwords}" >> /tmp/comp_test.txt
}

_foo ()
{
	typeset -h words

	local cur words cwords
	set_vars

	echo "foo: cur=${cur} words=${words} cwords=${cwords}" >> /tmp/comp_test.txt

	_foo_bar
}

bash_complete ()
{
	(( COMP_CWORD = CURRENT - 1))
	COMP_WORDS=( $words )
	COMPREPLY=()

	echo "complete: words=${words} (before)" >> /tmp/comp_test.txt
	_foo "${words[0]}" "${words[CURRENT-1]}" "${words[CURRENT-2]}"
	echo "complete: words=${words} (after)" >> /tmp/comp_test.txt
}

compdef bash_complete foo