Re: [PATCH] git-completion.bash: update obsolete code.

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 18, 2012, 01:37 UTC
Message-ID: <7v7gogjg2q.fsf@alter.siamese.dyndns.org>
In-Reply-To: <50CF4E05.7050103@gmail.com>

Manlio Perillo <manlio.perillo@gmail.com> writes:

Show 17 quoted lines

> Il 17/12/2012 05:54, Junio C Hamano ha scritto:
>> Manlio Perillo <manlio.perillo@gmail.com> writes:
>> 
>>> The git-completion.bash script was using the git ls-tree command
>>> without the --name-only option, with a sed filter to parse path names;
>>> use the --name-only option, instead.
>>>
>>> Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
>>> ---
>> 
>> Did you miss the different handling between blobs and trees the
>> latter gets trailing slash in the completion)?
>> 
>
> Yes, I totally missed it, sorry.
> I was assuming the bash completion script was written before --name-only
> option was added to ls-tree.

There is no need to say "sorry" here; catching this kind of mistake is what we have the patch review process for, after all. If anything, "thanks" is more appropriate ;-)

And the way you stated the reasoning behind this change in the proposed commit log message was really good. It showed clearly that the patch was an attempt to clean up the code, not was an attempt to change the behaviour to drop the trailing slash or space. If it said "let's use ls-tree --name-only" without such an explanation on "why", future readers of "git log" would have been left scratching their heads, wondering "why" the change was made.

Thanks.