Re: [PATCH] completion: simplify __git_complete_revlist_file

From: SZEDER Gábor <szeder@ira.uka.de>
Date: May 5, 2012, 22:40 UTC
Message-ID: <20120505224014.GH2164@goldbirke>
In-Reply-To: <CAMP44s0z5ghcDQOeL1TrJJ7nw4=tN5MhACqcmf4EjxsNNWiQnA@mail.gmail.com>

Hi,

On Wed, May 02, 2012 at 01:46:32AM +0200, Felipe Contreras wrote:

Show 5 quoted lines

> On Wed, May 2, 2012 at 1:39 AM, Junio C Hamano <gitster@pobox.com> wrote:
> > Felipe Contreras <felipe.contreras@gmail.com> writes:
> >
> >> Use new __gitcomp_nl; this is the last place that uses COMPREPLY and
> >> compgen directly.

I didn't dare to touch that code back then while doing the __gitcomp_nl() changes; that sed command looked way too scary ;) But all it does is to process the output of 'git ls-tree' and removes the cruft and appends a space to filenames and a '/' to directories (without a trailing space). So invoking __gitcomp_nl() with an empty suffix parameter is OK, as it doesn't change the current behavior.

Show 20 quoted lines

> >> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> >> ---
> >>  contrib/completion/git-completion.bash |    6 ++----
> >>  1 file changed, 2 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> >> index 31f714d..d877a23 100755
> >
> > This seems to be based on 'maint', which has 31f714d, but I see two hits
> > with "git grep 'COMPREPLY.*compgen' contrib/completion" there, so I am not
> > sure if that is where you wanted it to apply.
> 
> The last place that is not the __gitcomp* functions, of course.
> 
> > I see this was not addressed to me (neither was the other zsh one), so I
> > take it that the patch is still for discussion, at least for the purpose
> > of today's integration round.  Ping me when it is ready for either 'pu' or
> > 'next'.
> 
> I'd say at least one person should say LGTM before asking you to integrate it.

Yeah, it's good to go.

Best, Gábor