Re: [PATCH] grep: do not do external grep on skip-worktree entries

Linus Torvalds [off-list ref] writes:

The bad news is that you broke multi-line greps:

	git grep --no-ext-grep -2 qwerty.*as

results in:

	drivers/char/keyboard.c-unsigned char kbd_sysrq_xlate[KEY_MAX + 1] =
	drivers/char/keyboard.c-        "\000\0331234567890-=\177\t"                    /* 0x00 - 0x0f */
	drivers/char/keyboard.c:        "qwertyuiop[]\r\000as"                          /* 0x10 - 0x1f */

Meh.  I checked pre-context codepath before sending the patch and was very
satisfied that René did the right thing in 49de321 (grep: handle pre
context lines on demand, 2009-07-02), but somehow forgot about the post
context codepath.

An ObviouslyRightThing fix is this two-liner.  We shouldn't lookahead if
we want to do something more than just skipping when we see an unmatch for
the line we are currently looking at.

 grep.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/grep.c b/grep.c
index 940e200..ac0ce0b 100644
--- a/grep.c
+++ b/grep.c

@@ -719,6 +719,8 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 		int hit;
 
 		if (try_lookahead
+		    && !(last_hit
+			 && lno <= last_hit + opt->post_context)
 		    && look_ahead(opt, &left, &lno, &bol))
 			break;
 		eol = end_of_line(bol, &left);

On Mon, 11 Jan 2010, Junio C Hamano wrote:

An ObviouslyRightThing fix is this two-liner.  We shouldn't lookahead if
we want to do something more than just skipping when we see an unmatch for
the line we are currently looking at.

Ack. Works for me. And with that, I'd love for it to go in, and get rid of 
the external grep. Performance is now a non-issue (it goes both ways), and 
the internal grep doesn't have the bug with separators between multi-line 
greps.

And dropping the external one gets rid of all the issues with PATHs, crap 
'grep' implementations, and removes actual code. Goodie.

		Linus