Re: [PATCH] Support author and commiter based revision list limiting
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:42:38
Subsystem:
the rest · Maintainer:
Linus Torvalds
Jonas Fonseca [off-list ref] writes:
quoted hunk ↗ jump to hunk
Adds the two options: --author=string and --committer=string, which can be used to limit the set of interesting commits to the ones matching the given idents.diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt index d6c86db..4d5f5ab 100644 --- a/Documentation/git-rev-list.txt +++ b/Documentation/git-rev-list.txt@@ -12,6 +12,8 @@ SYNOPSIS 'git-rev-list' [ \--max-count=number ] [ \--max-age=timestamp ] [ \--min-age=timestamp ] + [ \--author=string ] + [ \--committer=string ] [ \--sparse ] [ \--no-merges ] [ \--remove-empty ]
These two separate options are Ok at the level of front-end, but...
quoted hunk ↗ jump to hunk
diff --git a/builtin-rev-list.c b/builtin-rev-list.c index 402af8e..09e3d37 100644 --- a/builtin-rev-list.c +++ b/builtin-rev-list.c@@ -352,7 +352,8 @@ int cmd_rev_list(int argc, const char ** revs.diff) usage(rev_list_usage); - save_commit_buffer = revs.verbose_header; + if (!revs.author_pattern && !revs.committer_pattern) + save_commit_buffer = revs.verbose_header; track_object_refs = 0; if (bisect_list) revs.limited = 1;
I wonder if it is simpler and yet more powerful to internally use a regex to match the contents of commit buffer, not just specific its header fields. When --author or --committer is given, you internally synthesize a regex "^author Jonas Fonseca <fonseca@" from the string. And then, instead of doing commit_match_ident() twice like this:
quoted hunk ↗ jump to hunk
@@ -1074,6 +1123,10 @@ struct commit *get_revision(struct rev_i if (revs->no_merges && commit->parents && commit->parents->next) continue; + if (!commit_match_ident(commit, "author", revs->author_pattern)) + continue; + if (!commit_match_ident(commit, "committer", revs->committer_pattern)) + continue;
you would just do:
if (revs->commit_filter_pattern &&
commit_search_message(commit, revs->commit_filter_pattern))
continue;
instead.
For an extra bonus point, the matching logic might want to steal
from builtin-grep to allow multiple regular expressions, case
insensitive match and other bells and whistles. You probably
could lift the whole grep_buffer() -- add another option that
behaves similarly to opt->name_only (name it opt->status_only)
but make it not even print anything upon hit, so that you can
tell from the return value if it found the pattern in the
buffer, like this:
diff --git a/builtin-grep.c b/builtin-grep.c
index 8213ce2..714ad50 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c@@ -128,6 +128,7 @@ struct grep_opt { unsigned linenum:1; unsigned invert:1; unsigned name_only:1; + unsigned status_only:1; unsigned unmatch_name_only:1; unsigned count:1; unsigned word_regexp:1;
@@ -492,6 +493,8 @@ static int grep_buffer(struct grep_opt * } if (hit) { count++; + if (opt->status_only) + return 1; if (binary_match_only) { printf("Binary file %s matches\n", name); return 1;
Hmm?