Re: [PATCH v7 1/2] grep: refactor next_match() and match_one_pattern() for external use

From: Hamza Mahfooz <someguy@effective-light.com>
Date: Sep 24, 2021, 12:04 UTC
Message-ID: <HJTXZQ.ONLOCDR1B7O51@effective-light.com>
In-Reply-To: <xmqq8rzn2ohj.fsf@gitster.g>

On Thu, Sep 23 2021 at 10:25:28 AM -0700, Junio C Hamano <gitster@pobox.com> wrote:

Show 15 quoted lines

> But how would the new caller that points "bol" at middle of a line
> make sure that we are looking at the right kind of header?  If the
> pattern p is set to match only for an author line, the first call
> with "bol" set to the true beginning of the line will correctly
> reject a "committer" header, but because you lost the sanity check
> above, the second and subsequent one will go ahead and scan for the
> pattern p on the line, even if p->field asks for author line and the
> line records the committer.  You'd end up finding a commit object
> that is committed by (but not authored by) the person when you are
> looking for a commit that was authored by somebody, no?
> 
> If you ask for commits by somebody (e.g. "--author=Hazma") with an
> output format that shows both the author and the committer
> (e.g. "log --pretty=fuller"), wouldn't your "hit coloring" code
> show Hazma on the committer name as a grep hit, too?

Actually, this issue doesn't arise because I filter away the irrelevant (header) patterns in grep_next_match(). However, maybe it's a better idea to handle that in match_one_pattern().