Re: [PATCHv2 6/6] rev-list/log: document logic with several limiting options

Subsystems: documentation, the rest

13 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCHv2 6/6] rev-list/log: document logic with several limiting options

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:54:44 

Junio C Hamano [off-list ref] writes:

Assuming that the patch I posted earlier actually works, I think the
description can become vastly simpler, if you stop explaining author
and committer in terms of "grep".
And here is a replacement in a patch form.  

-- >8 --
Subject: [PATCH] log: document use of multiple commit limiting options

Generally speaking, using more options will further narrow the
selection, but there are a few exceptions.  Document them.

Signed-off-by: Junio C Hamano <redacted>
---
 Documentation/rev-list-options.txt | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 1ae3c89..57d6c90 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -3,7 +3,14 @@ Commit Limiting
 
 Besides specifying a range of commits that should be listed using the
 special notations explained in the description, additional commit
-limiting may be applied. Note that they are applied before commit
+limiting may be applied.
+
+Using more options generally further limits the output (e.g.
+"--since=<date1>" limits to commits newer than <date1>, and using it
+with "--grep=<pattern>" further limits to commits whose log message
+has a line that match <pattern>), unless otherwise noted.
+
+Note that these are applied before commit
 ordering and formatting options, such as '--reverse'.
 
 --
@@ -38,16 +45,22 @@ endif::git-rev-list[]
 --committer=<pattern>::
 
 	Limit the commits output to ones with author/committer
-	header lines that match the specified pattern (regular expression).
+	header lines that match the specified pattern (regular
+	expression).  With more than one `--author=<pattern>`,
+	commits whose author match any of the given patterns are
+	chosen (similarly for multiple `--committer=<pattern>`).
 
 --grep=<pattern>::
 
 	Limit the commits output to ones with log message that
-	matches the specified pattern (regular expression).
+	matches the specified pattern (regular expression).  With
+	more than one `--grep=<pattern>`, commits whose message
+	match any of the given patterns are chosen (but see
+	`--all-match`).
 
 --all-match::
 	Limit the commits output to ones that match all given --grep,
-	--author and --committer instead of ones that match at least one.
+	instead of ones that match at least one.
 
 -i::
 --regexp-ignore-case::
-- 
1.7.12.469.g5235eb6

[PATCHv3 01/11] grep: teach --debug option to dump the parse tree

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:45 

From: Junio C Hamano <redacted>

Our "grep" allows complex boolean expressions to be formed to match
each individual line with operators like --and, '(', ')' and --not.
Introduce the "--debug" option to show the parse tree to help people
who want to debug and enhance it.

Also "log" learns "--debug-grep" option to do the same.  The command
line parser to the log family is a lot more limited than the general
"git grep" parser, but it has special handling for header matching
(e.g. "--author"), and a parse tree is valuable when working on it.

Note that "--all-match" is *not* any individual node in the parse
tree.  It is an instruction to the evaluator to check all the nodes
in the top-level backbone have matched and reject a document as
non-matching otherwise.

Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Michael J Gruber <redacted>
---
 builtin/grep.c |  3 ++
 grep.c         | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 grep.h         |  1 +
 revision.c     |  2 ++
 4 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index fe1726f..8aea00c 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -772,6 +772,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			   "indicate hit with exit status without output"),
 		OPT_BOOLEAN(0, "all-match", &opt.all_match,
 			"show only matches from files that match all patterns"),
+		{ OPTION_SET_INT, 0, "debug", &opt.debug, NULL,
+		  "show parse tree for grep expression",
+		  PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1 },
 		OPT_GROUP(""),
 		{ OPTION_STRING, 'O', "open-files-in-pager", &show_in_pager,
 			"pager", "show matching files in the pager",
diff --git a/grep.c b/grep.c
index 04e3ec6..be15c47 100644
--- a/grep.c
+++ b/grep.c
@@ -332,6 +332,87 @@ static struct grep_expr *compile_pattern_expr(struct grep_pat **list)
 	return compile_pattern_or(list);
 }
 
