git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder@ira.uka.de>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>,
	Felipe Contreras <felipe.contreras@gmail.com>,
	<git@vger.kernel.org>, Stefan Haller <lists@haller-berlin.de>,
	Mark Lodato <lodatom@gmail.com>
Subject: Re: [RFC/PATCH] completion: avoid "words" as variable name for zsh portability
Date: Thu, 28 Apr 2011 18:01:15 +0200	[thread overview]
Message-ID: <20110428160115.GA19003@goldbirke> (raw)
In-Reply-To: <20110427064033.GB4226@elie>

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(-)

  parent reply	other threads:[~2011-04-28 16:01 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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       ` SZEDER Gábor [this message]
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

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=20110428160115.GA19003@goldbirke \
    --to=szeder@ira.uka.de \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=lists@haller-berlin.de \
    --cc=lodatom@gmail.com \
    /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).