Subject: Re: [PATCH] rev-list/log: document logic with several limiting options

From: Michael J Gruber <[email-masked]>
Date: Wed, 12 Sep 2012 15:43:13 +0200

Junio C Hamano venit, vidit, dixit 11.09.2012 18:22:
> Michael J Gruber <[email-masked]> writes:
> 
>> The current behavior is probably as useful as it is confusing. In any
>> case it is going to stay. So, document it.
>>
>> Signed-off-by: Michael J Gruber <[email-masked]>
>> ---
>> I would have written a test but don't really know where to stick it in.
>> rev-list has many small tests where it doesn't fit.
> 
> "git log" would be the more natural place, I would say.

Well, to me, "git log" is the ui to "git rev-list", so everything which
tests the revision walker should be tested using "git rev-list". But I
don't care as long as it fits in - and can be found later.

That being said, I now see lots of "log --grep" and "log --author" tests
in the grep-test. So I guess there it fits best (despite the misnomer).

>>
>>  Documentation/rev-list-options.txt | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
>> index 5436eba..9c13df3 100644
>> --- a/Documentation/rev-list-options.txt
>> +++ b/Documentation/rev-list-options.txt
>> @@ -6,6 +6,12 @@ special notations explained in the description, additional commit
>>  limiting may be applied. Note that they are applied before commit
>>  ordering and formatting options, such as '--reverse'.
>>  
>> +All occurrences of the same option are ORed: '--grep=foo --grep=bar'
>> +limits to commits which match at least one of these conditions.
> 
> How would this interact with "--all-match"?

Badly.

It was introduced in 0ab7befa with a clear meaning (AND everything),
then the general logic (without --all-match) was modified in 80235ba7
(to take headermatch AND (all greps ORed)), and 5aaeb733 finally made
multiple authors resp. committers get ORed among each other. All of this
in an attempt to make the standard usage most useful, of course. As a
consequence, --all-match does not influence multiple --author options at
all (contrary to the doc), e.g.

I don't see any of this reflected in the doc, though. I noticed only by
reading t/t7810-grep.sh. Before that, I had only gone by my own testing
which didn't reveal the multiple author multiple special casing effect.

I guess I'll have to wrap my head around the current implementation a
few more times before trying to describe the status quo in the
documentation...