git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/2] log --author: take union of multiple "author" requests

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 13, 2010, 08:18 UTC
Message-ID
<7vmxrmawg0.fsf_-_@alter.siamese.dyndns.org>
In-Reply-To
<7veidlkxdb.fsf@alter.siamese.dyndns.org>
In the olden days,
    log --author=me --committer=him --grep=this --grep=that
used to be turned into:
    (OR (HEADER-AUTHOR me)
        (HEADER-COMMITTER him)
        (PATTERN this)
        (PATTERN that))

showing my patches that do not have any "this" nor "that", which was totally useless.

80235ba ("log --author=me --grep=it" should find intersection, not union, 2010-01-17) improved it greatly to turn the same into:

    (ALL-MATCH
      (HEADER-AUTHOR me)
      (HEADER-COMMITTER him)
      (OR (PATTERN this) (PATTERN that)))

That is, "show only patches by me and committed by him, that have either this or that", which is a lot more natural thing to ask.

We however need to be a bit more clever when the user asks more than one "author" (or "committer"); because a commit has only one author (and one committer), they ought to be interpreted as asking for union to be useful. The current implementation simply added another author/committer pattern at the same top-level for ALL-MATCH to insist on matching all, finding nothing.

Turn
    log --author=me --author=her \
    	--committer=him --committer=you \
	--grep=this --grep=that
into
    (ALL-MATCH
      (OR (HEADER-AUTHOR me) (HEADER-AUTHOR her))
      (OR (HEADER-COMMITTER him) (HEADER-COMMITTER you))
      (OR (PATTERN this) (PATTERN that)))
instead.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 grep.c          |   65 ++++++++++++++++++++++++++++++++++++++++++++----------
 grep.h          |    2 +
 t/t7810-grep.sh |   29 +++++++++++++++++++++++-
 3 files changed, 83 insertions(+), 13 deletions(-)
diff --git a/grep.c b/grep.c
index 718a3c2..63c4280 100644
--- a/grep.c
+++ b/grep.c
@@ -189,17 +189,32 @@ static struct grep_expr *compile_pattern_expr(struct grep_pat **list)
 	return compile_pattern_or(list);
 }
 
+static struct grep_expr *grep_true_expr(void)
+{
+	struct grep_expr *z = xcalloc(1, sizeof(*z));
+	z->node = GREP_NODE_TRUE;
+	return z;
+}
+
+static struct grep_expr *grep_or_expr(struct grep_expr *left, struct grep_expr *right)
+{
+	struct grep_expr *z = xcalloc(1, sizeof(*z));
+	z->node = GREP_NODE_OR;
+	z->u.binary.left = left;
+	z->u.binary.right = right;
+	return z;
+}
+
 static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
 {
 	struct grep_pat *p;
 	struct grep_expr *header_expr;
+	struct grep_expr *(header_group[GREP_HEADER_FIELD_MAX]);
+	enum grep_header_field fld;
 
 	if (!opt->header_list)
 		return NULL;
 	p = opt->header_list;
-	header_expr = compile_pattern_expr(&p);
-	if (p)
-		die("incomplete pattern expression: %s", p->pattern);
 	for (p = opt->header_list; p; p = p->next) {
 		if (p->token != GREP_PATTERN_HEAD)
 			die("bug: a non-header pattern in grep header list.");
@@ -207,6 +222,33 @@ static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
 			die("bug: unknown header field %d", p->field);
 		compile_regexp(p, opt);
 	}
+
+	for (fld = 0; fld < GREP_HEADER_FIELD_MAX; fld++)
+		header_group[fld] = NULL;
+
+	for (p = opt->header_list; p; p = p->next) {
+		struct grep_expr *h;
+		struct grep_pat *pp = p;
+
+		h = compile_pattern_atom(&pp);
+		if (!h || pp != p->next)
+			die("bug: malformed header expr");
+		if (!header_group[p->field]) {
+			header_group[p->field] = h;
+			continue;
+		}
+		header_group[p->field] = grep_or_expr(h, header_group[p->field]);
+	}
+
+	header_expr = NULL;
+
+	for (fld = 0; fld < GREP_HEADER_FIELD_MAX; fld++) {
+		if (!header_group[fld])
+			continue;
+		if (!header_expr)
+			header_expr = grep_true_expr();
+		header_expr = grep_or_expr(header_group[fld], header_expr);
+	}
 	return header_expr;
 }
 
