[PATCH] git-grep: add --color to highlight matches

[PATCH] git-grep: add --color to highlight matches

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Documentation/git-grep.txt |    4 +++
 builtin-grep.c             |    6 +++-
 grep.c                     |   62 +++++++++++++++++++++++++++++++++++++++++---
 grep.h                     |    1 +
 4 files changed, 68 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index a97f055..20ef098 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -16,6 +16,7 @@ SYNOPSIS
 	   [-F | --fixed-strings] [-n]
 	   [-l | --files-with-matches] [-L | --files-without-match]
 	   [-c | --count] [--all-match]
+	   [--color]
 	   [-A <post-context>] [-B <pre-context>] [-C <context>]
 	   [-f <file>] [-e] <pattern>
 	   [--and|--or|--not|(|)|-e <pattern>...] [<tree>...]
@@ -85,6 +86,9 @@ OPTIONS
 	Instead of showing every matched line, show the number of
 	lines that match.
 
+--color::
+	Show colored matches.
+
 -[ABC] <context>::
 	Show `context` trailing (`A` -- after), or leading (`B`
 	-- before), or both (`C` -- context) lines, and place a
diff --git a/builtin-grep.c b/builtin-grep.c
index ef29910..118ec32 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -386,7 +386,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)
 	 * we grep through the checked-out files. It tends to
 	 * be a lot more optimized
 	 */
-	if (!cached) {
+	if (!cached && !opt->color) {
 		hit = external_grep(opt, paths, cached);
 		if (hit >= 0)
 			return hit;
@@ -708,6 +708,10 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			opt.relative = 0;
 			continue;
 		}
+		if (!strcmp("--color", arg)) {
+			opt.color = 1;
+			continue;
+		}
 		if (!strcmp("--", arg)) {
 			/* later processing wants to have this at argv[1] */
 			argv--;
diff --git a/grep.c b/grep.c
index f67d671..7cc05b0 100644
--- a/grep.c
+++ b/grep.c
@@ -247,7 +247,10 @@ static int fixmatch(const char *pattern, char *line, regmatch_t *match)
 	}
 }
 
-static int match_one_pattern(struct grep_opt *opt, struct grep_pat *p, char *bol, char *eol, enum grep_context ctx)
+static int match_one_pattern_1(struct grep_opt *opt, struct grep_pat *p,
+			       char *bol, char *eol,
+			       enum grep_context ctx, int eflags,
+			       int *rm_so, int *rm_eo)
 {
 	int hit = 0;
 	int at_true_bol = 1;
@@ -261,7 +264,7 @@ static int match_one_pattern(struct grep_opt *opt, struct grep_pat *p, char *bol
 	if (!opt->fixed) {
 		regex_t *exp = &p->regexp;
 		hit = !regexec(exp, bol, ARRAY_SIZE(pmatch),
-			       pmatch, 0);
+			       pmatch, eflags);
 	}
 	else {
 		hit = !fixmatch(p->pattern, bol, pmatch);
@@ -298,9 +301,20 @@ static int match_one_pattern(struct grep_opt *opt, struct grep_pat *p, char *bol
 			goto again;
 		}
 	}
+	if (hit) {
+		if (rm_so)
+			*rm_so = pmatch[0].rm_so;
+		if (rm_eo)
+			*rm_eo = pmatch[0].rm_eo;
+	}
 	return hit;
 }
 
+static int match_one_pattern(struct grep_opt *opt, struct grep_pat *p, char *bol, char *eol, enum grep_context ctx)
+{
+	return match_one_pattern_1(opt, p, bol, eol, ctx, 0, NULL, NULL);
+}
+
 static int match_expr_eval(struct grep_opt *o,
 			   struct grep_expr *x,
 			   char *bol, char *eol,
@@ -365,6 +379,42 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,
 	return 0;
 }
 
+static void show_line_colored(struct grep_opt *opt, char *bol, char *eol,
+			      const char *name, unsigned lno, char sign)
+{
+	struct grep_pat *p;
+	int rm_so, rm_eo, eflags = 0;
+	int ch;
+
+	if (opt->pathname)
+		printf("%s%c", name, sign);
+	if (opt->linenum)
+		printf("%d%c", lno, sign);
+
+	ch = *eol;
+	*eol = 0;
+	while (bol < eol) {
+		for (p = opt->pattern_list; p; p = p->next) {
+			if (match_one_pattern_1(opt, p, bol, eol, GREP_CONTEXT_BODY, eflags, &rm_so, &rm_eo))
+				break;
+		}
+
+		/* No match, break the loop */
+		if (!p ||
+		    (rm_so < 0) || (eol - bol) <= rm_so ||
+		    (rm_eo < 0) || (eol - bol) < rm_eo)
+			break;
+
+		printf("%.*s\033[31m\033[1m%.*s\033[m",
+		       rm_so, bol,
+		       rm_eo-rm_so, bol+rm_so);
+		bol += rm_eo;
+		eflags = REG_NOTBOL;
+	}
+	printf("%s\n", bol);
+	*eol = ch;
+}
+
 static int grep_buffer_1(struct grep_opt *opt, const char *name,
 			 char *buf, unsigned long size, int collect_hits)
 {
@@ -466,8 +516,12 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 			}
 			if (last_shown && lno != last_shown + 1)
 				printf(hunk_mark);
