Re: [PATCH 2/3] revision: add --grep-reflog to filter commits by reflog messages

Jeff King [off-list ref] writes:

quoted

+	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.

I do not think it is a layering violation.  compile_grep_exp()
should be aware of the short-cut possibilities and your "our
expression is interested in reflog so we need to read it" is very
similar in spirit to the existing opt->extended bit.

It will obviously allow us to avoid reading reflog information
unnecessarily here.  I think it makes perfect sense.

We may also want to flag the use of the --grep-reflog option when
the --walk-reflogs option is not in effect in setup_revisions() as
an error, or something.