[PATCH] grep: fix match highlighting for combined patterns with context lines

[PATCH] grep: fix match highlighting for combined patterns with context lines

[Zoltan Klinger]

When git grep is run with combined patterns such as '-e p1 --and -e p2'
and surrounding context lines are requested, the output contains
incorrectly highlighted matches.

Consider the following output (highlighted matches are surrounded by '*'
characters):
    $ cat testfile
    foo a
    foo b
    foo bar
    baz bar foo
    bar x
    bar y
    $ git grep -n -C2 -e foo --and -e bar testfile
    testfile-1-*foo* a
    testfile-2-*foo* b
    testfile:3:*foo* *bar*
    testfile:4:baz *bar* *foo*
    testfile-5-*bar* x
    testfile-6-*bar* y

Lines 1, 2, 5 and 6 do not match the combined patterns, they only
contain incorrectly highlighted 'false positives'.

Modify the show_line() function in grep.c to highlight matches only on
lines that match the combined pattern. Do not highlight matches on lines
that provide only context or contain only the function name of the match.

The output of the same command after the change:
    $ git grep -n -C2 -e foo --and -e bar testfile
    testfile-1-foo a
    testfile-2-foo b
    testfile:3:*foo* *bar*
    testfile:4:baz *bar* *foo*
    testfile-5-bar x
    testfile-6-bar y

Signed-off-by: Zoltan Klinger <zoltan.klinger@gmail.com>
---
 grep.c          |  7 +++--
 t/t7810-grep.sh | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 95 insertions(+), 2 deletions(-)

diff --git a/grep.c b/grep.c
index 4dc31ea..3c4d68e 100644
--- a/grep.c
+++ b/grep.c
@@ -1121,9 +1121,12 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		enum grep_context ctx = GREP_CONTEXT_BODY;
 		int ch = *eol;
 		int eflags = 0;
+		char *match_color = NULL;
 
-		if (sign == ':')
+		if (sign == ':') {
 			line_color = opt->color_selected;
+			match_color = opt->color_match;
+		}
 		else if (sign == '-')
 			line_color = opt->color_context;
 		else if (sign == '=')
@@ -1136,7 +1139,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_color);
 			bol += match.rm_eo;
 			rest -= match.rm_eo;
 			eflags = REG_NOTBOL;
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 40615de..b0d6b6f 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1202,4 +1202,94 @@ 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.match		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.match		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.match		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.match		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
-- 
2.1.1

Re: [PATCH] grep: fix match highlighting for combined patterns with context lines

[Junio C Hamano]

Zoltan Klinger <zoltan.klinger@gmail.com> writes:

> When git grep is run with combined patterns such as '-e p1 --and -e p2'
> and surrounding context lines are requested, the output contains
> incorrectly highlighted matches.
>
> Consider the following output (highlighted matches are surrounded by '*'
> characters):
>     $ cat testfile
>     foo a
>     foo b
>     foo bar
>     baz bar foo
>     bar x
>     bar y
>     $ git grep -n -C2 -e foo --and -e bar testfile
>     testfile-1-*foo* a
>     testfile-2-*foo* b
>     testfile:3:*foo* *bar*
>     testfile:4:baz *bar* *foo*
>     testfile-5-*bar* x
>     testfile-6-*bar* y
>
> Lines 1, 2, 5 and 6 do not match the combined patterns, they only
> contain incorrectly highlighted 'false positives'.
>
> Modify the show_line() function in grep.c to highlight matches only on
> lines that match the combined pattern. Do not highlight matches on lines
> that provide only context or contain only the function name of the match.
>
> The output of the same command after the change:
>     $ git grep -n -C2 -e foo --and -e bar testfile
>     testfile-1-foo a
>     testfile-2-foo b
>     testfile:3:*foo* *bar*
>     testfile:4:baz *bar* *foo*
>     testfile-5-bar x
>     testfile-6-bar y

