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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to