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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 21, 2014, 22:40 UTC
Message-ID: <xmqqoat5hwri.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqqh9yxz0pc.fsf@gitster.dls.corp.google.com>

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

Show 6 quoted lines

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