"log --author=me --grep=it" should find intersection, not union

Historically, any grep filter in "git log" family of commands were taken
as restricting to commits with any of the words in the commit log message.
However, the user almost always want to find commits "done by this person
on that topic".  With "--all-match" option, a series of grep patterns can
be turned into a requirement that all of them must produce a match, but
that makes it impossible to ask for "done by me, on either this or that"
with:

log --author=me --committer=him --grep=this --grep=that

because it will require both "this" and "that" to appear.

Change the "header" parser of grep library to treat the headers specially,
and parse it as:

(all-match-OR (HEADER-AUTHOR me)
(HEADER-COMMITTER him)
(OR
(PATTERN this)
(PATTERN that) ) )

Even though the "log" command line parser doesn't give direct access to
the extended grep syntax to group terms with parentheses, this change will
cover the majority of the case the users would want.

This incidentally revealed that one test in t7002 was bogus.  It ran:

log --author=Thor --grep=Thu --format='%s'

and expected (wrongly) "Thu" to match "Thursday" in the author/committer
date, but that would never match, as the timestamp in raw commit buffer
does not have the name of the day-of-the-week.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-grep.c b/builtin-grep.c
index 529461fb8fdf6bdbee86b8cc46fa610696cb9382..d57c4d9bed1b3c4f64bbafa8cf0a8d9bb0040bb5 100644 (file)
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -820,6 +820,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
        opt.relative = 1;
        opt.pathname = 1;
        opt.pattern_tail = &opt.pattern_list;
        opt.relative = 1;
        opt.pathname = 1;
        opt.pattern_tail = &opt.pattern_list;

+       opt.header_tail = &opt.header_list;

        opt.regflags = REG_NEWLINE;
        opt.max_depth = -1;
 
        opt.regflags = REG_NEWLINE;
        opt.max_depth = -1;
 

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index cd97ded4d249b3beb5179150a283e8be43b9ba44..c53ad9fa08a24ffcf89fd91b2a22ef36cbd7f2e4 100644 (file)
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -371,8 +371,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
            revs.diff)
                usage(rev_list_usage);
 
            revs.diff)
                usage(rev_list_usage);
 

-       save_commit_buffer = revs.verbose_header ||
-               revs.grep_filter.pattern_list;
+       save_commit_buffer = (revs.verbose_header ||
+                             revs.grep_filter.pattern_list ||
+                             revs.grep_filter.header_list);

        if (bisect_list)
                revs.limited = 1;
 
        if (bisect_list)
                revs.limited = 1;
 

diff --git a/grep.c b/grep.c
index bdadf2c0ccfafc18011beee3bc05754860392523..eee99f31f1ac65d55b09fbeae2ecf7a4a99843bd 100644 (file)
--- a/grep.c
+++ b/grep.c
@@ -11,8 +11,8 @@ void append_header_grep_pattern(struct grep_opt *opt, enum grep_header_field fie
        p->no = 0;
        p->token = GREP_PATTERN_HEAD;
        p->field = field;
        p->no = 0;
        p->token = GREP_PATTERN_HEAD;
        p->field = field;

-       *opt->pattern_tail = p;
-       opt->pattern_tail = &p->next;
+       *opt->header_tail = p;
+       opt->header_tail = &p->next;

        p->next = NULL;
 }
 
        p->next = NULL;
 }
 


