Subject: Re: [PATCH][RFC] grep: add color.grep.matchcontext and color.grep.matchselected

From: Junio C Hamano <[email-masked]>
Date: Mon, 27 Oct 2014 12:47:14 -0700

Junio C Hamano <[email-masked]> writes:

> René Scharfe <[email-masked]> writes:
>
>> The config option color.grep.match can be used to specify the highlighting
>> color for matching strings.  Add the options matchContext and matchSelected
>> to allow different colors to be specified for matching strings in the
>> context vs. in selected lines.  This is similar to the ms and mc specifiers
>> in GNU grep's environment variable GREP_COLORS.
>>
>> Signed-off-by: Rene Scharfe <[email-masked]>
>> ---
>> Only *very* lightly tested, and a test for t/is missing anyway.  Just
>> wanted to quickly show what I meant.  You'd set color.grep.matchContext=""
>> to turn off highlighting in context lines.  What do you think?
>
> I didn't realize that people wanted to see pieces on non-matching
> lines highlighted.  It makes certain sense, e.g. it would allow you
> to spot near-misses, but that is only true for lines that neighbour
> real hits, so...
>
> I like this approach better in that it makes those who want a
> different behaviour to do the work without breaking the expectation
> of those who are used to the established behaviour.

FWIW, here is a backport on top of maint-1.8.5 with Zoltan's tests,
not because I wanted to apply this as a bugfix to maintenance track,
but because I wanted to compare with what has been queued already.

To apply to post f6c5a296 (color_parse: do not mention variable name
in error message, 2014-10-07) codebase, the mangling I did for two
calls to color_parse() function needs to be undone, obviously.

 Documentation/config.txt |  6 +++-
 grep.c                   | 29 +++++++++++----
 grep.h                   |  3 +-
 t/t7810-grep.sh          | 94 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 123 insertions(+), 9 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index ab26963..aa881fc 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -860,7 +860,11 @@ color.grep.<slot>::
 `linenumber`;;
 	line number prefix (when using `-n`)
 `match`;;
-	matching text
+	matching text (same as setting `matchContext` and `matchSelected`)
+`matchContext`;;
+	matching text in context lines
+`matchSelected`;;
+	matching text in selected lines
 `selected`;;
 	non-matching text in selected lines
 `separator`;;
diff --git a/grep.c b/grep.c
index c668034..f950651 100644
--- a/grep.c
+++ b/grep.c
@@ -35,7 +35,8 @@ void init_grep_defaults(void)
 	strcpy(opt->color_filename, "");
 	strcpy(opt->color_function, "");
 	strcpy(opt->color_lineno, "");
-	strcpy(opt->color_match, GIT_COLOR_BOLD_RED);
+	strcpy(opt->color_match_context, GIT_COLOR_BOLD_RED);
+	strcpy(opt->color_match_selected, GIT_COLOR_BOLD_RED);
 	strcpy(opt->color_selected, "");
 	strcpy(opt->color_sep, GIT_COLOR_CYAN);
 	opt->color = -1;
@@ -96,12 +97,22 @@ int grep_config(const char *var, const char *value, void *cb)
 		color = opt->color_function;
 	else if (!strcmp(var, "color.grep.linenumber"))
 		color = opt->color_lineno;
-	else if (!strcmp(var, "color.grep.match"))
-		color = opt->color_match;
+	else if (!strcmp(var, "color.grep.matchcontext"))
+		color = opt->color_match_context;
+	else if (!strcmp(var, "color.grep.matchselected"))
+		color = opt->color_match_selected;
 	else if (!strcmp(var, "color.grep.selected"))
 		color = opt->color_selected;
 	else if (!strcmp(var, "color.grep.separator"))
 		color = opt->color_sep;
+	else if (!strcmp(var, "color.grep.match")) {
+		int rc = 0;
+		if (!value)
+			return config_error_nonbool(var);
+		color_parse(value, var, opt->color_match_context);
+		color_parse(value, var, opt->color_match_selected);
+		return rc;
+	}
 
 	if (color) {
 		if (!value)
@@ -139,7 +150,8 @@ void grep_init(struct grep_opt *opt, const char *prefix)
 	strcpy(opt->color_filename, def->color_filename);
 	strcpy(opt->color_function, def->color_function);
 	strcpy(opt->color_lineno, def->color_lineno);
-	strcpy(opt->color_match, def->color_match);
+	strcpy(opt->color_match_context, def->color_match_context);
+	strcpy(opt->color_match_selected, def->color_match_selected);
 	strcpy(opt->color_selected, def->color_selected);
 	strcpy(opt->color_sep, def->color_sep);
 }