+static void indent(int in)
+{
+	while (in-- > 0)
+		fputc(' ', stderr);
+}
+
+static void dump_grep_pat(struct grep_pat *p)
+{
+	switch (p->token) {
+	case GREP_AND: fprintf(stderr, "*and*"); break;
+	case GREP_OPEN_PAREN: fprintf(stderr, "*(*"); break;
+	case GREP_CLOSE_PAREN: fprintf(stderr, "*)*"); break;
+	case GREP_NOT: fprintf(stderr, "*not*"); break;
+	case GREP_OR: fprintf(stderr, "*or*"); break;
+
+	case GREP_PATTERN: fprintf(stderr, "pattern"); break;
+	case GREP_PATTERN_HEAD: fprintf(stderr, "pattern_head"); break;
+	case GREP_PATTERN_BODY: fprintf(stderr, "pattern_body"); break;
+	}
+
+	switch (p->token) {
+	default: break;
+	case GREP_PATTERN_HEAD:
+		fprintf(stderr, "<head %d>", p->field); break;
+	case GREP_PATTERN_BODY:
+		fprintf(stderr, "<body>"); break;
+	}
+	switch (p->token) {
+	default: break;
+	case GREP_PATTERN_HEAD:
+	case GREP_PATTERN_BODY:
+	case GREP_PATTERN:
+		fprintf(stderr, "%.*s", (int)p->patternlen, p->pattern);
+		break;
+	}
+	fputc('\n', stderr);
+}
+
+static void dump_grep_expression_1(struct grep_expr *x, int in)
+{
+	indent(in);
+	switch (x->node) {
+	case GREP_NODE_TRUE:
+		fprintf(stderr, "true\n");
+		break;
+	case GREP_NODE_ATOM:
+		dump_grep_pat(x->u.atom);
+		break;
+	case GREP_NODE_NOT:
+		fprintf(stderr, "(not\n");
+		dump_grep_expression_1(x->u.unary, in+1);
+		indent(in);
+		fprintf(stderr, ")\n");
+		break;
+	case GREP_NODE_AND:
+		fprintf(stderr, "(and\n");
+		dump_grep_expression_1(x->u.binary.left, in+1);
+		dump_grep_expression_1(x->u.binary.right, in+1);
+		indent(in);
+		fprintf(stderr, ")\n");
+		break;
+	case GREP_NODE_OR:
+		fprintf(stderr, "(or\n");
+		dump_grep_expression_1(x->u.binary.left, in+1);
+		dump_grep_expression_1(x->u.binary.right, in+1);
+		indent(in);
+		fprintf(stderr, ")\n");
+		break;
+	}
+}
+
+void dump_grep_expression(struct grep_opt *opt)
+{
+	struct grep_expr *x = opt->pattern_expression;
+
+	if (opt->all_match)
+		fprintf(stderr, "[all-match]\n");
+	dump_grep_expression_1(x, 0);
+	fflush(NULL);
+}
+
 static struct grep_expr *grep_true_expr(void)
 {
 	struct grep_expr *z = xcalloc(1, sizeof(*z));
@@ -395,7 +476,7 @@ static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
 	return header_expr;
 }
 
-void compile_grep_patterns(struct grep_opt *opt)
+static void compile_grep_patterns_real(struct grep_opt *opt)
 {
 	struct grep_pat *p;
 	struct grep_expr *header_expr = prep_header_patterns(opt);
@@ -415,7 +496,7 @@ void compile_grep_patterns(struct grep_opt *opt)
 
 	if (opt->all_match || header_expr)
 		opt->extended = 1;
-	else if (!opt->extended)
+	else if (!opt->extended && !opt->debug)
 		return;
 
 	p = opt->pattern_list;
@@ -435,6 +516,13 @@ void compile_grep_patterns(struct grep_opt *opt)
 	opt->all_match = 1;
 }
 
