Re: Zsh completion regression

SZEDER Gábor [off-list ref] writes:

quoted

but e.g. "git checkout
master<TAB>" does not add the trailing space, at all.

I'm not sure what you mean; did you got a trailing space after
'master<TAB>' before a31e6262 (completion: optimize refs completion,
2011-10-15)?

No. My above sentence should read "... not add the trailing space, at
all, even for bash users". IOW, your understanding is correct.

I hate to see special case for different shells, but if no one finds a
better solution, then yes, this is the way to go. Not having the space
may be irritating, but having the quoted space hurts really much more (I
have to delete the space and the backslash manually to continue).

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Hi,


On Sat, Jan 14, 2012 at 03:32:08PM +0100, Matthieu Moy wrote:

SZEDER Gábor [off-list ref] writes:

quoted

We could fix the regression by not appending a space suffix to
completion words in __gitcomp_nl(), but only when the completion
script is running under zsh to avoid hurting bash users, like this:

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2d02a7f3..49393243 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -601,6 +601,9 @@ __gitcomp_nl ()
 			suffix="$4"
 		fi
 	fi
+	if [ -n "${ZSH_VERSION-}" ] && [ "$suffix" = " " ]; then
+		suffix=""
+	fi
 
 	IFS=$s
 	COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))

I hate to see special case for different shells,

I agree and thought about that, too.  We are dealing with zsh quirks
in the completion script either by

- having whole functions with different definitions in zsh and
  in bash (currently__git_shopt() and _get_comp_words_by_ref()), or 
- having similar shell-specific cases inside functions (currently
  _git() and _gitk()).

We use the former when the implementations for the two shells differ
significantly, and the latter when the shell-specific parts are small
and most of the function's implementation can be used in both shells.
I think this regression fix fits into the latter category.

but if no one finds a
better solution, then yes, this is the way to go. Not having the space
may be irritating, but having the quoted space hurts really much more (I
have to delete the space and the backslash manually to continue).

I have no better idea for fixing this regression, and no idea at all
for a proper fix (i.e. to make zsh behave the same way in this respect
as bash).  I can imagine that it's irritating, that's why I aimed for
this minimal regression fix, which, maybe with a Tested-by from you or
Stefan, can even go into maint, so affected users can get it faster.


Best,
Gábor

Commit a31e626 (completion: optimize refs completion) introduced a
regression for ZSH users: ref names were completed with a quoted trailing
space (i.e. "git checkout ma" completes to "git checkout master\ "). The
space is convenient for bash users since we use "-o nospace", but a
quoted space is worse than nothing. The absence of trailing space for ZSH
is a long-standing issue, that this patch is not fixing. We just fix the
regression by not appending a space when the shell is ZSH.

Original-patch-by: SZEDER Gábor [off-list ref]
Reported-by: Stefan Haller <redacted>
Signed-off-by: Matthieu Moy <redacted>
---
 contrib/completion/git-completion.bash |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b0062ba..488e1f4 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash

@@ -534,6 +534,12 @@ __gitcomp_nl ()
 		fi
 	fi
 
+	# ZSH would quote the trailing space added with -S. bash users
+	# will appreciate the extra space to compensate the use of -o nospace.
+	if [ -n "${ZSH_VERSION-}" ] && [ "$suffix" = " " ]; then
+		suffix=""
+	fi
+
 	IFS=$s
 	COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
 }

-- 
1.7.9.rc0.25.gb9a1f.dirty

SZEDER Gábor [off-list ref] wrote:

I can imagine that it's irritating, that's why I aimed for
this minimal regression fix, which, maybe with a Tested-by from you or
Stefan, can even go into maint, so affected users can get it faster.

The patch works well for me, so feel free to add a Tested-by from me.


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/

On Sat, Jan 14, 2012 at 8:55 PM, Matthieu Moy [off-list ref] wrote:

Commit a31e626 (completion: optimize refs completion) introduced a
regression for ZSH users: ref names were completed with a quoted trailing
space (i.e. "git checkout ma" completes to "git checkout master\ "). The
space is convenient for bash users since we use "-o nospace", but a
quoted space is worse than nothing. The absence of trailing space for ZSH
is a long-standing issue, that this patch is not fixing. We just fix the
regression by not appending a space when the shell is ZSH.

I have this issue with the script from v1.7.8.3, and I think it
started with a zsh update.

-- 
Felipe Contreras

On Tue, Jan 17, 2012 at 9:18 PM, Felipe Contreras
[off-list ref] wrote:

On Sat, Jan 14, 2012 at 8:55 PM, Matthieu Moy [off-list ref] wrote:

quoted

Commit a31e626 (completion: optimize refs completion) introduced a
regression for ZSH users: ref names were completed with a quoted trailing
space (i.e. "git checkout ma" completes to "git checkout master\ "). The
space is convenient for bash users since we use "-o nospace", but a
quoted space is worse than nothing. The absence of trailing space for ZSH
is a long-standing issue, that this patch is not fixing. We just fix the
regression by not appending a space when the shell is ZSH.

I have this issue with the script from v1.7.8.3, and I think it
started with a zsh update.

Yeah, works fine with zsh 4.3.11, not 4.3.14 or 15.

-- 
Felipe Contreras