Re: [PATCHv2 3/6] t7810-grep: test multiple --author with --all-match

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 13, 2012, 23:26 UTC
Message-ID: <7vwqzx4i9i.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7vmx0t7iq0.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 26 quoted lines

> 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.

And this is an attempt to do exactly that. Earlier, when we have both header expression (which by the way has to be an OR node by construction) and pattern expression (which could be anything), we created a top-level OR node (again, look at this as if you are reading LISP),

           OR
        /        \
       /          \
   pattern            OR
     / \           /     \
    .....    committer    OR
                         /   \ 
                     author   TRUE

in other words, the three elements on the top-level backbone are "pattern", "committer" and "author"; when there are more than one elements in the "pattern", the top-level node of it is OR, so that node is inspected by "all-match", hence the result ends up ignoring the "--all-match" given from the command line.

This patch turns it into

	     OR
          /      \
         /         \
        /              OR
    committer        /    \
                 author    \
                           pattern

when "--all-match" was given from the command line, so that the "committer", "author" and elements on the top-level backbone in "pattern" form the top-level backbone of the resulting expression to be inspected by the "all-match" logic.

Does this pass the expect-failure test in your [PATCH 5/6]?

 grep.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git c/grep.c w/grep.c
index be15c47..925aa92 100644
--- c/grep.c
+++ w/grep.c
@@ -476,6 +476,22 @@ static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
 	return header_expr;
 }
 
+static struct grep_expr *grep_splice_or(struct grep_expr *x, struct grep_expr *y)
+{
+	struct grep_expr *z = x;
+
+	while (x) {
+		assert(x->node == GREP_NODE_OR);
+		if (x->u.binary.right &&
+		    x->u.binary.right->node == GREP_NODE_TRUE) {
+			x->u.binary.right = y;
+			break;
+		}
+		x = x->u.binary.right;
+	}
+	return z;
+}
+
 static void compile_grep_patterns_real(struct grep_opt *opt)
 {
 	struct grep_pat *p;
@@ -510,6 +526,9 @@ static void compile_grep_patterns_real(struct grep_opt *opt)
 
 	if (!opt->pattern_expression)
 		opt->pattern_expression = header_expr;
+	else if (opt->all_match)
+		opt->pattern_expression = grep_splice_or(header_expr,
+							 opt->pattern_expression);
 	else
 		opt->pattern_expression = grep_or_expr(opt->pattern_expression,
 						       header_expr);