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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jeff King <peff@xxxxxxxx> 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.

-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]