On Sat, Sep 29, 2012 at 11:41:28AM +0700, Nguyen Thai Ngoc Duy wrote:
> @@ -2210,10 +2213,23 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
>
> static int commit_match(struct commit *commit, struct rev_info *opt)
> {
> + int retval;
> + struct strbuf buf = STRBUF_INIT;
> if (!opt->grep_filter.pattern_list && !opt->grep_filter.header_list)
> return 1;
> - return grep_buffer(&opt->grep_filter,
> - commit->buffer, strlen(commit->buffer));
> + if (opt->reflog_info) {
> + strbuf_addstr(&buf, "reflog ");
> + get_reflog_message(&buf, opt->reflog_info);
> + strbuf_addch(&buf, '\n');
> + strbuf_addstr(&buf, commit->buffer);
> + }
> + if (buf.len)
> + retval = grep_buffer(&opt->grep_filter, buf.buf, buf.len);
> + else
> + retval = grep_buffer(&opt->grep_filter,
> + commit->buffer, strlen(commit->buffer));
> + strbuf_release(&buf);
> + return retval;
I like how callers not doing a reflog walk do not have to pay the price
to do the extra allocating. We could further limit it to only when
--grep-reflog is in effect, but I guess that would mean wading through
grep_filter's patterns, since it could be buried amidst ANDs and ORs?
One alternative would be to set a bit in the grep_opt when we call
append_header_grep_pattern. It feels a bit like a layering violation,
though. I guess the bit could also go into rev_info. It may not even be
a measurable slowdown, though. Premature optimization and all that.
Other than that, I didn't notice anything wrong in the patch.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html