@matheusbsbs / matheus-bagliano / commits / 80235ba79e
Fork 0

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

Junio C Hamano committed Jan 17, 2010 at 20:09 UTC 80235ba79ef43349f455cce869397b3e726f4058
6 files changed +57 -10
builtin-grep.c
+1
@@ -820,6 +820,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
820 opt.relative = 1;
821 opt.pathname = 1;
822 opt.pattern_tail = &opt.pattern_list;
823 + opt.header_tail = &opt.header_list;
824 opt.regflags = REG_NEWLINE;
825 opt.max_depth = -1;
826
builtin-rev-list.c
+3 -2
@@ -371,8 +371,9 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
371 revs.diff)
372 usage(rev_list_usage);
373
374 - save_commit_buffer = revs.verbose_header ||
375 - revs.grep_filter.pattern_list;
374 + save_commit_buffer = (revs.verbose_header ||
375 + revs.grep_filter.pattern_list ||
376 + revs.grep_filter.header_list);
377 if (bisect_list)
378 revs.limited = 1;
379
grep.c
+40 -6
@@ -11,8 +11,8 @@ void append_header_grep_pattern(struct grep_opt *opt, enum grep_header_field fie
11 p->no = 0;
12 p->token = GREP_PATTERN_HEAD;
13 p->field = field;
14 - *opt->pattern_tail = p;
15 - opt->pattern_tail = &p->next;
14 + *opt->header_tail = p;
15 + opt->header_tail = &p->next;
16 p->next = NULL;
17 }
18
@@ -172,9 +172,26 @@ static struct grep_expr *compile_pattern_expr(struct grep_pat **list)
172 void compile_grep_patterns(struct grep_opt *opt)
173 {
174 struct grep_pat *p;
175 -
176 - if (opt->all_match)
177 - opt->extended = 1;
175 + struct grep_expr *header_expr = NULL;
176 +
177 + if (opt->header_list) {
178 + p = opt->header_list;
179 + header_expr = compile_pattern_expr(&p);
180 + if (p)
181 + die("incomplete pattern expression: %s", p->pattern);
182 + for (p = opt->header_list; p; p = p->next) {
183 + switch (p->token) {
184 + case GREP_PATTERN: /* atom */
185 + case GREP_PATTERN_HEAD:
186 + case GREP_PATTERN_BODY:
187 + compile_regexp(p, opt);
188 + break;
189 + default:
190 + opt->extended = 1;
191 + break;
192 + }
193 + }
194 + }
195
196 for (p = opt->pattern_list; p; p = p->next) {
197 switch (p->token) {
@@ -189,7 +206,9 @@ void compile_grep_patterns(struct grep_opt *opt)
206 }
207 }
208
192 - if (!opt->extended)
209 + if (opt->all_match || header_expr)
210 + opt->extended = 1;
211 + else if (!opt->extended)
212 return;
213
214 /* Then bundle them up in an expression.
@@ -200,6 +219,21 @@ void compile_grep_patterns(struct grep_opt *opt)
219 opt->pattern_expression = compile_pattern_expr(&p);
220 if (p)
221 die("incomplete pattern expression: %s", p->pattern);
222 +
223 + if (!header_expr)
224 + return;
225 +
226 + if (opt->pattern_expression) {
227 + struct grep_expr *z;
228 + z = xcalloc(1, sizeof(*z));
229 + z->node = GREP_NODE_OR;
230 + z->u.binary.left = opt->pattern_expression;
231 + z->u.binary.right = header_expr;
232 + opt->pattern_expression = z;
233 + } else {
234 + opt->pattern_expression = header_expr;
235 + }
236 + opt->all_match = 1;
237 }
238
239 static void free_pattern_expr(struct grep_expr *x)
grep.h
+2
@@ -59,6 +59,8 @@ struct grep_expr {
59 struct grep_opt {
60 struct grep_pat *pattern_list;
61 struct grep_pat **pattern_tail;
62 + struct grep_pat *header_list;
63 + struct grep_pat **header_tail;
64 struct grep_expr *pattern_expression;
65 const char *prefix;
66 int prefix_length;
revision.c
+2 -1
@@ -806,6 +806,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)
806
807 revs->grep_filter.status_only = 1;
808 revs->grep_filter.pattern_tail = &(revs->grep_filter.pattern_list);
809 + revs->grep_filter.header_tail = &(revs->grep_filter.header_list);
810 revs->grep_filter.regflags = REG_NEWLINE;
811
812 diff_setup(&revs->diffopt);
@@ -1751,7 +1752,7 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
1752
1753 static int commit_match(struct commit *commit, struct rev_info *opt)
1754 {
1754 - if (!opt->grep_filter.pattern_list)
1755 + if (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)
1756 return 1;
1757 return grep_buffer(&opt->grep_filter,
1758 NULL, /* we say nothing, not even filename */
t/t7002-grep.sh
+9 -1
@@ -357,7 +357,7 @@ test_expect_success 'log grep (4)' '
357 '
358
359 test_expect_success 'log grep (5)' '
360 - git log --author=Thor -F --grep=Thu --pretty=tformat:%s >actual &&
360 + git log --author=Thor -F --pretty=tformat:%s >actual &&
361 ( echo third ; echo initial ) >expect &&
362 test_cmp expect actual
363 '
@@ -368,6 +368,14 @@ test_expect_success 'log grep (6)' '
368 test_cmp expect actual
369 '
370
371 +test_expect_success 'log --grep --author implicitly uses all-match' '
372 + # grep matches initial and second but not third
373 + # author matches only initial and third
374 + git log --author="A U Thor" --grep=s --grep=l --format=%s >actual &&
375 + echo initial >expect &&
376 + test_cmp expect actual
377 +'
378 +
379 test_expect_success 'grep with CE_VALID file' '
380 git update-index --assume-unchanged t/t &&
381 rm t/t &&