* [PATCH] git-completion: fix zsh support
@ 2011-04-27 1:26 Felipe Contreras
2011-04-27 1:35 ` Jonathan Nieder
2011-04-27 2:21 ` Jonathan Nieder
0 siblings, 2 replies; 28+ messages in thread
From: Felipe Contreras @ 2011-04-27 1:26 UTC (permalink / raw)
To: git; +Cc: Jonathan Nieder, Felipe Contreras
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(-)
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] git-completion: fix zsh support
2011-04-27 1:26 [PATCH] git-completion: fix zsh support Felipe Contreras
@ 2011-04-27 1:35 ` Jonathan Nieder
2011-04-27 1:42 ` Felipe Contreras
2011-04-27 4:55 ` Junio C Hamano
2011-04-27 2:21 ` Jonathan Nieder
1 sibling, 2 replies; 28+ messages in thread
From: Jonathan Nieder @ 2011-04-27 1:35 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Stefan Haller, SZEDER Gábor, Mark Lodato
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>
>
> 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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] git-completion: fix zsh support
2011-04-27 1:35 ` Jonathan Nieder
@ 2011-04-27 1:42 ` Felipe Contreras
2011-04-27 4:55 ` Junio C Hamano
1 sibling, 0 replies; 28+ messages in thread
From: Felipe Contreras @ 2011-04-27 1:42 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: git, Stefan Haller, SZEDER Gábor, Mark Lodato
On Wed, Apr 27, 2011 at 4:35 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> 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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] git-completion: fix zsh support
2011-04-27 1:26 [PATCH] git-completion: fix zsh support Felipe Contreras
2011-04-27 1:35 ` Jonathan Nieder
@ 2011-04-27 2:21 ` Jonathan Nieder
1 sibling, 0 replies; 28+ messages in thread
From: Jonathan Nieder @ 2011-04-27 2:21 UTC (permalink / raw)
To: Felipe Contreras; +Cc: git, Stefan Haller, SZEDER Gábor, Mark Lodato
Felipe Contreras wrote:
> +++ b/contrib/completion/git-completion.bash
[...]
> @@ -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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] git-completion: fix zsh support
2011-04-27 1:35 ` Jonathan Nieder
2011-04-27 1:42 ` Felipe Contreras
@ 2011-04-27 4:55 ` Junio C Hamano
2011-04-27 6:40 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability Jonathan Nieder
2011-04-27 8:20 ` [PATCH] git-completion: fix zsh support Felipe Contreras
1 sibling, 2 replies; 28+ messages in thread
From: Junio C Hamano @ 2011-04-27 4:55 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Felipe Contreras, git, Stefan Haller, SZEDER Gábor,
Mark Lodato
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [RFC/PATCH] completion: avoid "words" as variable name for zsh portability
2011-04-27 4:55 ` Junio C Hamano
@ 2011-04-27 6:40 ` Jonathan Nieder
2011-04-27 8:42 ` Felipe Contreras
2011-04-28 16:01 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability SZEDER Gábor
2011-04-27 8:20 ` [PATCH] git-completion: fix zsh support Felipe Contreras
1 sibling, 2 replies; 28+ messages in thread
From: Jonathan Nieder @ 2011-04-27 6:40 UTC (permalink / raw)
To: Junio C Hamano
Cc: Felipe Contreras, git, Stefan Haller, SZEDER Gábor,
Mark Lodato
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(-)
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] git-completion: fix zsh support
2011-04-27 4:55 ` Junio C Hamano
2011-04-27 6:40 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability Jonathan Nieder
@ 2011-04-27 8:20 ` Felipe Contreras
2011-04-27 16:56 ` Junio C Hamano
1 sibling, 1 reply; 28+ messages in thread
From: Felipe Contreras @ 2011-04-27 8:20 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, Stefan Haller, SZEDER Gábor,
Mark Lodato
On Wed, Apr 27, 2011 at 7:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability
2011-04-27 6:40 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability Jonathan Nieder
@ 2011-04-27 8:42 ` Felipe Contreras
2011-04-27 9:11 ` Jonathan Nieder
2011-04-28 16:01 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability SZEDER Gábor
1 sibling, 1 reply; 28+ messages in thread
From: Felipe Contreras @ 2011-04-27 8:42 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, git, Stefan Haller, SZEDER Gábor,
Mark Lodato
On Wed, Apr 27, 2011 at 9:40 AM, Jonathan Nieder <jrnieder@gmail.com> 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?
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability
2011-04-27 8:42 ` Felipe Contreras
@ 2011-04-27 9:11 ` Jonathan Nieder
2011-04-27 9:49 ` Felipe Contreras
0 siblings, 1 reply; 28+ messages in thread
From: Jonathan Nieder @ 2011-04-27 9:11 UTC (permalink / raw)
To: Felipe Contreras
Cc: Junio C Hamano, git, Stefan Haller, SZEDER Gábor,
Mark Lodato
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability
2011-04-27 9:11 ` Jonathan Nieder
@ 2011-04-27 9:49 ` Felipe Contreras
2011-04-27 9:59 ` John Szakmeister
2011-04-27 10:09 ` Felipe Contreras
0 siblings, 2 replies; 28+ messages in thread
From: Felipe Contreras @ 2011-04-27 9:49 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, git, Stefan Haller, SZEDER Gábor,
Mark Lodato
On Wed, Apr 27, 2011 at 12:11 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> 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?
> 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.
>>> 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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability
2011-04-27 9:49 ` Felipe Contreras
@ 2011-04-27 9:59 ` John Szakmeister
2011-04-27 10:09 ` Felipe Contreras
1 sibling, 0 replies; 28+ messages in thread
From: John Szakmeister @ 2011-04-27 9:59 UTC (permalink / raw)
To: Felipe Contreras
Cc: Jonathan Nieder, Junio C Hamano, git, Stefan Haller,
SZEDER Gábor, Mark Lodato
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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability
2011-04-27 9:49 ` Felipe Contreras
2011-04-27 9:59 ` John Szakmeister
@ 2011-04-27 10:09 ` Felipe Contreras
2011-04-27 21:27 ` [PATCH] completion: move private shopt shim for zsh to __git_ namespace Jonathan Nieder
1 sibling, 1 reply; 28+ messages in thread
From: Felipe Contreras @ 2011-04-27 10:09 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, git, Stefan Haller, SZEDER Gábor,
Mark Lodato
On Wed, Apr 27, 2011 at 12:49 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> 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:
--- 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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] git-completion: fix zsh support
2011-04-27 8:20 ` [PATCH] git-completion: fix zsh support Felipe Contreras
@ 2011-04-27 16:56 ` Junio C Hamano
2011-04-27 17:17 ` Felipe Contreras
0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2011-04-27 16:56 UTC (permalink / raw)
To: Felipe Contreras
Cc: Jonathan Nieder, git, Stefan Haller, SZEDER Gábor,
Mark Lodato
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?
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] git-completion: fix zsh support
2011-04-27 16:56 ` Junio C Hamano
@ 2011-04-27 17:17 ` Felipe Contreras
0 siblings, 0 replies; 28+ messages in thread
From: Felipe Contreras @ 2011-04-27 17:17 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, git, Stefan Haller, SZEDER Gábor,
Mark Lodato
On Wed, Apr 27, 2011 at 7:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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.
> 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
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH] completion: move private shopt shim for zsh to __git_ namespace
2011-04-27 10:09 ` Felipe Contreras
@ 2011-04-27 21:27 ` Jonathan Nieder
2011-04-27 22:48 ` Felipe Contreras
2011-05-06 5:46 ` Jonathan Nieder
0 siblings, 2 replies; 28+ messages in thread
From: Jonathan Nieder @ 2011-04-27 21:27 UTC (permalink / raw)
To: Felipe Contreras
Cc: Junio C Hamano, git, Stefan Haller, SZEDER Gábor,
Mark Lodato
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:
> +++ 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(-)
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] completion: move private shopt shim for zsh to __git_ namespace
2011-04-27 21:27 ` [PATCH] completion: move private shopt shim for zsh to __git_ namespace Jonathan Nieder
@ 2011-04-27 22:48 ` Felipe Contreras
2011-04-27 23:00 ` Jonathan Nieder
2011-05-06 5:46 ` Jonathan Nieder
1 sibling, 1 reply; 28+ messages in thread
From: Felipe Contreras @ 2011-04-27 22:48 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, git, Stefan Haller, SZEDER Gábor,
Mark Lodato
On Thu, Apr 28, 2011 at 12:27 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> 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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] completion: move private shopt shim for zsh to __git_ namespace
2011-04-27 22:48 ` Felipe Contreras
@ 2011-04-27 23:00 ` Jonathan Nieder
0 siblings, 0 replies; 28+ messages in thread
From: Jonathan Nieder @ 2011-04-27 23:00 UTC (permalink / raw)
To: Felipe Contreras
Cc: Junio C Hamano, git, Stefan Haller, SZEDER Gábor,
Mark Lodato
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.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability
2011-04-27 6:40 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability Jonathan Nieder
2011-04-27 8:42 ` Felipe Contreras
@ 2011-04-28 16:01 ` SZEDER Gábor
2011-04-28 16:01 ` [PATCH 1/3] bash: don't modify the $cur variable in completion functions SZEDER Gábor
2011-04-28 20:24 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability Felipe Contreras
1 sibling, 2 replies; 28+ messages in thread
From: SZEDER Gábor @ 2011-04-28 16:01 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Junio C Hamano, Felipe Contreras, git, Stefan Haller, Mark Lodato
Hi,
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(-)
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 1/3] bash: don't modify the $cur variable in completion functions
2011-04-28 16:01 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability SZEDER Gábor
@ 2011-04-28 16:01 ` SZEDER Gábor
2011-04-28 16:01 ` [PATCH 2/3] bash: remove unnecessary _get_comp_words_by_ref() invocations SZEDER Gábor
2011-04-28 16:01 ` [PATCH 3/3] bash: don't declare 'local words' to make zsh happy SZEDER Gábor
2011-04-28 20:24 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability Felipe Contreras
1 sibling, 2 replies; 28+ messages in thread
From: SZEDER Gábor @ 2011-04-28 16:01 UTC (permalink / raw)
To: Jonathan Nieder, Junio C Hamano
Cc: Felipe Contreras, git, Stefan Haller, Mark Lodato,
SZEDER Gábor
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(-)
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/3] bash: remove unnecessary _get_comp_words_by_ref() invocations
2011-04-28 16:01 ` [PATCH 1/3] bash: don't modify the $cur variable in completion functions SZEDER Gábor
@ 2011-04-28 16:01 ` SZEDER Gábor
2011-04-28 16:01 ` [PATCH 3/3] bash: don't declare 'local words' to make zsh happy SZEDER Gábor
1 sibling, 0 replies; 28+ messages in thread
From: SZEDER Gábor @ 2011-04-28 16:01 UTC (permalink / raw)
To: Jonathan Nieder, Junio C Hamano
Cc: Felipe Contreras, git, Stefan Haller, Mark Lodato,
SZEDER Gábor
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(-)
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/3] bash: don't declare 'local words' to make zsh happy
2011-04-28 16:01 ` [PATCH 1/3] bash: don't modify the $cur variable in completion functions SZEDER Gábor
2011-04-28 16:01 ` [PATCH 2/3] bash: remove unnecessary _get_comp_words_by_ref() invocations SZEDER Gábor
@ 2011-04-28 16:01 ` SZEDER Gábor
2011-05-03 17:53 ` Felipe Contreras
1 sibling, 1 reply; 28+ messages in thread
From: SZEDER Gábor @ 2011-04-28 16:01 UTC (permalink / raw)
To: Jonathan Nieder, Junio C Hamano
Cc: Felipe Contreras, git, Stefan Haller, Mark Lodato,
SZEDER Gábor
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability
2011-04-28 16:01 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability SZEDER Gábor
2011-04-28 16:01 ` [PATCH 1/3] bash: don't modify the $cur variable in completion functions SZEDER Gábor
@ 2011-04-28 20:24 ` Felipe Contreras
2011-04-28 20:52 ` Junio C Hamano
1 sibling, 1 reply; 28+ messages in thread
From: Felipe Contreras @ 2011-04-28 20:24 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Jonathan Nieder, Junio C Hamano, git, Stefan Haller, Mark Lodato
2011/4/28 SZEDER Gábor <szeder@ira.uka.de>:
> 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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability
2011-04-28 20:24 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability Felipe Contreras
@ 2011-04-28 20:52 ` Junio C Hamano
2011-04-28 21:27 ` Felipe Contreras
0 siblings, 1 reply; 28+ messages in thread
From: Junio C Hamano @ 2011-04-28 20:52 UTC (permalink / raw)
To: Felipe Contreras
Cc: SZEDER Gábor, Jonathan Nieder, Junio C Hamano, git,
Stefan Haller, Mark Lodato
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?
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability
2011-04-28 20:52 ` Junio C Hamano
@ 2011-04-28 21:27 ` Felipe Contreras
0 siblings, 0 replies; 28+ messages in thread
From: Felipe Contreras @ 2011-04-28 21:27 UTC (permalink / raw)
To: Junio C Hamano
Cc: SZEDER Gábor, Jonathan Nieder, git, Stefan Haller,
Mark Lodato
On Thu, Apr 28, 2011 at 11:52 PM, Junio C Hamano <gitster@pobox.com> wrote:
> 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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 3/3] bash: don't declare 'local words' to make zsh happy
2011-04-28 16:01 ` [PATCH 3/3] bash: don't declare 'local words' to make zsh happy SZEDER Gábor
@ 2011-05-03 17:53 ` Felipe Contreras
0 siblings, 0 replies; 28+ messages in thread
From: Felipe Contreras @ 2011-05-03 17:53 UTC (permalink / raw)
To: SZEDER Gábor
Cc: Jonathan Nieder, Junio C Hamano, git, Stefan Haller, Mark Lodato
On Thu, Apr 28, 2011 at 7:01 PM, SZEDER Gábor <szeder@ira.uka.de> 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.
>
> 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(-)
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
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH] completion: move private shopt shim for zsh to __git_ namespace
2011-04-27 21:27 ` [PATCH] completion: move private shopt shim for zsh to __git_ namespace Jonathan Nieder
2011-04-27 22:48 ` Felipe Contreras
@ 2011-05-06 5:46 ` Jonathan Nieder
2011-05-06 8:35 ` Felipe Contreras
2011-05-08 10:48 ` SZEDER Gábor
1 sibling, 2 replies; 28+ messages in thread
From: Jonathan Nieder @ 2011-05-06 5:46 UTC (permalink / raw)
To: Felipe Contreras; +Cc: Junio C Hamano, git, SZEDER Gábor
(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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] completion: move private shopt shim for zsh to __git_ namespace
2011-05-06 5:46 ` Jonathan Nieder
@ 2011-05-06 8:35 ` Felipe Contreras
2011-05-08 10:48 ` SZEDER Gábor
1 sibling, 0 replies; 28+ messages in thread
From: Felipe Contreras @ 2011-05-06 8:35 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Junio C Hamano, git, SZEDER Gábor
On Fri, May 6, 2011 at 8:46 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> (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
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH] completion: move private shopt shim for zsh to __git_ namespace
2011-05-06 5:46 ` Jonathan Nieder
2011-05-06 8:35 ` Felipe Contreras
@ 2011-05-08 10:48 ` SZEDER Gábor
1 sibling, 0 replies; 28+ messages in thread
From: SZEDER Gábor @ 2011-05-08 10:48 UTC (permalink / raw)
To: Jonathan Nieder; +Cc: Felipe Contreras, Junio C Hamano, git
On Fri, May 06, 2011 at 12:46:04AM -0500, Jonathan Nieder wrote:
> (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>
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2011-05-08 10:48 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-27 1:26 [PATCH] git-completion: fix zsh support Felipe Contreras
2011-04-27 1:35 ` Jonathan Nieder
2011-04-27 1:42 ` Felipe Contreras
2011-04-27 4:55 ` Junio C Hamano
2011-04-27 6:40 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability Jonathan Nieder
2011-04-27 8:42 ` Felipe Contreras
2011-04-27 9:11 ` Jonathan Nieder
2011-04-27 9:49 ` Felipe Contreras
2011-04-27 9:59 ` John Szakmeister
2011-04-27 10:09 ` Felipe Contreras
2011-04-27 21:27 ` [PATCH] completion: move private shopt shim for zsh to __git_ namespace Jonathan Nieder
2011-04-27 22:48 ` Felipe Contreras
2011-04-27 23:00 ` Jonathan Nieder
2011-05-06 5:46 ` Jonathan Nieder
2011-05-06 8:35 ` Felipe Contreras
2011-05-08 10:48 ` SZEDER Gábor
2011-04-28 16:01 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability SZEDER Gábor
2011-04-28 16:01 ` [PATCH 1/3] bash: don't modify the $cur variable in completion functions SZEDER Gábor
2011-04-28 16:01 ` [PATCH 2/3] bash: remove unnecessary _get_comp_words_by_ref() invocations SZEDER Gábor
2011-04-28 16:01 ` [PATCH 3/3] bash: don't declare 'local words' to make zsh happy SZEDER Gábor
2011-05-03 17:53 ` Felipe Contreras
2011-04-28 20:24 ` [RFC/PATCH] completion: avoid "words" as variable name for zsh portability Felipe Contreras
2011-04-28 20:52 ` Junio C Hamano
2011-04-28 21:27 ` Felipe Contreras
2011-04-27 8:20 ` [PATCH] git-completion: fix zsh support Felipe Contreras
2011-04-27 16:56 ` Junio C Hamano
2011-04-27 17:17 ` Felipe Contreras
2011-04-27 2:21 ` Jonathan Nieder
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).