If your goal is to stop colouring words on context and other kinds
of lines, do you still need the "while (next_match(...))" loop for
them?  Can't you make the resulting code clearer by restructuring
the inside of the whole "if (opt->color)" block further, something
along the lines of...

	if (sign != ':') {
		regmatch_t match; ...
		enum grep_context ctx = GREP_CONTEXT_BODY;
                ...
        	while (next_match(...)) {
                	... the "word-by-word" loop ...
		}
	} else {
        	switch (sign) {
		case '-':
                       	line_color = opt->color_context;
                        break;
		case ':':
                       	line_color = opt->color_function;
                        break;
		}
                output_color(opt, bol, ..., line_color);
	}

Hmm?

Re: [PATCH] grep: fix match highlighting for combined patterns with context lines

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> If your goal is to stop colouring words on context and other kinds
> of lines, do you still need the "while (next_match(...))" loop for
> them?  Can't you make the resulting code clearer by restructuring
> the inside of the whole "if (opt->color)" block further, something
> along the lines of...
> Hmm?

It turns out that the result of such a change becomes more readable
than the original, in that it makes it clear that reinspection of
the lines are done only for matched ones and not context lines.

The diff looks unnecessarily noisy because it indents the while ()
loop that is only needed for sign == ':', though.

 grep.c | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/grep.c b/grep.c
index c668034..b363a94 100644
--- a/grep.c
+++ b/grep.c
@@ -1112,31 +1112,33 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		output_sep(opt, sign);
 	}
 	if (opt->color) {
-		regmatch_t match;
-		enum grep_context ctx = GREP_CONTEXT_BODY;
-		int ch = *eol;
-		int eflags = 0;
+		if (sign == ':') {
+			/* paint the hits on matched lines */
+			regmatch_t match;
+			enum grep_context ctx = GREP_CONTEXT_BODY;
+			int ch = *eol;
+			int eflags = 0;
 
-		if (sign == ':')
 			line_color = opt->color_selected;
-		else if (sign == '-')
+			*eol = '\0';
+			while (next_match(opt, bol, eol, ctx, &match, eflags)) {
+				if (match.rm_so == match.rm_eo)
+					break;
+
+				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);
+				bol += match.rm_eo;
+				rest -= match.rm_eo;
+				eflags = REG_NOTBOL;
+			}
+			*eol = ch;
+		} else if (sign == '-') {
 			line_color = opt->color_context;
-		else if (sign == '=')
+		} else if (sign == '=') {
 			line_color = opt->color_function;
-		*eol = '\0';
-		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
-			if (match.rm_so == match.rm_eo)
-				break;
-
-			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);
-			bol += match.rm_eo;
-			rest -= match.rm_eo;
-			eflags = REG_NOTBOL;
 		}
-		*eol = ch;
 	}
 	output_color(opt, bol, rest, line_color);
 	opt->output(opt, "\n", 1);

Re: [PATCH] grep: fix match highlighting for combined patterns with context lines

[Zoltan Klinger]

> Junio C Hamano <gitster@pobox.com> writes:
>
> It turns out that the result of such a change becomes more readable
> than the original, in that it makes it clear that reinspection of
> the lines are done only for matched ones and not context lines.
>
>
Agree, it looks much clearer now. Happy if you squashed your
change (commit da736e6) in zk/grep-color-words branch.

Re: [PATCH] grep: fix match highlighting for combined patterns with context lines

[Junio C Hamano]

Zoltan Klinger <zoltan.klinger@gmail.com> writes:

>> Junio C Hamano <gitster@pobox.com> writes:
>>
>> It turns out that the result of such a change becomes more readable
>> than the original, in that it makes it clear that reinspection of
>> the lines are done only for matched ones and not context lines.
>>
>>
> Agree, it looks much clearer now. Happy if you squashed your
> change (commit da736e6) in zk/grep-color-words branch.

OK, will do.  Thanks.

Re: [PATCH] grep: fix match highlighting for combined patterns with context lines

[René Scharfe]

Am 21.10.2014 um 07:56 schrieb Zoltan Klinger:
> When git grep is run with combined patterns such as '-e p1 --and -e p2'
> and surrounding context lines are requested, the output contains
> incorrectly highlighted matches.
>
> Consider the following output (highlighted matches are surrounded by '*'
> characters):
>      $ cat testfile
>      foo a
>      foo b
>      foo bar
>      baz bar foo
>      bar x
>      bar y
>      $ git grep -n -C2 -e foo --and -e bar testfile
>      testfile-1-*foo* a
>      testfile-2-*foo* b
>      testfile:3:*foo* *bar*
>      testfile:4:baz *bar* *foo*
>      testfile-5-*bar* x
>      testfile-6-*bar* y
>
> Lines 1, 2, 5 and 6 do not match the combined patterns, they only
> contain incorrectly highlighted 'false positives'.

