git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder@ira.uka.de>
To: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Cc: Stefan Haller <lists@haller-berlin.de>, git@vger.kernel.org
Subject: Re: Zsh completion regression
Date: Sat, 14 Jan 2012 14:23:43 +0100	[thread overview]
Message-ID: <20120114132343.GW30469@goldbirke> (raw)
In-Reply-To: <vpq62ghj7fn.fsf@bauges.imag.fr>

Hi,


On Thu, Jan 12, 2012 at 03:56:28PM +0100, Matthieu Moy wrote:
> lists@haller-berlin.de (Stefan Haller) writes:
> 
> > I'm using zsh   4.3.11.
> >
> > When I type "git log mas<TAB>", it completes to "git log master\ " (a
> > backslash, a space, and then the cursor).

Stefan, thanks for reporting and bisecting.

> The following patch makes the situation better, but is not really a fix:
> 
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -525,7 +525,7 @@ __gitcomp ()
>  __gitcomp_nl ()
>  {
>         local s=$'\n' IFS=' '$'\t'$'\n'
> -       local cur_="$cur" suffix=" "
> +       local cur_="$cur" suffix=""
>  
>         if [ $# -gt 2 ]; then
>                 cur_="$3"
> 
> With this, the trailing space isn't added,

Yeah, this would be a regression for Bash users, because then they
will need to manually append that space.

> 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)?
I'm not a zsh user myself, but just tried it and found that in
a31e626^ 'git checkout master<TAB>' doesn't add the trailing space,
and neither does 'git checkout mas<TAB>', which is in sync with what
Stefan reported ("Before this commit, I get "git log master" (with no
space at the end).").

> The problem is a little bit below:
> 
> 	IFS=$s
> 	COMPREPLY=($(compgen -P "${2-}" -S "$suffix" -W "$1" -- "$cur_"))
> 
> The -S "$suffix" adds a space to the completion, but ZSH escapes the
> space (which sounds sensible in general, but is not at all what we
> expect).

So it seems we have two issues here with zsh:

- Spaces appended with 'compgen -S " "' in __gitcomp_nl() are quoted.
  This is the regression caused by a31e6262.
- Spaces appended in __gitcomp_1() (by echo "$word ") are stripped.
  This is a long-standing issue; if those spaces weren't stripped, the
  quoting issue would have been noted earlier, I suspect.

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_"))


Although it wouldn't append a space at all for zsh users, that's
nothing new, it's just restoring old behavior.  And then later someone
more knowledgable about the quirks of zsh's completion and in
particular zsh's bash completion emulation can come up with a proper
fix to the issue of quoted spaces and stripped trailing spaces.


However.

While playing around with zsh, I noticed that git completion works
without even sourcing git's bash completion script.  As it turned out,
zsh ships its own git completion script[1], and from my cursory tests
it seems to be quite capable: it supports commands, aliases, options,
refs, ref1..ref2, config variables, ...  and I also saw a __git_ps1()
equivalent for zsh.

So, is there any reason why you are still using git's bash completion
under zsh (which has some quirks and lacks some features) instead of
zsh's own?  Perhaps it would make sense to point zsh users to zsh's
git completion and drop zsh compatibility from git's bash completion.
We did similar with vim config files: git included a vim syntax
highlight config file for commit messages under contrib/vim/, but
eventually we dropped it after vim started shipping more capable
git-specific config files (for git config files, rebase instruction
sheets, etc.).


[1] http://zsh.git.sourceforge.net/git/gitweb.cgi?p=zsh/zsh;a=blob;f=Completion/Unix/Command/_git;hb=HEAD


Best,
Gábor

  reply	other threads:[~2012-01-14 13:23 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-12 11:52 Zsh completion regression Stefan Haller
2012-01-12 14:56 ` Matthieu Moy
2012-01-14 13:23   ` SZEDER Gábor [this message]
2012-01-14 14:32     ` Matthieu Moy
2012-01-14 15:20       ` SZEDER Gábor
2012-01-14 18:55         ` [PATCH] bash-completion: don't add quoted space for ZSH (fix regression) Matthieu Moy
2012-01-15  2:29           ` Junio C Hamano
2012-01-16 11:49             ` Matthieu Moy
2012-01-16 22:47               ` Junio C Hamano
2012-01-17 12:21                 ` Matthieu Moy
2012-01-17 18:46                   ` Junio C Hamano
2012-01-17 19:18           ` Felipe Contreras
2012-01-17 20:03             ` Felipe Contreras
2012-01-17 20:11               ` Junio C Hamano
2012-01-17 23:04                 ` Felipe Contreras
2012-01-17 23:42                   ` Junio C Hamano
2012-01-18  8:16                     ` Matthieu Moy
2012-01-25  1:39                       ` Felipe Contreras
2012-01-25  4:06                         ` Junio C Hamano
2012-01-14 21:36         ` Zsh completion regression Stefan Haller
2012-01-14 21:36     ` Stefan Haller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120114132343.GW30469@goldbirke \
    --to=szeder@ira.uka.de \
    --cc=Matthieu.Moy@grenoble-inp.fr \
    --cc=git@vger.kernel.org \
    --cc=lists@haller-berlin.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).