Re: [PATCH 3/3] revision traversal: --author, --committer, and --grep.

From: Junio C Hamano <junkio@cox.net>
Date: Sep 18, 2006, 06:51 UTC
Message-ID: <7vslipsm4x.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <20060918060552.GA2833@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

>The important difference is that your approach means that the user's
>regex is implicitly anchored at the beginning of the field. Thus,
>searching by email address does not work with --author=junkio, but
>rather requires --author='.*junkio'.

I wanted to default it to left anchored, so this was somewhat deliberate, but this is probably subject to taste.

I actually wanted to do only --grep-header and --grep-message internally with --author and --committer as simple shorthands, and that is why --author=<pattern> is just an internal synonym for "author <pattern>". I even considered doing without distinction between header and body (meaning, a line in the commit log message that happens to begin with "author " would match --author string), which is technically incorrect but would be more efficient. I do not think occasional false hits would matter much in practice.

Not making the match case insensitive was probably a mistake. Looking for "--author=linus" would be easier to type.

One thing that I know is broken is the match at the tail; I am actually thinking of doing something like this:

	sprintf(pat, "^%s %s [0-9][0-9]* [+-][0-9][0-9][0-9][0-9]$",
		field, pattern);

to avoid matching the timestamps.

> A few other thoughts:
>   1. Case sensitivity?

I do not think much is lost if we make the match always case insensitive for this application. It might be reasonable to to default to case insensitive match but if there is a pattern that has an uppercase use case sensitive match.

>  2. Is there any use to exposing the "header_grep" functionality with
>     --grep-header?

I considered it but did not think of any. We obviously would need to revisit this if we do "note " header in the future, but not for now. On the other hand, it might be useful to allow

	--grep-header='^\(author\|committer\) Junio'

>   3. An alias (--who=foo?) for --author=foo --committer=foo. I believe
>      this doesn't require boolean magic, since we default to OR.

This should be trivial to implement (two calls to add_header_grep instead of one), but would it be useful? I dunno.