git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org, "SZEDER Gábor" <szeder@ira.uka.de>
Subject: Re: [PATCH v5 (for maint)] git-completion: fix regression in zsh support
Date: Tue, 10 May 2011 17:16:00 -0500	[thread overview]
Message-ID: <20110510221600.GD1994@elie> (raw)
In-Reply-To: <1305030039-16044-1-git-send-email-felipe.contreras@gmail.com>

Felipe Contreras wrote:

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

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

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

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

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

 - I included the "completion: move private shopt shim for zsh to
   __git_ namespace" patch in the series as a reminder to Junio,
   since he had requested one.

 - "in no part of [compgen] does the zsh emulation make use of the
   'words' special variable" is just plain false.  compgen -F uses the
   'words' variable, and we are lucky to have not needed to use it
   yet.

 - I changed the comment because when I read the original, putting
   myself in the shoes of someone who didn't know the context, I found
   it confusing.  "work around bug in old versions of zsh" just leaves
   me wondering "What bug?  Is it a bug in the behavior of 'local',
   and are we working around it by using an alternate spelling?"  

   That said, I suppose the main point of a comment is to say
   "something important happens here", and a person can use 

	git log -S"workaround zsh's bug" -- contrib/completion/git-completion.bash

   to find out what exactly it is, so no harm done.

      parent reply	other threads:[~2011-05-10 22:16 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-10 12:20 [PATCH v5 (for maint)] git-completion: fix regression in zsh support Felipe Contreras
2011-05-10 17:17 ` Junio C Hamano
2011-05-10 17:36   ` Felipe Contreras
2011-05-10 18:27     ` Junio C Hamano
2011-05-10 22:18       ` Jonathan Nieder
2011-05-10 22:16 ` Jonathan Nieder [this message]

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=20110510221600.GD1994@elie \
    --to=jrnieder@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=szeder@ira.uka.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).