The old code highlights all search terms, anywhere. I wouldn't call the 
ones in the context lines false positives.  The user might be interested 
in those occurrences as well (I know I am ;).

GNU grep allows coloring to be configured in much greater detail with 
its GREP_COLORS variable.  I didn't think that level of tuning is 
desirable until now.  What your patch does is equivalent to change the 
default of "ms=01;31:mc=01;31" (color matching string in selected lines 
and context lines) to "ms=01;31:mc=" (color matching string in selected 
lines).

The difference is only visible with -v or git grep's --not and --and.

So, if you really don't want matching string in context lines to be 
colored, perhaps it's time to add a color.grep.contextmatch for matching 
text in context lines?

René

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

[René Scharfe]

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 <l.s.r@web.de>
---
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?

 Documentation/config.txt |  6 +++++-
 grep.c                   | 29 ++++++++++++++++++++++-------
 grep.h                   |  3 ++-
 3 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 8b49813..78832ae 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -885,7 +885,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 4dc31ea..6e085f8 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;
@@ -101,12 +102,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);
+		rc |= color_parse(value, opt->color_match_context);
+		rc |= color_parse(value, opt->color_match_selected);
+		return rc;
+	}
 
 	if (color) {
 		if (!value)
@@ -144,7 +155,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);
 }
@@ -1084,7 +1096,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)
@@ -1123,6 +1135,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;
@@ -1135,8 +1151,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;
-- 
2.1.2

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

[Junio C Hamano]

René Scharfe <l.s.r@web.de> 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 <l.s.r@web.de>
> ---
> 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.

Zoltan?



>  Documentation/config.txt |  6 +++++-
>  grep.c                   | 29 ++++++++++++++++++++++-------
>  grep.h                   |  3 ++-
>  3 files changed, 29 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 8b49813..78832ae 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -885,7 +885,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 4dc31ea..6e085f8 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;
> @@ -101,12 +102,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);
> +		rc |= color_parse(value, opt->color_match_context);
> +		rc |= color_parse(value, opt->color_match_selected);
> +		return rc;
> +	}
>  
>  	if (color) {
>  		if (!value)
> @@ -144,7 +155,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);
>  }
> @@ -1084,7 +1096,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)
> @@ -1123,6 +1135,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;
> @@ -1135,8 +1151,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;

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> René Scharfe <l.s.r@web.de> 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 <l.s.r@web.de>
>> ---
>> 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

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

[Zoltan Klinger]

I like René's approach, too. It's more flexible, supports the old
behaviour and it scratches my itch as well.
Don't mind if you dropped my patch and used René's instead.

>> 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.
>
> Zoltan?

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

[Junio C Hamano]

Zoltan Klinger <zoltan.klinger@gmail.com> writes:

> I like René's approach, too. It's more flexible, supports the old
> behaviour and it scratches my itch as well.

OK, then let's go with that one.

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

[René Scharfe]

Am 28.10.2014 um 00:32 schrieb Zoltan Klinger:
> I like René's approach, too. It's more flexible, supports the old
> behaviour and it scratches my itch as well.
> Don't mind if you dropped my patch and used René's instead.


Good. :)  And here's the t/ part of your patch, slightly changed to
exercise the new config options.

---
 t/t7810-grep.sh | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 40615de..5d3e161 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -1202,4 +1202,97 @@ 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.match		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 <GREEN>with <RESET>leading space1
+space: line <RED>with <RESET>leading <RED>space2<RESET>
+space- line <GREEN>with <RESET>leading space3
+space-line without leading <GREEN>space2<RESET>
+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	green &&
+	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 <GREEN>with <RESET>leading <GREEN>space2<RESET>
+space: line <RED>with <RESET>leading space3
+space-line without leading <GREEN>space2<RESET>
+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	green &&
+	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=<GREEN>int<RESET> main(<GREEN>int<RESET> 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	green &&
+	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
-- 
2.1.2