@@ -172,9 +172,26 @@ static struct grep_expr *compile_pattern_expr(struct grep_pat **list)
 void compile_grep_patterns(struct grep_opt *opt)
 {
        struct grep_pat *p;
 void compile_grep_patterns(struct grep_opt *opt)
 {
        struct grep_pat *p;

-
-       if (opt->all_match)
-               opt->extended = 1;
+       struct grep_expr *header_expr = NULL;
+
+       if (opt->header_list) {
+               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) {
+                       switch (p->token) {
+                       case GREP_PATTERN: /* atom */
+                       case GREP_PATTERN_HEAD:
+                       case GREP_PATTERN_BODY:
+                               compile_regexp(p, opt);
+                               break;
+                       default:
+                               opt->extended = 1;
+                               break;
+                       }
+               }
+       }

 
        for (p = opt->pattern_list; p; p = p->next) {
                switch (p->token) {
 
        for (p = opt->pattern_list; p; p = p->next) {
                switch (p->token) {


@@ -189,7 +206,9 @@ void compile_grep_patterns(struct grep_opt *opt)
                }
        }
 
                }
        }
 

-       if (!opt->extended)
+       if (opt->all_match || header_expr)
+               opt->extended = 1;
+       else if (!opt->extended)

                return;
 
        /* Then bundle them up in an expression.
                return;
 
        /* Then bundle them up in an expression.


@@ -200,6 +219,21 @@ void compile_grep_patterns(struct grep_opt *opt)
                opt->pattern_expression = compile_pattern_expr(&p);
        if (p)
                die("incomplete pattern expression: %s", p->pattern);
                opt->pattern_expression = compile_pattern_expr(&p);
        if (p)
                die("incomplete pattern expression: %s", p->pattern);

+
+       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 {
+               opt->pattern_expression = header_expr;
+       }
+       opt->all_match = 1;

 }
 
 static void free_pattern_expr(struct grep_expr *x)
 }
 
 static void free_pattern_expr(struct grep_expr *x)

diff --git a/grep.h b/grep.h
index 75370f60d7091becd3eaddf3b7069c26fca8c125..e39e5146d8701465f1d95a195de4183f28659f93 100644 (file)
--- a/grep.h
+++ b/grep.h
@@ -59,6 +59,8 @@ struct grep_expr {
 struct grep_opt {
        struct grep_pat *pattern_list;
        struct grep_pat **pattern_tail;
 struct grep_opt {
        struct grep_pat *pattern_list;
        struct grep_pat **pattern_tail;

+       struct grep_pat *header_list;
+       struct grep_pat **header_tail;

        struct grep_expr *pattern_expression;
        const char *prefix;
        int prefix_length;
        struct grep_expr *pattern_expression;
        const char *prefix;
        int prefix_length;

diff --git a/revision.c b/revision.c
index 25fa14d93e04531e8d1dde3d9ac8408670adf32a..c84243d7bb5c1c4c718556f840f3acc152700274 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -806,6 +806,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 
        revs->grep_filter.status_only = 1;
        revs->grep_filter.pattern_tail = &(revs->grep_filter.pattern_list);
 
        revs->grep_filter.status_only = 1;
        revs->grep_filter.pattern_tail = &(revs->grep_filter.pattern_list);

+       revs->grep_filter.header_tail = &(revs->grep_filter.header_list);

        revs->grep_filter.regflags = REG_NEWLINE;
 
        diff_setup(&revs->diffopt);
        revs->grep_filter.regflags = REG_NEWLINE;
 
        diff_setup(&revs->diffopt);


@@ -1751,7 +1752,7 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
 
 static int commit_match(struct commit *commit, struct rev_info *opt)
 {
 
 static int commit_match(struct commit *commit, struct rev_info *opt)
 {

-       if (!opt->grep_filter.pattern_list)
+       if (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)

                return 1;
        return grep_buffer(&opt->grep_filter,
                           NULL, /* we say nothing, not even filename */
                return 1;
        return grep_buffer(&opt->grep_filter,
                           NULL, /* we say nothing, not even filename */

diff --git a/t/t7002-grep.sh b/t/t7002-grep.sh
index 76c5e091b7d45bde38f34dc9726c51034ee7f9eb..6baa4461f704a39807cee861071f075f09da03da 100755 (executable)
--- a/t/t7002-grep.sh
+++ b/t/t7002-grep.sh
@@ -357,7 +357,7 @@ test_expect_success 'log grep (4)' '
 '
 
 test_expect_success 'log grep (5)' '
 '
 
 test_expect_success 'log grep (5)' '

-       git log --author=Thor -F --grep=Thu --pretty=tformat:%s >actual &&
+       git log --author=Thor -F --pretty=tformat:%s >actual &&

        ( echo third ; echo initial ) >expect &&
        test_cmp expect actual
 '
        ( echo third ; echo initial ) >expect &&
        test_cmp expect actual
 '


@@ -368,6 +368,14 @@ test_expect_success 'log grep (6)' '
        test_cmp expect actual
 '
 
        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 'grep with CE_VALID file' '
        git update-index --assume-unchanged t/t &&
        rm t/t &&
 test_expect_success 'grep with CE_VALID file' '
        git update-index --assume-unchanged t/t &&
        rm t/t &&