Re: [PATCH] grep: add known breakage of coloring when using extended patterns

From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: May 2, 2011, 21:07 UTC
Message-ID: <4DBF1D2A.6030807@lsrfire.ath.cx>
In-Reply-To: <7vd3k1ngun.fsf@alter.siamese.dyndns.org>

Am 02.05.2011 19:37, schrieb Junio C Hamano:

Show 13 quoted lines

> René Scharfe<rene.scharfe@lsrfire.ath.cx>  writes:
>
>>> +	test_commit initial input "foo bar baz
> ...
>>> +	git grep --color -e foo --or \( -e bar --and --not -e baz \) |
> ...
>> The current code highlights the given search terms ("atoms").
>
> Hmm, I was probably not paying attention to "coloring the parts that
> matched" topic at all, but wouldn't it be easier and more efficient to
> paint only "foo" without painting "bar baz"?  We know the first term "foo"
> matches, and the rest \(...\) that is --or'ed does not have to even be
> evaluated, no?

Aggregating the set of matching characters and passing them back during expression evaluation can be more efficient, yes, as it would avoid calling regexec() on the printed lines again only to find out what to color. But I wouldn't call it easy. E.g. how to return the set of matching characters in the following case?

	$ git grep --color -e foo --and -e bar

I can only think of using lists or, even uglier, perhaps a bitmap to remember which characters to highlight.

Also GNU grep doesn't only color the first sufficient match. E.g. this will color both b and d:

	$ echo abcde | grep --color -e b -e d

And I think this makes sense and meets my expectations.

But the other mode, which only colors the exact characters that were used to determine that the shown line matches (or doesn't match, if -v is given) may be interesting as well. I'm curious to see an implementation and wonder if the results are really better than the ones of the current (cheating) approach. :)

René