+void compile_grep_patterns(struct grep_opt *opt)
+{
+	compile_grep_patterns_real(opt);
+	if (opt->debug)
+		dump_grep_expression(opt);
+}
+
 static void free_pattern_expr(struct grep_expr *x)
 {
 	switch (x->node) {
diff --git a/grep.h b/grep.h
index ed7de6b..bf5be5a 100644
--- a/grep.h
+++ b/grep.h
@@ -90,6 +90,7 @@ struct grep_opt {
 	int word_regexp;
 	int fixed;
 	int all_match;
+	int debug;
 #define GREP_BINARY_DEFAULT	0
 #define GREP_BINARY_NOMATCH	1
 #define GREP_BINARY_TEXT	2
diff --git a/revision.c b/revision.c
index 9a0d9c7..90376e8 100644
--- a/revision.c
+++ b/revision.c
@@ -1578,6 +1578,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if ((argcount = parse_long_opt("grep", argv, &optarg))) {
 		add_message_grep(revs, optarg);
 		return argcount;
+	} else if (!strcmp(arg, "--grep-debug")) {
+		revs->grep_filter.debug = 1;
 	} else if (!strcmp(arg, "--extended-regexp") || !strcmp(arg, "-E")) {
 		revs->grep_filter.regflags |= REG_EXTENDED;
 	} else if (!strcmp(arg, "--regexp-ignore-case") || !strcmp(arg, "-i")) {
-- 
1.7.12.592.g41e7905

[PATCHv3 00/11] rev-list/log: document logic with several limiting options

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:45 

In v3 I've rebased my tests on top of Junio's commits which change
--all-match handling and provide a better documentation. Accordingly,
6/6 from v2 is dropped and 5/5 adjusted (expect_failure -> expect_success).

I'm sending out Junio's commits as in pu along with 3 intermixed fixups which
you may squash in freely. (I've only marked the more obvious one as fixup!.)
The last 5 are my rebased tests as described above.

[This approach seemed to be more convenient; hopefully for everyone ;)]

Junio C Hamano (3):
  grep: teach --debug option to dump the parse tree
  log --grep/--author: honor --all-match honored for multiple --grep
    patterns
  log: document use of multiple commit limiting options

Michael J Gruber (8):
  log: name --debug-grep option like in the commit message
  grep: show --debug output only once
  fixup! log: document use of multiple commit limiting options
  t7810-grep: bring log --grep tests in common form
  t7810-grep: test multiple --grep with and without --all-match
  t7810-grep: test multiple --author with --all-match
  t7810-grep: test interaction of multiple --grep and --author options
  t7810-grep: test --all-match with multiple --grep and --author
    options

 Documentation/rev-list-options.txt |  23 ++++++--
 builtin/grep.c                     |   4 ++
 grep.c                             | 111 ++++++++++++++++++++++++++++++++++++-
 grep.h                             |   1 +
 revision.c                         |   2 +
 t/t7810-grep.sh                    |  90 +++++++++++++++++++++++++-----
 6 files changed, 210 insertions(+), 21 deletions(-)

-- 
1.7.12.592.g41e7905

[PATCHv3 02/11] log: name --debug-grep option like in the commit message

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:45 

Signed-off-by: Michael J Gruber <redacted>
---
 revision.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/revision.c b/revision.c
index 90376e8..fad8040 100644
--- a/revision.c
+++ b/revision.c
@@ -1578,7 +1578,7 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	} else if ((argcount = parse_long_opt("grep", argv, &optarg))) {
 		add_message_grep(revs, optarg);
 		return argcount;
-	} else if (!strcmp(arg, "--grep-debug")) {
+	} else if (!strcmp(arg, "--debug-grep")) {
 		revs->grep_filter.debug = 1;
 	} else if (!strcmp(arg, "--extended-regexp") || !strcmp(arg, "-E")) {
 		revs->grep_filter.regflags |= REG_EXTENDED;
-- 
1.7.12.592.g41e7905

[PATCHv3 03/11] grep: show --debug output only once

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:45 

When threaded grep is in effect, the patterns are duplicated and
recompiled for each thread. Avoid "--debug" output during the
recompilation so that the output is given once instead of "1+nthreads"
times.

Signed-off-by: Michael J Gruber <redacted>
---
 builtin/grep.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/builtin/grep.c b/builtin/grep.c
index 8aea00c..a7e8df0 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -209,6 +209,7 @@ static void start_threads(struct grep_opt *opt)
 		int err;
 		struct grep_opt *o = grep_opt_dup(opt);
 		o->output = strbuf_out;
+		o->debug = 0;
 		compile_grep_patterns(o);
 		err = pthread_create(&threads[i], NULL, run, o);
 
-- 
1.7.12.592.g41e7905

[PATCHv3 06/11] fixup! log: document use of multiple commit limiting options

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:45 

Here are a few typo fixes.

There is a mix of single and back ticks already before this patch,
i.e. ` vs. ' -- I thought we had guidelines for this but don't find them
at the moment.

Signed-off-by: Michael J Gruber <redacted>
---
 Documentation/rev-list-options.txt | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 57d6c90..c828408 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -6,12 +6,12 @@ special notations explained in the description, additional commit
 limiting may be applied.
 
 Using more options generally further limits the output (e.g.
-"--since=<date1>" limits to commits newer than <date1>, and using it
-with "--grep=<pattern>" further limits to commits whose log message
-has a line that match <pattern>), unless otherwise noted.
+`--since=<date1>` limits to commits newer than `<date1>`, and using it
+with `--grep=<pattern>` further limits to commits whose log message
+has a line that matches `<pattern>`), unless otherwise noted.
 
 Note that these are applied before commit
-ordering and formatting options, such as '--reverse'.
+ordering and formatting options, such as `--reverse`.
 
 --
 
@@ -47,7 +47,7 @@ endif::git-rev-list[]
 	Limit the commits output to ones with author/committer
 	header lines that match the specified pattern (regular
 	expression).  With more than one `--author=<pattern>`,
-	commits whose author match any of the given patterns are
+	commits whose author matches any of the given patterns are
 	chosen (similarly for multiple `--committer=<pattern>`).
 
 --grep=<pattern>::
@@ -55,7 +55,7 @@ endif::git-rev-list[]
 	Limit the commits output to ones with log message that
 	matches the specified pattern (regular expression).  With
 	more than one `--grep=<pattern>`, commits whose message
-	match any of the given patterns are chosen (but see
+	matches any of the given patterns are chosen (but see
 	`--all-match`).
 
 --all-match::
-- 
1.7.12.592.g41e7905

[PATCHv3 08/11] t7810-grep: test multiple --grep with and without --all-match

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:45 

Signed-off-by: Michael J Gruber <redacted>
---
 t/t7810-grep.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 180e998..b841909 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -479,6 +479,22 @@ test_expect_success 'log grep (6)' '
 	test_cmp expect actual
 '
 
+test_expect_success 'log with multiple --grep uses union' '
+	git log --grep=i --grep=r --format=%s >actual &&
+	{
+		echo fourth && echo third && echo initial
+	} >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --all-match with multiple --grep uses intersection' '
+	git log --all-match --grep=i --grep=r --format=%s >actual &&
+	{
+		echo third
+	} >expect &&
+	test_cmp expect actual
+'
+
 test_expect_success 'log --grep --author implicitly uses all-match' '
 	# grep matches initial and second but not third
 	# author matches only initial and third
-- 
1.7.12.592.g41e7905

[PATCHv3 05/11] log: document use of multiple commit limiting options

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:45 

From: Junio C Hamano <redacted>

Generally speaking, using more options will further narrow the
selection, but there are a few exceptions.  Document them.

Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Michael J Gruber <redacted>
---
 Documentation/rev-list-options.txt | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 1ae3c89..57d6c90 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -3,7 +3,14 @@ Commit Limiting
 
 Besides specifying a range of commits that should be listed using the
 special notations explained in the description, additional commit
-limiting may be applied. Note that they are applied before commit
+limiting may be applied.
+
+Using more options generally further limits the output (e.g.
+"--since=<date1>" limits to commits newer than <date1>, and using it
+with "--grep=<pattern>" further limits to commits whose log message
+has a line that match <pattern>), unless otherwise noted.
+
+Note that these are applied before commit
 ordering and formatting options, such as '--reverse'.
 
 --
@@ -38,16 +45,22 @@ endif::git-rev-list[]
 --committer=<pattern>::
 
 	Limit the commits output to ones with author/committer
-	header lines that match the specified pattern (regular expression).
+	header lines that match the specified pattern (regular
+	expression).  With more than one `--author=<pattern>`,
+	commits whose author match any of the given patterns are
+	chosen (similarly for multiple `--committer=<pattern>`).
 
 --grep=<pattern>::
 
 	Limit the commits output to ones with log message that
-	matches the specified pattern (regular expression).
+	matches the specified pattern (regular expression).  With
+	more than one `--grep=<pattern>`, commits whose message
+	match any of the given patterns are chosen (but see
+	`--all-match`).
 
 --all-match::
 	Limit the commits output to ones that match all given --grep,
-	--author and --committer instead of ones that match at least one.
+	instead of ones that match at least one.
 
 -i::
 --regexp-ignore-case::
-- 
1.7.12.592.g41e7905

[PATCHv3 10/11] t7810-grep: test interaction of multiple --grep and --author options

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:45 

There are tests for this interaction already. Restructure slightly and
avoid any claims about --all-match.

Signed-off-by: Michael J Gruber <redacted>
---
 t/t7810-grep.sh | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index be81d96..f6edb4d 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -495,16 +495,6 @@ test_expect_success 'log --all-match with multiple --grep uses intersection' '
 	test_cmp expect actual
 '
 
-test_expect_success 'log --grep --author implicitly uses all-match' '
-	# grep matches initial and second but not third
-	# author matches only initial and third
-	git log --author="A U Thor" --grep=s --grep=l --format=%s >actual &&
-	{
-		echo initial
-	} >expect &&
-	test_cmp expect actual
-'
-
 test_expect_success 'log with multiple --author uses union' '
 	git log --author="Thor" --author="Aster" --format=%s >actual &&
 	{
@@ -521,17 +511,33 @@ test_expect_success 'log --all-match with multiple --author still uses union' '
 	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 &&
+test_expect_success 'log --grep --author uses intersection' '
+	# grep matches only third and fourth
+	# author matches only initial and third
+	git log --author="A U Thor" --grep=r --format=%s >actual &&
 	{
-	    echo third && echo initial
+		echo third
 	} >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_expect_success 'log --grep --grep --author takes union of greps and intersects with author' '
+	# grep matches initial and second but not third
+	# author matches only initial and third
+	git log --author="A U Thor" --grep=s --grep=l --format=%s >actual &&
+	{
+		echo initial
+	} >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --grep --author --author takes union of authors and intersects with grep' '
+	# grep matches only initial and third
+	# author matches all but second
+	git log --author="Thor" --author="Night" --grep=i --format=%s >actual &&
+	{
+	    echo third && echo initial
+	} >expect &&
 	test_cmp expect actual
 '
 
-- 
1.7.12.592.g41e7905

[PATCHv3 11/11] t7810-grep: test --all-match with multiple --grep and --author options

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:45 

--all-match is ignored with multiple author options on purpose but
requires all --grep to be matched on some line.

Signed-off-by: Michael J Gruber <redacted>
---
 t/t7810-grep.sh | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index f6edb4d..b5c488e 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -531,6 +531,16 @@ test_expect_success 'log --grep --grep --author takes union of greps and interse
 	test_cmp expect actual
 '
 
+test_expect_success 'log ---all-match -grep --author --author still takes union of authors and intersects with grep' '
+	# grep matches only initial and third
+	# author matches all but second
+	git log --all-match --author="Thor" --author="Night" --grep=i --format=%s >actual &&
+	{
+	    echo third && echo initial
+	} >expect &&
+	test_cmp expect actual
+'
+
 test_expect_success 'log --grep --author --author takes union of authors and intersects with grep' '
 	# grep matches only initial and third
 	# author matches all but second
@@ -541,6 +551,16 @@ test_expect_success 'log --grep --author --author takes union of authors and int
 	test_cmp expect actual
 '
 
+test_expect_success 'log --all-match --grep --grep --author takes intersection' '
+	# grep matches only third
+	# author matches only initial and third
+	git log --all-match --author="A U Thor" --grep=i --grep=r --format=%s >actual &&
+	{
+		echo third
+	} >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.12.592.g41e7905

[PATCHv3 09/11] t7810-grep: test multiple --author with --all-match

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:45 

--all-match is ignored for author matching on purpose.

Signed-off-by: Michael J Gruber <redacted>
---
 t/t7810-grep.sh | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index b841909..be81d96 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -513,6 +513,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 &&
 	{
-- 
1.7.12.592.g41e7905

[PATCHv3 07/11] t7810-grep: bring log --grep tests in common form

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:45 

The log --grep tests generate the expected out in different ways.
Make them all use command blocks so that subshells are avoided and the
expected output is easier to grasp visually.

Signed-off-by: Michael J Gruber <redacted>
---
 t/t7810-grep.sh | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 24e9b19..180e998 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -435,31 +435,41 @@ test_expect_success 'log grep setup' '
 
 test_expect_success 'log grep (1)' '
 	git log --author=author --pretty=tformat:%s >actual &&
-	( echo third ; echo initial ) >expect &&
+	{
+		echo third && echo initial
+	} >expect &&
 	test_cmp expect actual
 '
 
 test_expect_success 'log grep (2)' '
 	git log --author=" * " -F --pretty=tformat:%s >actual &&
-	( echo second ) >expect &&
+	{
+		echo second
+	} >expect &&
 	test_cmp expect actual
 '
 
 test_expect_success 'log grep (3)' '
 	git log --author="^A U" --pretty=tformat:%s >actual &&
-	( echo third ; echo initial ) >expect &&
+	{
+		echo third && echo initial
+	} >expect &&
 	test_cmp expect actual
 '
 
 test_expect_success 'log grep (4)' '
 	git log --author="frotz\.com>$" --pretty=tformat:%s >actual &&
-	( echo second ) >expect &&
+	{
+		echo second
+	} >expect &&
 	test_cmp expect actual
 '
 
 test_expect_success 'log grep (5)' '
 	git log --author=Thor -F --pretty=tformat:%s >actual &&
-	( echo third ; echo initial ) >expect &&
+	{
+		echo third && echo initial
+	} >expect &&
 	test_cmp expect actual
 '
 
@@ -473,7 +483,9 @@ test_expect_success 'log --grep --author implicitly uses all-match' '
 	# grep matches initial and second but not third
 	# author matches only initial and third
 	git log --author="A U Thor" --grep=s --grep=l --format=%s >actual &&
-	echo initial >expect &&
+	{
+		echo initial
+	} >expect &&
 	test_cmp expect actual
 '
 
-- 
1.7.12.592.g41e7905

[PATCHv3 04/11] log --grep/--author: honor --all-match honored for multiple --grep patterns

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:54:45 

From: Junio C Hamano <redacted>

Earlier, when we have both header expression (which has to be an OR
node by construction) and a pattern expression (which could be
anything), we created a top-level OR node that looks like this to
bind them together:

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

In other words, the three elements on the top-level backbone that
were inspected by the "all-match" logic are "pattern", "committer"
and "author".  When there are more than one elements in the
"pattern", the top-level node of it is that OR, so that node is
inspected by "all-match", hence the result ends up ignoring the
"--all-match" given from the command line.  A match on either side
of the pattern was considered a match, hence

        git log --grep=A --grep=B --author=C --all-match

showed the same "authored by C and has either A or B" with or
without --all-match.

This patch turns the grep expression into this form:

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

when "--all-match" was given from the command line.  This way, the
set of nodes on the top-level backbone in the resulting expression
consists of "committer", "author", and whatever nodes were on the
top-level backbone of the "pattern" expression.  The "all-match"
logic inspects the same nodes in "pattern" as the case without the
author and/or the committer restriction.

Signed-off-by: Junio C Hamano <redacted>
Signed-off-by: Michael J Gruber <redacted>
---
 grep.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/grep.c b/grep.c
index be15c47..925aa92 100644
--- a/grep.c
+++ b/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);
-- 
1.7.12.592.g41e7905
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help