@@ -242,22 +284,18 @@ void compile_grep_patterns(struct grep_opt *opt)
 	if (!header_expr)
 		return;
 
-	if (opt->pattern_expression) {
-		struct grep_expr *z;
-		z = xcalloc(1, sizeof(*z));
-		z->node = GREP_NODE_OR;
-		z->u.binary.left = opt->pattern_expression;
-		z->u.binary.right = header_expr;
-		opt->pattern_expression = z;
-	} else {
+	if (!opt->pattern_expression)
 		opt->pattern_expression = header_expr;
-	}
+	else
+		opt->pattern_expression = grep_or_expr(opt->pattern_expression,
+						       header_expr);
 	opt->all_match = 1;
 }
 
 static void free_pattern_expr(struct grep_expr *x)
 {
 	switch (x->node) {
+	case GREP_NODE_TRUE:
 	case GREP_NODE_ATOM:
 		break;
 	case GREP_NODE_NOT:
@@ -486,6 +524,9 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,
 	if (!x)
 		die("Not a valid grep expression");
 	switch (x->node) {
+	case GREP_NODE_TRUE:
+		h = 1;
+		break;
 	case GREP_NODE_ATOM:
 		h = match_one_pattern(x->u.atom, bol, eol, ctx, &match, 0);
 		break;
diff --git a/grep.h b/grep.h
index efa8cff..06621fe 100644
--- a/grep.h
+++ b/grep.h
@@ -22,6 +22,7 @@ enum grep_header_field {
 	GREP_HEADER_AUTHOR = 0,
 	GREP_HEADER_COMMITTER
 };
+#define GREP_HEADER_FIELD_MAX (GREP_HEADER_COMMITTER + 1)
 
 struct grep_pat {
 	struct grep_pat *next;
@@ -41,6 +42,7 @@ enum grep_expr_node {
 	GREP_NODE_ATOM,
 	GREP_NODE_NOT,
 	GREP_NODE_AND,
+	GREP_NODE_TRUE,
 	GREP_NODE_OR
 };
 
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 8a63227..dc5c085 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -324,8 +324,13 @@ test_expect_success 'log grep setup' '
 
 	echo a >>file &&
 	test_tick &&
-	git commit -a -m "third"
+	git commit -a -m "third" &&
 
+	echo a >>file &&
+	test_tick &&
+	GIT_AUTHOR_NAME="Night Fall" \
+	GIT_AUTHOR_EMAIL="nitfol@frobozz.com" \
+	git commit -a -m "fourth"
 '
 
 test_expect_success 'log grep (1)' '
@@ -372,6 +377,28 @@ test_expect_success 'log --grep --author implicitly uses all-match' '
 	test_cmp expect actual
 '
 
+test_expect_success 'log with multiple --author uses union' '
+	git log --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 &&
+	{
+	    echo third && 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=q --format=%s >actual &&
+	>expect &&
+	test_cmp expect actual
+'
+
 test_expect_success 'grep with CE_VALID file' '
 	git update-index --assume-unchanged t/t &&
 	rm t/t &&

-- 
1.7.3.rc1.227.gee5c7b
Previous: Junio C HamanoNext: Emil Sit
Message 4 of 6 in “Regression in git log with multiple authors”
  1. Emil SitAug 26, 2010
  2. Junio C HamanoAug 26, 2010
  3. 1/2 grep: move logic to compile header pattern into a separate helperJunio C Hamano, Sep 13, 2010
  4. 2/2 log --author: take union of multiple "author" requestsJunio C Hamano, Sep 13, 2010
  5. Emil SitSep 13, 2010
  6. Junio C HamanoSep 13, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.