grep: color search patterns

PATCH In review René Scharfe 2 participants 10 messages Jun 15, 2016 Documentation

Share & tools

René Scharfe

author

2016-06-15 22:46 UTC #0 lore raw

cover grep: color search patterns

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.

René Scharfe

author

2016-06-15 22:46 UTC #1 lore raw

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

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 <[email protected]>

---
 grep.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

grep.c +3 -7

index 062b2b6..db341b6 100644
@@ -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)

Signature · 1 line

1.6.2

René Scharfe

author

2016-06-15 22:46 UTC #2 lore raw

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

In reply to 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 <[email protected]>

---
 grep.c |   34 +++++++++++++++++-----------------
 grep.h |    1 +
 2 files changed, 18 insertions(+), 17 deletions(-)

grep.c +17 ~~-17~~

index db341b6..f455182 100644
@@ -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;

grep.h +1 -0

index 5102ce3..d2a8674 100644
@@ -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 {

Signature · 1 line

1.6.2

René Scharfe

author

2016-06-15 22:46 UTC #3 lore raw

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

In reply to 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 <[email protected]>

---
 grep.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

grep.c +10 ~~-11~~

index f455182..bdcff7b 100644
@@ -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;

Signature · 1 line

1.6.2

René Scharfe

author

2016-06-15 22:46 UTC #4 lore raw

PATCH 4/5 grep: color patterns in output

In reply to 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 <[email protected]>

---
 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(-)

Documentation/config.txt +9 -0

index f5152c5..b75dada 100644
@@ -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").

Documentation/git-grep.txt +8 -0

index 553da6c..fccb82d 100644
@@ -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

builtin-grep.c +32 -0

index 3f12ba3..e2c0f01 100644
@@ -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--;

grep.c +78 ~~-12~~

102 lines not on this page · raw message

grep.h +3 -0

index d2a8674..73b33ab 100644
@@ -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;

Signature · 1 line

1.6.2

René Scharfe

author

2016-06-15 22:46 UTC #5 lore raw

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

In reply to 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 <[email protected]>

---
 Documentation/config.txt |   12 +++++++++++-
 builtin-grep.c           |   36 ++++++++++++++++++++++++++++++++++++
 grep.h                   |    1 +
 3 files changed, 48 insertions(+), 1 deletions(-)

Documentation/config.txt +11 -1

index b75dada..4d42bff 100644
@@ -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

builtin-grep.c +36 -0

index e2c0f01..9e7e766 100644
@@ -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;

grep.h +1 -0

index 73b33ab..a67005d 100644
@@ -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;

Signature · 1 line

1.6.2

Nguyen Thai Ngoc Duy

2016-06-15 22:46 UTC #6 lore raw

In reply to René Scharfe

Quoted 3 lines · 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.

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

Signature · 1 line

Duy

René Scharfe

author

2016-06-15 22:46 UTC #7 lore raw

Quoted 10 lines · Nguyen Thai Ngoc Duy

Quoted 3 lines · 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.
"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é

[email protected]

2016-06-15 22:46 UTC #8 lore raw

PATCH 1/1 grep: prefer builtin over external one when coloring results

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 <[email protected]>

---
How about this?

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

builtin-grep.c +2 -0

index 9e7e766..89489dd 100644
@@ -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)

Signature · 1 line

1.6.1.446.gc7851

René Scharfe

author

2016-06-15 22:46 UTC #9 lore raw

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

Quoted 16 lines · [email protected] 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 <[email protected]>
---
How about this?
 builtin-grep.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
builtin-grep.c +0 -0
index 9e7e766..89489dd 100644

builtin-grep.c int cmd_grep(int argc, const char **argv, const char *prefix) · line 832

Quoted 8 lines

builtin-grep.c +2 -0

@@ -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é