@@ -1079,7 +1091,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		      const char *name, unsigned lno, char sign)
 {
 	int rest = eol - bol;
-	char *line_color = NULL;
+	const char *match_color, *line_color = NULL;
 
 	if (opt->file_break && opt->last_shown == 0) {
 		if (opt->show_hunk_mark)
@@ -1118,6 +1130,10 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		int eflags = 0;
 
 		if (sign == ':')
+			match_color = opt->color_match_selected;
+		else
+			match_color = opt->color_match_context;
+		if (sign == ':')
 			line_color = opt->color_selected;
 		else if (sign == '-')
 			line_color = opt->color_context;
@@ -1130,8 +1146,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 
 			output_color(opt, bol, match.rm_so, line_color);
 			output_color(opt, bol + match.rm_so,
-				     match.rm_eo - match.rm_so,
-				     opt->color_match);
+				     match.rm_eo - match.rm_so, match_color);
 			bol += match.rm_eo;
 			rest -= match.rm_eo;
 			eflags = REG_NOTBOL;
diff --git a/grep.h b/grep.h
index eaaced1..95f197a 100644
--- a/grep.h
+++ b/grep.h
@@ -124,7 +124,8 @@ struct grep_opt {
 	char color_filename[COLOR_MAXLEN];
 	char color_function[COLOR_MAXLEN];
 	char color_lineno[COLOR_MAXLEN];
-	char color_match[COLOR_MAXLEN];
+	char color_match_context[COLOR_MAXLEN];
+	char color_match_selected[COLOR_MAXLEN];
 	char color_selected[COLOR_MAXLEN];
 	char color_sep[COLOR_MAXLEN];
 	int regflags;
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index f698001..e3eeaf9 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1195,4 +1195,98 @@ test_expect_success LIBPCRE 'grep -P "^ "' '
 	test_cmp expected actual
 '
 
+cat >expected <<EOF
+space-line without leading space1
+space: line <RED>with <RESET>leading space1
+space: line <RED>with <RESET>leading <RED>space2<RESET>
+space: line <RED>with <RESET>leading space3
+space:line without leading <RED>space2<RESET>
+EOF
+
+test_expect_success 'grep --color -e A -e B with context' '
+	test_config color.grep.context		normal &&
+	test_config color.grep.filename		normal &&
+	test_config color.grep.function		normal &&
+	test_config color.grep.linenumber	normal &&
+	test_config color.grep.matchContext	normal &&
+	test_config color.grep.matchSelected	red &&
+	test_config color.grep.selected		normal &&
+	test_config color.grep.separator	normal &&
+
+	git grep --color=always -C2 -e "with " -e space2  space |
+	test_decode_color >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<EOF
+space-line without leading space1
+space- line with leading space1
+space: line <RED>with <RESET>leading <RED>space2<RESET>
+space- line with leading space3
+space-line without leading space2
+EOF
+
+test_expect_success 'grep --color -e A --and -e B with context' '
+	test_config color.grep.context		normal &&
+	test_config color.grep.filename		normal &&
+	test_config color.grep.function		normal &&
+	test_config color.grep.linenumber	normal &&
+	test_config color.grep.matchContext	normal &&
+	test_config color.grep.matchSelected	red &&
+	test_config color.grep.selected		normal &&
+	test_config color.grep.separator	normal &&
+
+	git grep --color=always -C2 -e "with " --and -e space2  space |
+	test_decode_color >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<EOF
+space-line without leading space1
+space: line <RED>with <RESET>leading space1
+space- line with leading space2
+space: line <RED>with <RESET>leading space3
+space-line without leading space2
+EOF
+
+test_expect_success 'grep --color -e A --and --not -e B with context' '
+	test_config color.grep.context		normal &&
+	test_config color.grep.filename		normal &&
+	test_config color.grep.function		normal &&
+	test_config color.grep.linenumber	normal &&
+	test_config color.grep.matchContext	normal &&
+	test_config color.grep.matchSelected	red &&
+	test_config color.grep.selected		normal &&
+	test_config color.grep.separator	normal &&
+
+	git grep --color=always -C2 -e "with " --and --not -e space2  space |
+	test_decode_color >actual &&
+	test_cmp expected actual
+'
+
+cat >expected <<EOF
+hello.c-#include <stdio.h>
+hello.c=int main(int argc, const char **argv)
+hello.c-{
+hello.c:	pr<RED>int<RESET>f("<RED>Hello<RESET> world.\n");
+hello.c-	return 0;
+hello.c-	/* char ?? */
+hello.c-}
+EOF
+
+test_expect_success 'grep --color -e A --and -e B -p with context' '
+	test_config color.grep.context		normal &&
+	test_config color.grep.filename		normal &&
+	test_config color.grep.function		normal &&
+	test_config color.grep.linenumber	normal &&
+	test_config color.grep.matchContext	normal &&
+	test_config color.grep.matchSelected	red &&
+	test_config color.grep.selected		normal &&
+	test_config color.grep.separator	normal &&
+
+	git grep --color=always -p -C3 -e int --and -e Hello --no-index hello.c |
+	test_decode_color >actual &&
+	test_cmp expected actual
+'
+
 test_done