From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 13 Sep 2012 20:47:51 GMT
Subject: Re: [PATCHv2 3/6] t7810-grep: test multiple --author with --all-match
Message-ID: <7vmx0t7iq0.fsf@alter.siamese.dyndns.org>
In-Reply-To: <8fd93bb87098298677426735dd354fa4f64abc17.1347544259.git.git@drmicha.warpmail.net>

Michael J Gruber <git@drmicha.warpmail.net> writes:

> --all-match is ignored for author matching on purpose.
>
> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
> ---

What the added test expects is correct, but I do not think the above
description is correct.  "all-match" is implicitly turned on when
you use header match.

When you say

	git log --grep=Linus --grep=Junio

you will get

    (or
     pattern_body<body>Junio
     pattern_body<body>Linus
    )

but when you say

	git log --author=Linus --author=Junio

you will get

    [all-match]
    (or
     (or
      pattern_head<head 0>Linus
      pattern_head<head 0>Junio
     )
     true
    )

instead.  Notice that there is one extra level of "OR" node, so that
two OR nodes on the top-level backbone (think of these as cons cells
with car and cdr) are "author is either Linus or junio" and "True".
Because "all-match" is about rejecting a document as non-matching
unless all the OR nodes on the top-level backbone have fired, this
allows commit that is authored either by Linus or by Junio to match,
and "on purpose" part in your message is correct.

But

	git log --author=Linus --author=Junio --grep=commit

will be parsed to

    [all-match]
    (or
     pattern_body<body>commit
     (or
      (or
       pattern_head<head 0>Linus
       pattern_head<head 0>Junio
      )
      true
     )
    )

to have three OR nodes on the backbone: "the log message must have commit",
"authored by either Linus or Junio", and "true".  All three must
match somewhere in the "git cat-file commit" output for the commit
to be considered a match (but obviously they do not have to match on
the same line).

So what is giving commits made by Linus, even though it is not
authored by Junio, with "--author=Linus --author=Junio" is not the
lack of --all-match.  In fact, --all-match is implicitly set for
other things, so that the last example finds commits that mention
"commit" authored by one of these two people.  Commits that do
mention "commit" but are done by other people are excluded.  Commits
that do not mention "commit" are excluded even if they were done by
Linus or Junio.

	git log --committer=Linus --author=Junio

turns into

    [all-match]
    (or
     pattern_head<head 1>Linus
     (or
      pattern_head<head 0>Junio
      true
     )
    )

which has "committed by Linus", "authored by Junio" on the top-level
backbone, so both has to be true for a commit to match.

Adding --grep=commit makes it

    [all-match]
    (or
     pattern_body<body>commit
     (or
      pattern_head<head 1>Linus
      (or
       pattern_head<head 0>Junio
       true
      )
     )
    )

which has "committed by Linus", "authored by Junio", "mentions
commit" on the top-level, and all three has to be true.

	git log --committer=Linus --author=Junio --grep=commit --grep=tag

groups the "mentions commit" and "mentions tag" under a new
top-level OR node, i.e.

    [all-match]
    (or
     (or
      pattern_body<body>commit
      pattern_body<body>tag
     )
     (or
      pattern_head<head 1>Linus
      (or
       pattern_head<head 0>Junio
       true
      )
     )
    )

so the top-level backbone "all-match" works on becomes

 - Mentions either commit or tag,
 - Committed by Linus,
 - Authored by Junio

One possible improvement we can make is to parse the command line in
the last example with "--all-match" to

    [all-match]
    (or
     pattern_body<body>commit
     (or
      pattern_body<body>tag
      (or
       pattern_head<head 1>Linus
       (or
        pattern_head<head 0>Junio
        true
       )
      )
     )
    )

so that the backbone becomes

 - Mentions commit,
 - Mentions tag,
 - Committed by Linus,
 - Authored by Junio

to require that both commit and tag are mentioned in the message.

>  t/t7810-grep.sh | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
> index 1db3dcb..9bc63a3 100755
> --- a/t/t7810-grep.sh
> +++ b/t/t7810-grep.sh
> @@ -580,6 +580,14 @@ test_expect_success 'log with multiple --author uses union' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'log --all-match with multiple --author still uses union' '
> +	git log --all-match --author="Thor" --author="Aster" --format=%s >actual &&
> +	{
> +	    echo third && echo second && echo initial
> +	} >expect &&
> +	test_cmp expect actual
> +'
> +
>  test_expect_success 'log with --grep and multiple --author uses all-match' '
>  	git log --author="Thor" --author="Night" --grep=i --format=%s >actual &&
>  	{