Re: [PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 29, 2012, 19:11 UTC
- Message-ID
- <7vbogoiqzs.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <CACsJy8A1UroqCezJFjqOqBQg+puX=jc1Q-CMSB4f=CHULY=OyA@mail.gmail.com>
Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> reflog, in terms of both the number of commits and message length, is > usually short enough that slowdown does not really show, especially > when used with git-log, an interactive command.
You shouldn't do things you can easily tell you do not need to, especially when the effort to avoid doing unnecessary allocation, copying and deallocation is not excessively larger than the saving.
I personally think that lack of perceived performance impact is not an excuse to be sloppy. These things tend to add up.
> That's why I put "Ignored unless --walk-reflogs is given" in the > document. But an error would be fine too. I suppose an error is > preferable in case users do not read document carefully?
A reader who reads documentation carefully will spot that it is a sloppy coding that such a nonsense combination of two options are not caught and diagnosed as an error, when accepting and silently ignoring the option would not help anybody.
An alternative may be to turn -g on when --grep-reflog is given. Starting from a version that forbids --grep-reflog without -g will let us change the command line parser to do so in the future without backward incompatibility, but you cannot do so if you start from a version that accepts and silently ignores.
How about squashing this in? I've future-proofed commit_match() a bit while at it; it would be cleaner to add new fake headers if the function is done this way.
Documentation/rev-list-options.txt | 4 ++-- grep.c | 2 ++ grep.h | 1 + revision.c | 13 +++++++++++-- t/t7810-grep.sh | 4 ++++ 5 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index aa7cd9d..ca22106 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -56,8 +56,8 @@ endif::git-rev-list[] Limit the commits output to ones with reflog entries that match the specified pattern (regular expression). With more than one `--grep-reflog`, commits whose reflog message - matches any of the given patterns are chosen. Ignored unless - `--walk-reflogs` is given. + matches any of the given patterns are chosen. It is an + error to use this option unless `--walk-reflogs` is in use. --grep=<pattern>:: diff --git a/grep.c b/grep.c index d70dcdf..edc7776 100644 --- a/grep.c +++ b/grep.c @@ -64,6 +64,8 @@ void append_header_grep_pattern(struct grep_opt *opt, { struct grep_pat *p = create_grep_pat(pat, strlen(pat), "header", 0, GREP_PATTERN_HEAD, field); + if (field == GREP_HEADER_REFLOG) + opt->use_reflog_filter = 1; do_append_grep_pat(&opt->header_tail, p); } diff --git a/grep.h b/grep.h index 6e78b96..c256ac6 100644 --- a/grep.h +++ b/grep.h @@ -107,6 +107,7 @@ struct grep_opt { #define GREP_BINARY_TEXT 2 int binary; int extended; + int use_reflog_filter; int pcre; int relative; int pathname; diff --git a/revision.c b/revision.c index 109bec1..9ad72df 100644 --- a/revision.c +++ b/revision.c @@ -1908,6 +1908,8 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s if (revs->reflog_info && revs->graph) die("cannot combine --walk-reflogs with --graph"); + if (!revs->reflog_info && revs->grep_filter.use_reflog_filter) + die("cannot use --grep-reflog without --walk-reflogs"); return left; } @@ -2217,12 +2219,19 @@ static int commit_match(struct commit *commit, struct rev_info *opt) struct strbuf buf = STRBUF_INIT; if (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list) return 1; - if (opt->reflog_info) { + + /* Prepend "fake" headers as needed */ + if (opt->grep_filter.use_reflog_filter) { strbuf_addstr(&buf, "reflog "); get_reflog_message(&buf, opt->reflog_info); strbuf_addch(&buf, '\n'); - strbuf_addstr(&buf, commit->buffer); } + + /* Copy the commit to temporary if we are using "fake" headers */ + if (buf.len) + strbuf_addstr(&buf, commit->buffer); + + /* Find either in the commit object, or in the temporary */ if (buf.len) retval = grep_buffer(&opt->grep_filter, buf.buf, buf.len); else diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh index 3a5d0fd..f698001 100755 --- a/t/t7810-grep.sh +++ b/t/t7810-grep.sh @@ -572,6 +572,10 @@ test_expect_success 'log grep (9)' ' test_cmp expect actual ' +test_expect_success 'log --grep-reflog can only be used under -g' ' + test_must_fail git log --grep-reflog="commit: third" +' + test_expect_success 'log with multiple --grep uses union' ' git log --grep=i --grep=r --format=%s >actual && {
-- 1.7.12.1.484.g1764bc0