[PATCH 0/5] grep: color search patterns

[PATCH 0/5] grep: color search patterns

[René Scharfe]

Match coloring is a major missing feature of git grep.  It makes reading
the output much easier and brings grep onto the same visual level as the
other colorized git commands.

Two earlier attempts to implement this feature didn't go in.  They
didn't support support pattern expressions (--and, --or etc.) or
external greps, unlike this series.

[PATCH 1/5] grep: micro-optimize hit collection for AND nodes
[PATCH 2/5] grep: remove grep_opt argument from match_expr_eval()

   These are cleanup patches and unrelated to coloring.  They touch the
   same code, though, so it's a good idea to take this opportunity to
   beautify the code a bit before adding new features.

[PATCH 3/5] grep: add pmatch and eflags arguments to match_one_pattern()

   The two arguments added by this patch are used by the next one.

[PATCH 4/5] grep: color patterns in output

   Add color support to internal git grep.

[PATCH 5/5] grep: add support for coloring with external greps

   Extends color support to external greps.

[PATCH 1/5] grep: micro-optimize hit collection for AND nodes

[René Scharfe]

In addition to returning if an expression matches a line,
match_expr_eval() updates the expression's hit flag if the parameter
collect_hits is set.  It never sets collect_hits for children of AND
nodes, though, so their hit flag will never be updated.  Because of
that we can return early if the first child didn't match, no matter
if collect_hits is set or not.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 grep.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/grep.c b/grep.c
index 062b2b6..db341b6 100644
--- a/grep.c
+++ b/grep.c
@@ -394,13 +394,9 @@ static int match_expr_eval(struct grep_opt *o,
 		h = !match_expr_eval(o, x->u.unary, bol, eol, ctx, 0);
 		break;
 	case GREP_NODE_AND:
-		if (!collect_hits)
-			return (match_expr_eval(o, x->u.binary.left,
-						bol, eol, ctx, 0) &&
-				match_expr_eval(o, x->u.binary.right,
-						bol, eol, ctx, 0));
-		h = match_expr_eval(o, x->u.binary.left, bol, eol, ctx, 0);
-		h &= match_expr_eval(o, x->u.binary.right, bol, eol, ctx, 0);
+		if (!match_expr_eval(o, x->u.binary.left, bol, eol, ctx, 0))
+			return 0;
+		h = match_expr_eval(o, x->u.binary.right, bol, eol, ctx, 0);
 		break;
 	case GREP_NODE_OR:
 		if (!collect_hits)
-- 
1.6.2

[PATCH 2/5] grep: remove grep_opt argument from match_expr_eval()

[René Scharfe]

The only use of the struct grep_opt argument of match_expr_eval()
is to pass the option word_regexp to match_one_pattern().  By adding
a pattern flag for it we can reduce the number of function arguments
of these two functions, as a cleanup and preparation for adding more
in the next patch.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 grep.c |   34 +++++++++++++++++-----------------
 grep.h |    1 +
 2 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/grep.c b/grep.c
index db341b6..f455182 100644
--- a/grep.c
+++ b/grep.c
@@ -39,6 +39,8 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
 {
 	int err;
 
+	p->word_regexp = opt->word_regexp;
+
 	if (opt->fixed || is_fixed(p->pattern))
 		p->fixed = 1;
 	if (opt->regflags & REG_ICASE)
@@ -306,7 +308,8 @@ static struct {
 	{ "committer ", 10 },
 };
 
-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(struct grep_pat *p, char *bol, char *eol,
+			     enum grep_context ctx)
 {
 	int hit = 0;
 	int saved_ch = 0;
@@ -338,7 +341,7 @@ static int match_one_pattern(struct grep_opt *opt, struct grep_pat *p, char *bol
 		hit = !fixmatch(p->pattern, bol, pmatch);
 	}
 
-	if (hit && opt->word_regexp) {
+	if (hit && p->word_regexp) {
 		if ((pmatch[0].rm_so < 0) ||
 		    (eol - bol) <= pmatch[0].rm_so ||
 		    (pmatch[0].rm_eo < 0) ||
@@ -378,35 +381,32 @@ static int match_one_pattern(struct grep_opt *opt, struct grep_pat *p, char *bol
 	return hit;
 }
 
-static int match_expr_eval(struct grep_opt *o,
-			   struct grep_expr *x,
-			   char *bol, char *eol,
-			   enum grep_context ctx,
-			   int collect_hits)
+static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,
+			   enum grep_context ctx, int collect_hits)
 {
 	int h = 0;
 
 	switch (x->node) {
 	case GREP_NODE_ATOM:
-		h = match_one_pattern(o, x->u.atom, bol, eol, ctx);
+		h = match_one_pattern(x->u.atom, bol, eol, ctx);
 		break;
 	case GREP_NODE_NOT:
-		h = !match_expr_eval(o, x->u.unary, bol, eol, ctx, 0);
+		h = !match_expr_eval(x->u.unary, bol, eol, ctx, 0);
 		break;
 	case GREP_NODE_AND:
-		if (!match_expr_eval(o, x->u.binary.left, bol, eol, ctx, 0))
+		if (!match_expr_eval(x->u.binary.left, bol, eol, ctx, 0))
 			return 0;
-		h = match_expr_eval(o, x->u.binary.right, bol, eol, ctx, 0);
+		h = match_expr_eval(x->u.binary.right, bol, eol, ctx, 0);
 		break;
 	case GREP_NODE_OR:
 		if (!collect_hits)
-			return (match_expr_eval(o, x->u.binary.left,
+			return (match_expr_eval(x->u.binary.left,
 						bol, eol, ctx, 0) ||
-				match_expr_eval(o, x->u.binary.right,
+				match_expr_eval(x->u.binary.right,
 						bol, eol, ctx, 0));
-		h = match_expr_eval(o, x->u.binary.left, bol, eol, ctx, 0);
+		h = match_expr_eval(x->u.binary.left, bol, eol, ctx, 0);
 		x->u.binary.left->hit |= h;
-		h |= match_expr_eval(o, x->u.binary.right, bol, eol, ctx, 1);
+		h |= match_expr_eval(x->u.binary.right, bol, eol, ctx, 1);
 		break;
 	default:
 		die("Unexpected node type (internal error) %d", x->node);
@@ -420,7 +420,7 @@ static int match_expr(struct grep_opt *opt, char *bol, char *eol,
 		      enum grep_context ctx, int collect_hits)
 {
 	struct grep_expr *x = opt->pattern_expression;
-	return match_expr_eval(opt, x, bol, eol, ctx, collect_hits);
+	return match_expr_eval(x, bol, eol, ctx, collect_hits);
 }
 
 static int match_line(struct grep_opt *opt, char *bol, char *eol,
@@ -432,7 +432,7 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,
 
 	/* we do not call with collect_hits without being extended */
 	for (p = opt->pattern_list; p; p = p->next) {
-		if (match_one_pattern(opt, p, bol, eol, ctx))
+		if (match_one_pattern(p, bol, eol, ctx))
 			return 1;
 	}
 	return 0;
diff --git a/grep.h b/grep.h
index 5102ce3..d2a8674 100644
--- a/grep.h
+++ b/grep.h
@@ -31,6 +31,7 @@ struct grep_pat {
 	enum grep_header_field field;
 	regex_t regexp;
 	unsigned fixed:1;
+	unsigned word_regexp:1;
 };
 
 enum grep_expr_node {
-- 
1.6.2

[PATCH 3/5] grep: add pmatch and eflags arguments to match_one_pattern()

[René Scharfe]

Push pmatch and eflags to the callers of match_one_pattern(), which
allows them to specify regex execution flags and to get the location
of a match.

Since we only use the first element of the matches array and aren't
interested in submatches, no provision is made for callers to
provide a larger array.

eflags are ignored for fixed patterns, but that's OK, since they
only have a meaning in connection with regular expressions
containing ^ or $.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 grep.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/grep.c b/grep.c
index f455182..bdcff7b 100644
--- a/grep.c
+++ b/grep.c
@@ -309,11 +309,11 @@ static struct {
 };
 
 static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
-			     enum grep_context ctx)
+			     enum grep_context ctx,
+			     regmatch_t *pmatch, int eflags)
 {
 	int hit = 0;
 	int saved_ch = 0;
-	regmatch_t pmatch[10];
 
 	if ((p->token != GREP_PATTERN) &&
 	    ((p->token == GREP_PATTERN_HEAD) != (ctx == GREP_CONTEXT_HEAD)))
@@ -332,14 +332,10 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 	}
 
  again:
-	if (!p->fixed) {
-		regex_t *exp = &p->regexp;
-		hit = !regexec(exp, bol, ARRAY_SIZE(pmatch),
-			       pmatch, 0);
-	}
-	else {
+	if (p->fixed)
 		hit = !fixmatch(p->pattern, bol, pmatch);
-	}
+	else
+		hit = !regexec(&p->regexp, bol, 1, pmatch, eflags);
 
 	if (hit && p->word_regexp) {
 		if ((pmatch[0].rm_so < 0) ||
@@ -385,10 +381,11 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,
 			   enum grep_context ctx, int collect_hits)
 {
 	int h = 0;
+	regmatch_t match;
 
 	switch (x->node) {
 	case GREP_NODE_ATOM:
-		h = match_one_pattern(x->u.atom, bol, eol, ctx);
+		h = match_one_pattern(x->u.atom, bol, eol, ctx, &match, 0);
 		break;
 	case GREP_NODE_NOT:
 		h = !match_expr_eval(x->u.unary, bol, eol, ctx, 0);
@@ -427,12 +424,14 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,
 		      enum grep_context ctx, int collect_hits)
 {
 	struct grep_pat *p;
+	regmatch_t match;
+
 	if (opt->extended)
 		return match_expr(opt, bol, eol, ctx, collect_hits);
 
 	/* we do not call with collect_hits without being extended */
 	for (p = opt->pattern_list; p; p = p->next) {
-		if (match_one_pattern(p, bol, eol, ctx))
+		if (match_one_pattern(p, bol, eol, ctx, &match, 0))
 			return 1;
 	}
 	return 0;
-- 
1.6.2

[PATCH 4/5] grep: color patterns in output

[René Scharfe]

Coloring matches makes them easier to spot in the output.

Add two options and two parameters: color.grep (to turn coloring on
or off), color.grep.match (to set the color of matches), --color
and --no-color (to turn coloring on or off, respectively).

The output of external greps is not changed.

This patch is based on earlier ones by Nguyễn Thái Ngọc Duy and
Thiago Alves.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 Documentation/config.txt   |    9 ++++
 Documentation/git-grep.txt |    8 ++++
 builtin-grep.c             |   32 +++++++++++++++
 grep.c                     |   90 ++++++++++++++++++++++++++++++++++++++------
 grep.h                     |    3 +
 5 files changed, 130 insertions(+), 12 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index f5152c5..b75dada 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -548,6 +548,15 @@ color.diff.<slot>::
 	whitespace errors). The values of these variables may be specified as
 	in color.branch.<slot>.
 
+color.grep::
+	When set to `always`, always highlight matches.  When `false` (or
+	`never`), never.  When set to `true` or `auto`, use color only
+	when the output is written to the terminal.  Defaults to `false`.
+
+color.grep.match::
+	Use customized color for matches.  The value of this variable
+	may be specified as in color.branch.<slot>.
+
 color.interactive::
 	When set to `always`, always use colors for interactive prompts
 	and displays (such as those used by "git-add --interactive").
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 553da6c..fccb82d 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -17,6 +17,7 @@ SYNOPSIS
 	   [-l | --files-with-matches] [-L | --files-without-match]
 	   [-z | --null]
 	   [-c | --count] [--all-match]
+	   [--color | --no-color]
 	   [-A <post-context>] [-B <pre-context>] [-C <context>]
 	   [-f <file>] [-e] <pattern>
 	   [--and|--or|--not|(|)|-e <pattern>...] [<tree>...]
@@ -105,6 +106,13 @@ OPTIONS
 	Instead of showing every matched line, show the number of
 	lines that match.
 
+--color::
+	Show colored matches.
+
+--no-color::
+	Turn off match highlighting, even when the configuration file
+	gives the default to color output.
+
 -[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 3f12ba3..e2c0f01 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -22,6 +22,24 @@
 
 static int builtin_grep;
 
+static int grep_config(const char *var, const char *value, void *cb)
+{
+	struct grep_opt *opt = cb;
+
+	if (!strcmp(var, "grep.color") || !strcmp(var, "color.grep")) {
+		opt->color = git_config_colorbool(var, value, -1);
+		return 0;
+	}
+	if (!strcmp(var, "grep.color.match") ||
+	    !strcmp(var, "color.grep.match")) {
+		if (!value)
+			return config_error_nonbool(var);
+		color_parse(value, var, opt->color_match);
+		return 0;
+	}
+	return git_color_default_config(var, value, cb);
+}
+
 /*
  * git grep pathspecs are somewhat different from diff-tree pathspecs;
  * pathname wildcards are allowed.
@@ -536,6 +554,12 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	opt.pattern_tail = &opt.pattern_list;
 	opt.regflags = REG_NEWLINE;
 
+	strcpy(opt.color_match, GIT_COLOR_RED GIT_COLOR_BOLD);
+	opt.color = -1;
+	git_config(grep_config, &opt);
+	if (opt.color == -1)
+		opt.color = git_use_color_default;
+
 	/*
 	 * If there is no -- then the paths must exist in the working
 	 * tree.  If there is no explicit pattern specified with -e or
@@ -732,6 +756,14 @@ 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("--no-color", arg)) {
+			opt.color = 0;
+			continue;
+		}
 		if (!strcmp("--", arg)) {
 			/* later processing wants to have this at argv[1] */
 			argv--;
diff --git a/grep.c b/grep.c
index bdcff7b..cace1c8 100644
--- a/grep.c
+++ b/grep.c
@@ -253,18 +253,6 @@ static int word_char(char ch)
 	return isalnum(ch) || ch == '_';
 }
 
-static void show_line(struct grep_opt *opt, const char *bol, const char *eol,
-		      const char *name, unsigned lno, char sign)
-{
-	if (opt->null_following_name)
-		sign = '\0';
-	if (opt->pathname)
-		printf("%s%c", name, sign);
-	if (opt->linenum)
-		printf("%d%c", lno, sign);
-	printf("%.*s\n", (int)(eol-bol), bol);
-}
-
 static void show_name(struct grep_opt *opt, const char *name)
 {
 	printf("%s%c", name, opt->null_following_name ? '\0' : '\n');
@@ -437,6 +425,84 @@ static int match_line(struct grep_opt *opt, char *bol, char *eol,
 	return 0;
 }
 
+static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,
+			      enum grep_context ctx,
+			      regmatch_t *pmatch, int eflags)
+{
+	regmatch_t match;
+
+	if (!match_one_pattern(p, bol, eol, ctx, &match, eflags))
+		return 0;
+	if (match.rm_so < 0 || match.rm_eo < 0)
+		return 0;
+	if (pmatch->rm_so >= 0 && pmatch->rm_eo >= 0) {
+		if (match.rm_so > pmatch->rm_so)
+			return 1;
+		if (match.rm_so == pmatch->rm_so && match.rm_eo < pmatch->rm_eo)
+			return 1;
+	}
+	pmatch->rm_so = match.rm_so;
+	pmatch->rm_eo = match.rm_eo;
+	return 1;
+}
+
+static int next_match(struct grep_opt *opt, char *bol, char *eol,
+		      enum grep_context ctx, regmatch_t *pmatch, int eflags)
+{
+	struct grep_pat *p;
+	int hit = 0;
+
+	pmatch->rm_so = pmatch->rm_eo = -1;
+	if (bol < eol) {
+		for (p = opt->pattern_list; p; p = p->next) {
+			switch (p->token) {
+			case GREP_PATTERN: /* atom */
+			case GREP_PATTERN_HEAD:
+			case GREP_PATTERN_BODY:
+				hit |= match_next_pattern(p, bol, eol, ctx,
+							  pmatch, eflags);
+				break;
+			default:
+				break;
+			}
+		}
+	}
+	return hit;
+}
+
+static void show_line(struct grep_opt *opt, char *bol, char *eol,
+		      const char *name, unsigned lno, char sign)
+{
+	int rest = eol - bol;
+
+	if (opt->null_following_name)
+		sign = '\0';
+	if (opt->pathname)
+		printf("%s%c", name, sign);
+	if (opt->linenum)
+		printf("%d%c", lno, sign);
+	if (opt->color) {
+		regmatch_t match;
+		enum grep_context ctx = GREP_CONTEXT_BODY;
+		int ch = *eol;
+		int eflags = 0;
+
+		*eol = '\0';
+		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
+			printf("%.*s%s%.*s%s",
+			       match.rm_so, bol,
+			       opt->color_match,
+			       match.rm_eo - match.rm_so, bol + match.rm_so,
+			       GIT_COLOR_RESET);
+			bol += match.rm_eo;
+			rest -= match.rm_eo;
+			eflags = REG_NOTBOL;
+		}
+		*eol = ch;
+	}
+	printf("%.*s\n", rest, bol);
+}
+
 static int grep_buffer_1(struct grep_opt *opt, const char *name,
 			 char *buf, unsigned long size, int collect_hits)
 {
diff --git a/grep.h b/grep.h
index d2a8674..73b33ab 100644
--- a/grep.h
+++ b/grep.h
@@ -1,5 +1,6 @@
 #ifndef GREP_H
 #define GREP_H
+#include "color.h"
 
 enum grep_pat_token {
 	GREP_PATTERN,
@@ -77,6 +78,8 @@ struct grep_opt {
 	unsigned relative:1;
 	unsigned pathname:1;
 	unsigned null_following_name:1;
+	int color;
+	char color_match[COLOR_MAXLEN];
 	int regflags;
 	unsigned pre_context;
 	unsigned post_context;
-- 
1.6.2

[PATCH 5/5] grep: add support for coloring with external greps

[René Scharfe]

Add the config variable color.grep.external, which can be used to
switch on coloring of external greps.  To enable auto coloring with
GNU grep, one needs to set color.grep.external to --color=always to
defeat the pager started by git grep.  The value of the config
variable will be passed to the external grep only if it would
colorize internal grep's output, so automatic terminal detected
works.  The default is to not pass any option, because the external
grep command could be a program without color support.

Also set the environment variables GREP_COLOR and GREP_COLORS to
pass the configured color for matches to the external grep.  This
works with GNU grep; other variables could be added as needed.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 Documentation/config.txt |   12 +++++++++++-
 builtin-grep.c           |   36 ++++++++++++++++++++++++++++++++++++
 grep.h                   |    1 +
 3 files changed, 48 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index b75dada..4d42bff 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -553,9 +553,19 @@ color.grep::
 	`never`), never.  When set to `true` or `auto`, use color only
 	when the output is written to the terminal.  Defaults to `false`.
 
+color.grep.external::
+	The string value of this variable is passed to an external 'grep'
+	command as a command line option if match highlighting is turned
+	on.  If set to an empty string, no option is passed at all,
+	turning off coloring for external 'grep' calls; this is the default.
+	For GNU grep, set it to `--color=always` to highlight matches even
+	when a pager is used.
+
 color.grep.match::
 	Use customized color for matches.  The value of this variable
-	may be specified as in color.branch.<slot>.
+	may be specified as in color.branch.<slot>.  It is passed using
+	the environment variables 'GREP_COLOR' and 'GREP_COLORS' when
+	calling an external 'grep'.
 
 color.interactive::
 	When set to `always`, always use colors for interactive prompts
diff --git a/builtin-grep.c b/builtin-grep.c
index e2c0f01..9e7e766 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -30,6 +30,10 @@ static int grep_config(const char *var, const char *value, void *cb)
 		opt->color = git_config_colorbool(var, value, -1);
 		return 0;
 	}
+	if (!strcmp(var, "grep.color.external") ||
+	    !strcmp(var, "color.grep.external")) {
+		return git_config_string(&(opt->color_external), var, value);
+	}
 	if (!strcmp(var, "grep.color.match") ||
 	    !strcmp(var, "color.grep.match")) {
 		if (!value)
@@ -287,6 +291,21 @@ static int flush_grep(struct grep_opt *opt,
 	return status;
 }
 
+static void grep_add_color(struct strbuf *sb, const char *escape_seq)
+{
+	size_t orig_len = sb->len;
+
+	while (*escape_seq) {
+		if (*escape_seq == 'm')
+			strbuf_addch(sb, ';');
+		else if (*escape_seq != '\033' && *escape_seq  != '[')
+			strbuf_addch(sb, *escape_seq);
+		escape_seq++;
+	}
+	if (sb->len > orig_len && sb->buf[sb->len - 1] == ';')
+		strbuf_setlen(sb, sb->len - 1);
+}
+
 static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 {
 	int i, nr, argc, hit, len, status;
@@ -357,6 +376,23 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 		push_arg("-e");
 		push_arg(p->pattern);
 	}
+	if (opt->color) {
+		struct strbuf sb = STRBUF_INIT;
+
+		grep_add_color(&sb, opt->color_match);
+		setenv("GREP_COLOR", sb.buf, 1);
+
+		strbuf_reset(&sb);
+		strbuf_addstr(&sb, "mt=");
+		grep_add_color(&sb, opt->color_match);
+		strbuf_addstr(&sb, ":sl=:cx=:fn=:ln=:bn=:se=");
+		setenv("GREP_COLORS", sb.buf, 1);
+
+		strbuf_release(&sb);
+
+		if (opt->color_external && strlen(opt->color_external) > 0)
+			push_arg(opt->color_external);
+	}
 
 	hit = 0;
 	argc = nr;
diff --git a/grep.h b/grep.h
index 73b33ab..a67005d 100644
--- a/grep.h
+++ b/grep.h
@@ -80,6 +80,7 @@ struct grep_opt {
 	unsigned null_following_name:1;
 	int color;
 	char color_match[COLOR_MAXLEN];
+	const char *color_external;
 	int regflags;
 	unsigned pre_context;
 	unsigned post_context;
-- 
1.6.2

Re: [PATCH 0/5] grep: color search patterns

[Nguyen Thai Ngoc Duy]

On 3/7/09, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> Match coloring is a major missing feature of git grep.  It makes reading
>  the output much easier and brings grep onto the same visual level as the
>  other colorized git commands.

"git --color test" did not colorize the result for me. "git --color
--no-ext-grep test" did. Maybe you should ignore external grep unless
it is explicitly requested, like in the last patch. I have very
limited net access these days. Let's see if I can work out something
for tomorrow, unless you beat me to it.
-- 
Duy

Re: [PATCH 0/5] grep: color search patterns

[René Scharfe]

Nguyen Thai Ngoc Duy schrieb:
> On 3/7/09, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
>> Match coloring is a major missing feature of git grep.  It makes reading
>>  the output much easier and brings grep onto the same visual level as the
>>  other colorized git commands.
> 
> "git --color test" did not colorize the result for me. "git --color
> --no-ext-grep test" did. Maybe you should ignore external grep unless
> it is explicitly requested, like in the last patch. I have very
> limited net access these days. Let's see if I can work out something
> for tomorrow, unless you beat me to it.

I assume with "last patch" you mean your last patch, right?  It
automatically switched to internal grep if coloring was turned on.

Patch 5 of my series adds support for coloring external greps, but you
have to explicitly specify the config option grep.color.external.  It
can be set by distributors in the system config file -- they should know
which grep option is needed.

But I can see that a half-colored git grep can be confusing.

René

[PATCH] grep: prefer builtin over external one when coloring results

[pclouds]

As far as I know, not all grep programs support coloring, so we should
rely on builtin grep. If you want external grep, set
color.grep.external to empty string.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 How about this?

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

diff --git a/builtin-grep.c b/builtin-grep.c
index 9e7e766..89489dd 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -825,6 +825,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		}
 	}
 
+	if (opt.color && !opt.color_external)
+		builtin_grep = 1;
 	if (!opt.pattern_list)
 		die("no pattern given.");
 	if ((opt.regflags != REG_NEWLINE) && opt.fixed)
-- 
1.6.1.446.gc7851

Re: [PATCH] grep: prefer builtin over external one when coloring results

[René Scharfe]

pclouds@gmail.com schrieb:
> As far as I know, not all grep programs support coloring, so we should
> rely on builtin grep. If you want external grep, set
> color.grep.external to empty string.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  How about this?
> 
>  builtin-grep.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/builtin-grep.c b/builtin-grep.c
> index 9e7e766..89489dd 100644
> --- a/builtin-grep.c
> +++ b/builtin-grep.c
> @@ -825,6 +825,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  		}
>  	}
>  
> +	if (opt.color && !opt.color_external)
> +		builtin_grep = 1;
>  	if (!opt.pattern_list)
>  		die("no pattern given.");
>  	if ((opt.regflags != REG_NEWLINE) && opt.fixed)

Makes sense, thanks!

René