Re: [PATCH] fix "git log -i --grep"

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 25, 2008, 05:12 UTC
Message-ID: <7vmyj1isot.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080825013837.GA17201@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 10 quoted lines

> On Fri, Aug 22, 2008 at 02:09:35PM -0700, Junio C Hamano wrote:
>
>>     [alias]
>>         who = "!sh -c 'git log -1 --pretty=\"format:%an <%ae>\" --author=\"$1\"' -"
>
> I have two improvements for this, and one of them caused me to find a
> git bug, for which the fix is below. :)
>
>   1. I tried this with --no-pager, which made it obvious that this
>      should be using --pretty=tformat to append a newline.

Strict reading of POSIX suggests that you are not supposed to send an input that has incomplete line to "sed", so tformat may be the right thing to use for that reason as well.

However.

My sed is non POSIX in a good sense and does not have problem handing such an input, and my use case is to say "\C-u \M-! git who Jeff <ENTER>" while typing e-mail message, and I do _not_ want an extra newline after the input. That is why I use format: (not tformat:) there.

Show 6 quoted lines

> The fix is to allocate the grep_filter member whenever we
> get _any_ grep information, be it actual filters or just
> flags. Thus checking for non-NULL revs->grep_filter is no
> longer sufficient to know that we have patterns; in
> commit_match we must actually check that the pattern list is
> not empty.

Well spotted, and thanks for the fix.

As you suggested, making the grep option structure embedded in rev_info may not be a bad idea. We used to keep track of the sub-options separately while we encounter, and updated grep_filter at the end of the loop, but the conversion to use parse-options broke it.

The only issue I still have, which I suspect your fix has made it easier to address, is to complain if sub-options to grep like -i and -E are given without --grep. That's not something v1.5.6 series did, though.