-			if (!opt->count)
-				show_line(opt, bol, eol, name, lno, ':');
+			if (!opt->count) {
+				if (opt->color)
+					show_line_colored(opt, bol, eol, name, lno, ':');
+				else
+					show_line(opt, bol, eol, name, lno, ':');
+			}
 			last_shown = last_hit = lno;
 		}
 		else if (last_hit &&
diff --git a/grep.h b/grep.h
index d252dd2..979f7d0 100644
--- a/grep.h
+++ b/grep.h
@@ -68,6 +68,7 @@ struct grep_opt {
 	unsigned extended:1;
 	unsigned relative:1;
 	unsigned pathname:1;
+	unsigned color:1;
 	int regflags;
 	unsigned pre_context;
 	unsigned post_context;
-- 
1.5.5.GIT

Re: [PATCH] git-grep: add --color to highlight matches

[Jakub Narebski]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> +--color::
> +	Show colored matches.
> +

Why not borrow from GNU grep manpage?

   --colour[=WHEN], --color[=WHEN]
      Surround  the  matching  string  with  the marker find in GREP_COLOR
      environment variable. WHEN may be 'never', 'always', or 'auto'

We probably would also want it configurable (via config variables),
as `color.grep' I guess.

I think compatibility with grep (using GREP_COLOR) would be good idea,
and easily implemented.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH] git-grep: add --color to highlight matches

[Nguyen Thai Ngoc Duy]

On Sat, May 24, 2008 at 3:28 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>
>> +--color::
>> +     Show colored matches.
>> +
>
> Why not borrow from GNU grep manpage?
>
>   --colour[=WHEN], --color[=WHEN]
>      Surround  the  matching  string  with  the marker find in GREP_COLOR
>      environment variable. WHEN may be 'never', 'always', or 'auto'
>
> We probably would also want it configurable (via config variables),
> as `color.grep' I guess.

Yes. But this patch was really to scratch my itch and I felt good
after having it colorized so did not bother with configuration system.
I will probably make it a config var when I get tired of typing
--color too often ;)

> I think compatibility with grep (using GREP_COLOR) would be good idea,
> and easily implemented.

I had no idea that variable existed.

> --
> Jakub Narebski
> Poland
> ShadeHawk on #git
>



-- 
Duy

Re: [PATCH] git-grep: add --color to highlight matches

[Jakub Narebski]

On Sat, 24 May 2008, Nguyen Thai Ngoc Duy wrote:
> On Sat, May 24, 2008 at 3:28 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
>>
>>> +--color::
>>> +     Show colored matches.
>>> +
>>
>> Why not borrow from GNU grep manpage?
>>
>>   --colour[=WHEN], --color[=WHEN]
>>      Surround  the  matching  string  with  the marker find in GREP_COLOR
>>      environment variable. WHEN may be 'never', 'always', or 'auto'
>>
>> We probably would also want it configurable (via config variables),
>> as `color.grep' I guess.

And color.grep.<slot>, where slot can be 'match', and probably later also
'context' (I don't remember if git-grep implements -C<num> option...).
 
> Yes. But this patch was really to scratch my itch and I felt good
> after having it colorized so did not bother with configuration system.
> I will probably make it a config var when I get tired of typing
> --color too often ;)

Thanks for sharing, although I guess Junio would prefer more complete
patch...

>> I think compatibility with grep (using GREP_COLOR) would be good idea,
>> and easily implemented.
> 
> I had no idea that variable existed.

Well, I have though that git always use external grep, so this would
be for free. After reading the patch more carefully it seems like it
isn't so.

Still, perhaps that would be a good idea, but I guess for later patch.
-- 
Jakub Narebski
Poland

Re: [PATCH] git-grep: add --color to highlight matches

[Andreas Ericsson]

Jakub Narebski wrote:
> On Sat, 24 May 2008, Nguyen Thai Ngoc Duy wrote:
>> On Sat, May 24, 2008 at 3:28 PM, Jakub Narebski <jnareb@gmail.com> wrote:
>>> Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
> 
>>> I think compatibility with grep (using GREP_COLOR) would be good idea,
>>> and easily implemented.
>> I had no idea that variable existed.
> 
> Well, I have though that git always use external grep, so this would
> be for free. After reading the patch more carefully it seems like it
> isn't so.
> 
> Still, perhaps that would be a good idea, but I guess for later patch.

I can't help but think that colorization for external-grep systems only
would still be worthwhile, as the majority of git users are on such
systems. It'd still be a separate implementation from adding it to the
internal grep functionality anyways, so it's not as if work would be
lost by going half the way here.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH] git-grep: add --color to highlight matches

[Johannes Schindelin]

Hi,

On Mon, 26 May 2008, Andreas Ericsson wrote:

> I can't help but think that colorization for external-grep systems only 
> would still be worthwhile, as the majority of git users are on such 
> systems. It'd still be a separate implementation from adding it to the 
> internal grep functionality anyways, so it's not as if work would be 
> lost by going half the way here.

Huh?  I thought that the external grep is only a matter of fallback for 
performance reasons (if the files are already checked out), or if the user 
specifies the external grep explicitely.

At least for the platform we bend over most, our beloved anachronistic M$ 
Windows, I do not recall forcing external grep.

Besides, it would be a kludge at best to work _twice_ to find out the 
search terms, once in the external grep, and a second time in the coloring 
code.  So I think it should not be done.

Ciao,
Dscho

Re: [PATCH] git-grep: add --color to highlight matches

[Nguyen Thai Ngoc Duy]

On Mon, May 26, 2008 at 5:16 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Besides, it would be a kludge at best to work _twice_ to find out the
> search terms, once in the external grep, and a second time in the coloring
> code.  So I think it should not be done.

I think if it's GNU grep, just passing it --color, it will grep and
colorize search terms in one turn.
-- 
Duy

Re: [PATCH] git-grep: add --color to highlight matches

[Johannes Schindelin]

Hi,

On Mon, 26 May 2008, Nguyen Thai Ngoc Duy wrote:

> On Mon, May 26, 2008 at 5:16 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
> > Besides, it would be a kludge at best to work _twice_ to find out the
> > search terms, once in the external grep, and a second time in the coloring
> > code.  So I think it should not be done.
> 
> I think if it's GNU grep, just passing it --color, it will grep and
> colorize search terms in one turn.

And what tells you that the called grep is GNU grep?

Ciao,
Dscho

Re: [PATCH] git-grep: add --color to highlight matches

[Nguyen Thai Ngoc Duy]

On Mon, May 26, 2008 at 6:00 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Mon, 26 May 2008, Nguyen Thai Ngoc Duy wrote:
>
>> On Mon, May 26, 2008 at 5:16 PM, Johannes Schindelin
>> <Johannes.Schindelin@gmx.de> wrote:
>> > Besides, it would be a kludge at best to work _twice_ to find out the
>> > search terms, once in the external grep, and a second time in the coloring
>> > code.  So I think it should not be done.
>>
>> I think if it's GNU grep, just passing it --color, it will grep and
>> colorize search terms in one turn.
>
> And what tells you that the called grep is GNU grep?

A newly added macro like HAS_GNU_GREP? Granted it won't work all the
time. The user who set the macro should know what he is doing. This
approach is IMHO fine as long as we don't allow color customization.
-- 
Duy

Re: [PATCH] git-grep: add --color to highlight matches

[Johannes Schindelin]

Hi,

On Mon, 26 May 2008, Nguyen Thai Ngoc Duy wrote:

> On Mon, May 26, 2008 at 6:00 PM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> wrote:
>
> > On Mon, 26 May 2008, Nguyen Thai Ngoc Duy wrote:
> >
> >> On Mon, May 26, 2008 at 5:16 PM, Johannes Schindelin
> >> <Johannes.Schindelin@gmx.de> wrote:
> >> > Besides, it would be a kludge at best to work _twice_ to find out 
> >> > the search terms, once in the external grep, and a second time in 
> >> > the coloring code.  So I think it should not be done.
> >>
> >> I think if it's GNU grep, just passing it --color, it will grep and 
> >> colorize search terms in one turn.
> >
> > And what tells you that the called grep is GNU grep?
> 
> A newly added macro like HAS_GNU_GREP? Granted it won't work all the
> time. The user who set the macro should know what he is doing. This
> approach is IMHO fine as long as we don't allow color customization.

IMO this is just a kludge that is not even necessary, because the whole 
notion of relying on an external grep's colorization is crap.

The OP has a patch that adds colorization to Git's internal grep code.  
The user specifies "--color" with grep?  Fine, then it's no external grep.  
Full stop.

Just like the original patch implements it.  Nice, simple, and easy.  What 
is so wrong with that?

Ciao,
Dscho