[PATCH] pretty: colorize pattern matches in commit messages

[PATCH] pretty: colorize pattern matches in commit messages

[Hamza Mahfooz]

Currently, for example when

  git log --grep=pattern

is executed, the outputted commits that are matched by the pattern do not
have the relevant substring matches highlighted.

Signed-off-by: Hamza Mahfooz <someguy@effective-light.com>
---
 pretty.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 98 insertions(+), 11 deletions(-)

diff --git a/pretty.c b/pretty.c
index 9631529c10..2886916ae6 100644
--- a/pretty.c
+++ b/pretty.c
@@ -431,15 +431,80 @@ const char *show_ident_date(const struct ident_split *ident,
 	return show_date(date, tz, mode);
 }
 
+static void append_matched_line(struct grep_opt *opt, const char *line,
+				size_t linelen, enum grep_pat_token token,
+				int field, struct strbuf *sb)
+{
+	struct grep_pat *pat;
+	struct strbuf tmp_sb;
+	regmatch_t tmp_match, match;
+	char *buf, *eol, *color;
+	int cflags = 0;
+
+	strbuf_init(&tmp_sb, linelen + 1);
+	strbuf_add(&tmp_sb, line, linelen);
+	buf = tmp_sb.buf;
+	eol = buf + linelen;
+
+	if (!opt || !want_color(opt->color))
+		goto skip;
+
+	color = opt->colors[GREP_COLOR_MATCH_CONTEXT];
+
+	for (;;) {
+		match.rm_so = match.rm_eo = -1;
+
+		for (pat = (token == GREP_PATTERN_HEAD ?
+			    opt->header_list : opt->pattern_list);
+		     pat; pat = pat->next) {
+			if (pat->token == token &&
+			    (field == -1 || pat->field == field) &&
+			    !regexec(&pat->regexp, buf, 1, &tmp_match,
+				     cflags)) {
+
+				if ((match.rm_so >= 0 && match.rm_eo >= 0) &&
+				    (tmp_match.rm_so > match.rm_so ||
+				     (tmp_match.rm_so == match.rm_so &&
+				      tmp_match.rm_eo < match.rm_eo)))
+					continue;
+
+				match.rm_so = tmp_match.rm_so;
+				match.rm_eo = tmp_match.rm_eo;
+			}
+		}
+
+		if (match.rm_so == match.rm_eo)
+			break;
+
+		strbuf_grow(sb, strlen(color) + strlen(GIT_COLOR_RESET));
+		strbuf_add(sb, buf, match.rm_so);
+		strbuf_add(sb, color, strlen(color));
+		strbuf_add(sb, buf + match.rm_so,
+			   match.rm_eo - match.rm_so);
+		strbuf_add(sb, GIT_COLOR_RESET,
+			   strlen(GIT_COLOR_RESET));
+		buf += match.rm_eo;
+		cflags = REG_NOTBOL;
+	}
+
+skip:
+	strbuf_add(sb, buf, eol - buf);
+
+	strbuf_release(&tmp_sb);
+}
+
 void pp_user_info(struct pretty_print_context *pp,
 		  const char *what, struct strbuf *sb,
 		  const char *line, const char *encoding)
 {
+	struct strbuf id;
 	struct ident_split ident;
 	char *line_end;
 	const char *mailbuf, *namebuf;
 	size_t namelen, maillen;
 	int max_length = 78; /* per rfc2822 */
+	int field = -1;
+	struct grep_opt *opt = pp->rev ? &pp->rev->grep_filter : NULL;
 
 	if (pp->fmt == CMIT_FMT_ONELINE)
 		return;
@@ -496,9 +561,22 @@ void pp_user_info(struct pretty_print_context *pp,
 			strbuf_addch(sb, '\n');
 		strbuf_addf(sb, " <%.*s>\n", (int)maillen, mailbuf);
 	} else {
-		strbuf_addf(sb, "%s: %.*s%.*s <%.*s>\n", what,
-			    (pp->fmt == CMIT_FMT_FULLER) ? 4 : 0, "    ",
-			    (int)namelen, namebuf, (int)maillen, mailbuf);
+		strbuf_init(&id, namelen + maillen + 4);
+
+		if (!strcmp(what, "Author"))
+			field = GREP_HEADER_AUTHOR;
+		else if (!strcmp(what, "Commit"))
+			field = GREP_HEADER_COMMITTER;
+
+		strbuf_addf(sb, "%s: %.*s", what,
+			    (pp->fmt == CMIT_FMT_FULLER) ? 4 : 0, "    ");
+		strbuf_addf(&id, "%.*s <%.*s>", (int)namelen, namebuf,
+			    (int)maillen, mailbuf);
+
+		append_matched_line(opt, id.buf, id.len,
+				    GREP_PATTERN_HEAD, field, sb);
+		strbuf_addch(sb, '\n');
+		strbuf_release(&id);
 	}
 
 	switch (pp->fmt) {
@@ -1855,6 +1933,7 @@ static void pp_header(struct pretty_print_context *pp,
 	}
 }
 
+
 void pp_title_line(struct pretty_print_context *pp,
 		   const char **msg_p,
 		   struct strbuf *sb,
@@ -1935,8 +2014,8 @@ static int pp_utf8_width(const char *start, const char *end)
 	return width;
 }
 
-static void strbuf_add_tabexpand(struct strbuf *sb, int tabwidth,
-				 const char *line, int linelen)
+static void strbuf_add_tabexpand(struct grep_opt *opt, struct strbuf *sb,
+				 int tabwidth, const char *line, int linelen)
 {
 	const char *tab;
 
@@ -1953,7 +2032,8 @@ static void strbuf_add_tabexpand(struct strbuf *sb, int tabwidth,
 			break;
 
 		/* Output the data .. */
-		strbuf_add(sb, line, tab - line);
+		append_matched_line(opt, line, tab - line, GREP_PATTERN_BODY,
+				    -1, sb);
 
 		/* .. and the de-tabified tab */
 		strbuf_addchars(sb, ' ', tabwidth - (width % tabwidth));
@@ -1968,7 +2048,8 @@ static void strbuf_add_tabexpand(struct strbuf *sb, int tabwidth,
 	 * worrying about width - there's nothing more to
 	 * align.
 	 */
-	strbuf_add(sb, line, linelen);
+	append_matched_line(opt, line, linelen,
+			    GREP_PATTERN_BODY, -1, sb);
 }
 
 /*
@@ -1980,11 +2061,14 @@ static void pp_handle_indent(struct pretty_print_context *pp,
 			     struct strbuf *sb, int indent,
 			     const char *line, int linelen)
 {
+	struct grep_opt *opt = pp->rev ? &pp->rev->grep_filter : NULL;
+
 	strbuf_addchars(sb, ' ', indent);
 	if (pp->expand_tabs_in_log)
-		strbuf_add_tabexpand(sb, pp->expand_tabs_in_log, line, linelen);
+		strbuf_add_tabexpand(opt, sb, pp->expand_tabs_in_log, line, linelen);
 	else
-		strbuf_add(sb, line, linelen);
+		append_matched_line(opt, line, linelen, GREP_PATTERN_BODY, -1,
+				    sb);
 }
 
 static int is_mboxrd_from(const char *line, int len)
@@ -2002,7 +2086,9 @@ void pp_remainder(struct pretty_print_context *pp,
 		  struct strbuf *sb,
 		  int indent)
 {
+	struct grep_opt *opt = pp->rev ? &pp->rev->grep_filter : NULL;
 	int first = 1;
+
 	for (;;) {
 		const char *line = *msg_p;
 		int linelen = get_one_line(line);
@@ -2023,14 +2109,15 @@ void pp_remainder(struct pretty_print_context *pp,
 		if (indent)
 			pp_handle_indent(pp, sb, indent, line, linelen);
 		else if (pp->expand_tabs_in_log)
-			strbuf_add_tabexpand(sb, pp->expand_tabs_in_log,
+			strbuf_add_tabexpand(opt, sb, pp->expand_tabs_in_log,
 					     line, linelen);
 		else {
 			if (pp->fmt == CMIT_FMT_MBOXRD &&
 					is_mboxrd_from(line, linelen))
 				strbuf_addch(sb, '>');
 
-			strbuf_add(sb, line, linelen);
+			append_matched_line(opt, line, linelen,
+					    GREP_PATTERN_BODY, -1, sb);
 		}
 		strbuf_addch(sb, '\n');
 	}
-- 
2.33.0

RE: [PATCH] pretty: colorize pattern matches in commit messages

[Felipe Contreras]

Hamza Mahfooz wrote:
> Currently, for example when
> 
>   git log --grep=pattern
> 
> is executed, the outputted commits that are matched by the pattern do not
> have the relevant substring matches highlighted.
> 
> Signed-off-by: Hamza Mahfooz <someguy@effective-light.com>
> ---
>  pretty.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++------

Can you add some tests?

-- 
Felipe Contreras

Re: [PATCH] pretty: colorize pattern matches in commit messages

[Junio C Hamano]

Hamza Mahfooz <someguy@effective-light.com> writes:

> Currently, for example when
>
>   git log --grep=pattern
>
> is executed, the outputted commits that are matched by the pattern do not
> have the relevant substring matches highlighted.

A proposed log message that stops after a description of the current
status alone invites a "so what?" response.  While it may be so
obvious to you why the current state is undesirable (after all, it
motivated you enough to write a patch to improve the situation), the
job of the proposed message is to explain and convince others to
agree what is wrong in the current state and what is a reasonable
design to improve it.  Among these three ingredients, the latter two
are missing from the above.

Because it is our convention to talk about the current status in
present tense first, you do not have to start the description with
"Currently,".

Taking the above two paragraphs together, perhaps:

    The "git log" command limits its output to the commits that
    contain strings that matched the "--grep=<pattern>" option, but
    unlike output from "git grep -e <pattern>", the matches are not
    highlighted, making them harder to spot.

    Teach the pretty-printer code to highlight matches from the
    "--grep=<pattern>", "--author=<pattern>" and
    "--committer=<pattern>" options (to view the last one, you may
    have to ask for --pretty=fuller).

Or something like that.

>  pretty.c | 109 +++++++++++++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 98 insertions(+), 11 deletions(-)

This new feature deserves to be tested.  I am surprised that we do
not have "git log --grep" tests whose expected output we need to
adjust for this change.  Perhaps it is becuase we are not otherwise
showing colors at all in "git log" tests?

It also needs a documentation update, if only to tell readers how to
customize the color used to paint the hits.

> +static void append_matched_line(struct grep_opt *opt, const char *line,
> +				size_t linelen, enum grep_pat_token token,
> +				int field, struct strbuf *sb)
> +{
> +	struct grep_pat *pat;
> +	struct strbuf tmp_sb;
> +	regmatch_t tmp_match, match;
> +	char *buf, *eol, *color;
> +	int cflags = 0;
> +
> +	strbuf_init(&tmp_sb, linelen + 1);
> +	strbuf_add(&tmp_sb, line, linelen);
> +	buf = tmp_sb.buf;
> +	eol = buf + linelen;

This copy of the whole line is wasted when ...

> +	if (!opt || !want_color(opt->color))
> +		goto skip;

... we are not doing any coloring.  Can we avoid it?

> +	color = opt->colors[GREP_COLOR_MATCH_CONTEXT];

Why is the context (as opposed to selected) color used here?  

In general, when you allow the foreground color to be customized,
you must also make the background color customizable, so that the
end user can avoid low contrast combinations.  If we look at how
opt->color is handled in grep.c::show_line(), we can tell how both
match_color (foregroud) and line_color (background) are taken from
the palette that the end user can customize.  We should do the same
here, without assuming that 'color' here will have a good contrast
against the COLOR_RESET backdrop.

> +	for (;;) {
> +		match.rm_so = match.rm_eo = -1;
> +
> +		for (pat = (token == GREP_PATTERN_HEAD ?
> +			    opt->header_list : opt->pattern_list);
> +		     pat; pat = pat->next) {
> +			if (pat->token == token &&
> +			    (field == -1 || pat->field == field) &&
> +			    !regexec(&pat->regexp, buf, 1, &tmp_match,
> +				     cflags)) {
> +
> +				if ((match.rm_so >= 0 && match.rm_eo >= 0) &&
> +				    (tmp_match.rm_so > match.rm_so ||
> +				     (tmp_match.rm_so == match.rm_so &&
> +				      tmp_match.rm_eo < match.rm_eo)))
> +					continue;
> +
> +				match.rm_so = tmp_match.rm_so;
> +				match.rm_eo = tmp_match.rm_eo;
> +			}
> +		}

For the current commit to come this far, "git log --grep=<pattern>"
must have done the above exact regexec() to decide if we need to
call this function in the first place, right?

We must be redoing the same computation here, which is unfortunate
for two reasons.  Performance and maintainability.

How much extra cycles are we looking at with this additional code?
Depending on how inefficient this code makes, we may need to make it
an optional feature, turned off by default.

Worse yet, this can easily become a source of future bugs, since the
above matching logic must be kept in sync with the existing matching
code elsewhere.  I would not be surprised if the above logic is
already broken when various options that affects how pattern
matching works with "log --grep" (e.g. "--invert-grep", "-E", "-i")
are in use.

Also, this function is misnamed.  It is not a function that appends
matched line.  From the caller's point of view, it is used to append
each and every line it wants to add to sb [*], and they do not even
need or want to know how the callee decides which parts of the line
to paint in different colors.  Perhaps append_line_with_color() or
something?

	Side note.  By the way, why is the sb the last parameter to
	this function?  Usually functions that operate on a strbuf
	take it as the first argument.

> +		if (match.rm_so == match.rm_eo)
> +			break;
> +
> +		strbuf_grow(sb, strlen(color) + strlen(GIT_COLOR_RESET));
> +		strbuf_add(sb, buf, match.rm_so);
> +		strbuf_add(sb, color, strlen(color));
> +		strbuf_add(sb, buf + match.rm_so,
> +			   match.rm_eo - match.rm_so);
> +		strbuf_add(sb, GIT_COLOR_RESET,
> +			   strlen(GIT_COLOR_RESET));
> +		buf += match.rm_eo;
> +		cflags = REG_NOTBOL;
> +	}
> +
> +skip:
> +	strbuf_add(sb, buf, eol - buf);
> +
> +	strbuf_release(&tmp_sb);
> +}
> +

I like what the new feature wants to do, but I am not sure if this
is the best execution of the idea (yet).

Thanks.

[PATCH] pretty: colorize pattern matches in commit messages

[Hamza Mahfooz]

The "git log" command limits its output to the commits that contain strings
matched by a pattern when the "--grep=<pattern>" option is used, but unlike
output from "git grep -e <pattern>", the matches are not highlighted,
making them harder to spot.

Teach the pretty-printer code to highlight matches from the
"--grep=<pattern>", "--author=<pattern>" and "--committer=<pattern>"
options (to view the last one, you may have to ask for --pretty=fuller).

Signed-off-by: Hamza Mahfooz <someguy@effective-light.com>
---
v2: make the commit message whole (add the missing ingredients), rename
    append_matched_line() to append_line_with_color(), use
    colors[GREP_COLOR_MATCH_SELECTED] instead of
    colors[GREP_COLOR_MATCH_CONTEXT], allow the background color to be
    customized, don't copy strings to a buffer when not coloring in
    append_line_with_color(), rename next_match() to grep_next_match(),
    repurpose grep_next_match()/match_one_pattern() for use in
    append_line_with_color() (allowing us to remove duplicated matching
    code in append_line_with_color()), document how to customize the
    feature and modify some of the tests to fit the feature better.
---
 Documentation/git-log.txt |   8 +++
 grep.c                    |  41 +++++++++------
 grep.h                    |   3 ++
 pretty.c                  | 108 +++++++++++++++++++++++++++++++++-----
 t/t4202-log.sh            |  42 +++++++++++++++
 5 files changed, 175 insertions(+), 27 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 0498e7bacb..8d08785863 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -232,6 +232,14 @@ notes.displayRef::
 	or `GIT_NOTES_REF`, to read notes from when showing commit
 	messages with the `log` family of commands.  See
 	linkgit:git-notes[1].
+
+color.grep.selected::
+	Determines the non matching text (background) color of selected lines,
+	when `--grep`, `--author` or `--committer` are used.
+
+color.grep.matchSelected::
+	Determines the matching text (foreground) color of selected lines, when
+	`--grep`, `--author` or `--committer` are used.
 +
 May be an unabbreviated ref name or a glob and may be specified
 multiple times.  A warning will be issued for refs that do not exist,
diff --git a/grep.c b/grep.c
index 424a39591b..6b036ee18a 100644
--- a/grep.c
+++ b/grep.c
@@ -956,18 +956,23 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 	const char *start = bol;
 
 	if ((p->token != GREP_PATTERN) &&
-	    ((p->token == GREP_PATTERN_HEAD) != (ctx == GREP_CONTEXT_HEAD)))
+	    ((p->token == GREP_PATTERN_HEAD) != (ctx == GREP_CONTEXT_HEAD)) &&
+	    ((p->token == GREP_PATTERN_BODY) != (ctx == GREP_CONTEXT_BODY)))
 		return 0;
 
 	if (p->token == GREP_PATTERN_HEAD) {
-		const char *field;
-		size_t len;
-		assert(p->field < ARRAY_SIZE(header_field));
-		field = header_field[p->field].field;
-		len = header_field[p->field].len;
-		if (strncmp(bol, field, len))
-			return 0;
-		bol += len;
+		if (!(eflags & REG_NOTBOL)) {
+			const char *field;
+			size_t len;
+
+			assert(p->field < ARRAY_SIZE(header_field));
+			field = header_field[p->field].field;
+			len = header_field[p->field].len;
+			if (strncmp(bol, field, len))
+				return 0;
+			bol += len;
+		}
+
 		switch (p->field) {
 		case GREP_HEADER_AUTHOR:
 		case GREP_HEADER_COMMITTER:
@@ -1159,21 +1164,26 @@ static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,
 	return 1;
 }
 
-static int next_match(struct grep_opt *opt, char *bol, char *eol,
-		      enum grep_context ctx, regmatch_t *pmatch, int eflags)
+int grep_next_match(struct grep_opt *opt, char *bol, char *eol,
+		    enum grep_context ctx, regmatch_t *pmatch,
+		    enum grep_header_field field, 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) {
+		for (p = ((ctx == GREP_CONTEXT_HEAD)
+			   ? opt->header_list : 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);
+				if ((field == GREP_HEADER_FIELD_MAX) || (p->field == field))
+					hit |= match_next_pattern(p, bol, eol,
+								  ctx, pmatch,
+								  eflags);
 				break;
 			default:
 				break;
@@ -1262,7 +1272,8 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 				line_color = opt->colors[GREP_COLOR_FUNCTION];
 		}
 		*eol = '\0';
-		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
+		while (grep_next_match(opt, bol, eol, ctx, &match,
+				       GREP_HEADER_FIELD_MAX, eflags)) {
 			if (match.rm_so == match.rm_eo)
 				break;
 
diff --git a/grep.h b/grep.h
index 72f82b1e30..d2943e29ea 100644
--- a/grep.h
+++ b/grep.h
@@ -177,6 +177,9 @@ void append_header_grep_pattern(struct grep_opt *, enum grep_header_field, const
 void compile_grep_patterns(struct grep_opt *opt);
 void free_grep_patterns(struct grep_opt *opt);
 int grep_buffer(struct grep_opt *opt, char *buf, unsigned long size);
+int grep_next_match(struct grep_opt *opt, char *bol, char *eol,
+		    enum grep_context ctx, regmatch_t *pmatch,
+		    enum grep_header_field field, int eflags);
 
 struct grep_source {
 	char *name;
diff --git a/pretty.c b/pretty.c
index 9631529c10..daf446ee53 100644
--- a/pretty.c
+++ b/pretty.c
@@ -431,15 +431,71 @@ const char *show_ident_date(const struct ident_split *ident,
 	return show_date(date, tz, mode);
 }
 
+static void append_line_with_color(struct strbuf *sb, struct grep_opt *opt,
+				   const char *line, size_t linelen,
+				   int color, enum grep_context ctx,
+				   enum grep_header_field field)
+{
+	char *buf, *eol;
+	const char *line_color, *match_color;
+	regmatch_t match;
+	struct strbuf tmp_sb;
+	int eflags = 0;
+
+	if (!opt || !want_color(color) || opt->invert) {
+		strbuf_add(sb, line, linelen);
+		return;
+	}
+
+	if (ctx == GREP_CONTEXT_HEAD)
+		eflags = REG_NOTBOL;
+
+	strbuf_init(&tmp_sb, linelen + 1);
+	strbuf_add(&tmp_sb, line, linelen);
+
+	buf = tmp_sb.buf;
+	eol = buf + linelen;
+	line_color = opt->colors[GREP_COLOR_SELECTED];
+	match_color = opt->colors[GREP_COLOR_MATCH_SELECTED];
+
+	while (grep_next_match(opt, buf, eol, ctx, &match, field, eflags)) {
+		if (match.rm_so == match.rm_eo)
+			break;
+
+		strbuf_grow(sb, strlen(line_color) + strlen(match_color) +
+			    strlen(GIT_COLOR_RESET));
+		strbuf_add(sb, line_color, strlen(line_color));
+		strbuf_add(sb, buf, match.rm_so);
+		strbuf_add(sb, match_color, strlen(match_color));
+		strbuf_add(sb, buf + match.rm_so,
+			   match.rm_eo - match.rm_so);
+		strbuf_add(sb, GIT_COLOR_RESET,
+			   strlen(GIT_COLOR_RESET));
+		buf += match.rm_eo;
+		eflags = REG_NOTBOL;
+	}
+
+	if (buf != line) {
+		strbuf_grow(sb, strlen(line_color));
+		strbuf_add(sb, line_color, strlen(line_color));
+	}
+
+	strbuf_add(sb, buf, eol - buf);
+	strbuf_release(&tmp_sb);
+}
+
 void pp_user_info(struct pretty_print_context *pp,
 		  const char *what, struct strbuf *sb,
 		  const char *line, const char *encoding)
 {
+	struct strbuf id;
 	struct ident_split ident;
 	char *line_end;
 	const char *mailbuf, *namebuf;
 	size_t namelen, maillen;
 	int max_length = 78; /* per rfc2822 */
+	enum grep_header_field field = GREP_HEADER_FIELD_MAX;
+	struct grep_opt *opt = pp->rev ? &pp->rev->grep_filter : NULL;
 
 	if (pp->fmt == CMIT_FMT_ONELINE)
 		return;
@@ -496,9 +552,22 @@ void pp_user_info(struct pretty_print_context *pp,
 			strbuf_addch(sb, '\n');
 		strbuf_addf(sb, " <%.*s>\n", (int)maillen, mailbuf);
 	} else {
-		strbuf_addf(sb, "%s: %.*s%.*s <%.*s>\n", what,
-			    (pp->fmt == CMIT_FMT_FULLER) ? 4 : 0, "    ",
-			    (int)namelen, namebuf, (int)maillen, mailbuf);
+		strbuf_init(&id, namelen + maillen + 4);
+
+		if (!strcmp(what, "Author"))
+			field = GREP_HEADER_AUTHOR;
+		else if (!strcmp(what, "Commit"))
+			field = GREP_HEADER_COMMITTER;
+
+		strbuf_addf(sb, "%s: %.*s", what,
+			    (pp->fmt == CMIT_FMT_FULLER) ? 4 : 0, "    ");
+		strbuf_addf(&id, "%.*s <%.*s>", (int)namelen, namebuf,
+			    (int)maillen, mailbuf);
+
+		append_line_with_color(sb, opt, id.buf, id.len, pp->color,
+				       GREP_CONTEXT_HEAD, field);
+		strbuf_addch(sb, '\n');
+		strbuf_release(&id);
 	}
 
 	switch (pp->fmt) {
@@ -1855,6 +1924,7 @@ static void pp_header(struct pretty_print_context *pp,
 	}
 }
 
+
 void pp_title_line(struct pretty_print_context *pp,
 		   const char **msg_p,
 		   struct strbuf *sb,
@@ -1935,8 +2005,9 @@ static int pp_utf8_width(const char *start, const char *end)
 	return width;
 }
 
-static void strbuf_add_tabexpand(struct strbuf *sb, int tabwidth,
-				 const char *line, int linelen)
+static void strbuf_add_tabexpand(struct strbuf *sb, struct grep_opt *opt,
+				 int color, int tabwidth, const char *line,
+				 int linelen)
 {
 	const char *tab;
 
@@ -1953,7 +2024,9 @@ static void strbuf_add_tabexpand(struct strbuf *sb, int tabwidth,
 			break;
 
 		/* Output the data .. */
-		strbuf_add(sb, line, tab - line);
+		append_line_with_color(sb, opt, line, tab - line, color,
+				       GREP_CONTEXT_BODY,
+				       GREP_HEADER_FIELD_MAX);
 
 		/* .. and the de-tabified tab */
 		strbuf_addchars(sb, ' ', tabwidth - (width % tabwidth));
@@ -1968,7 +2041,8 @@ static void strbuf_add_tabexpand(struct strbuf *sb, int tabwidth,
 	 * worrying about width - there's nothing more to
 	 * align.
 	 */
-	strbuf_add(sb, line, linelen);
+	append_line_with_color(sb, opt, line, linelen, color, GREP_CONTEXT_BODY,
+			       GREP_HEADER_FIELD_MAX);
 }
 
 /*
@@ -1980,11 +2054,16 @@ static void pp_handle_indent(struct pretty_print_context *pp,
 			     struct strbuf *sb, int indent,
 			     const char *line, int linelen)
 {
+	struct grep_opt *opt = pp->rev ? &pp->rev->grep_filter : NULL;
+
 	strbuf_addchars(sb, ' ', indent);
 	if (pp->expand_tabs_in_log)
-		strbuf_add_tabexpand(sb, pp->expand_tabs_in_log, line, linelen);
+		strbuf_add_tabexpand(sb, opt, pp->color, pp->expand_tabs_in_log,
+				     line, linelen);
 	else
-		strbuf_add(sb, line, linelen);
+		append_line_with_color(sb, opt, line, linelen, pp->color,
+				       GREP_CONTEXT_BODY,
+				       GREP_HEADER_FIELD_MAX);
 }
 
 static int is_mboxrd_from(const char *line, int len)
@@ -2002,7 +2081,9 @@ void pp_remainder(struct pretty_print_context *pp,
 		  struct strbuf *sb,
 		  int indent)
 {
+	struct grep_opt *opt = pp->rev ? &pp->rev->grep_filter : NULL;
 	int first = 1;
+
 	for (;;) {
 		const char *line = *msg_p;
 		int linelen = get_one_line(line);
@@ -2023,14 +2104,17 @@ void pp_remainder(struct pretty_print_context *pp,
 		if (indent)
 			pp_handle_indent(pp, sb, indent, line, linelen);
 		else if (pp->expand_tabs_in_log)
-			strbuf_add_tabexpand(sb, pp->expand_tabs_in_log,
-					     line, linelen);
+			strbuf_add_tabexpand(sb, opt, pp->color,
+					     pp->expand_tabs_in_log, line,
+					     linelen);
 		else {
 			if (pp->fmt == CMIT_FMT_MBOXRD &&
 					is_mboxrd_from(line, linelen))
 				strbuf_addch(sb, '>');
 
-			strbuf_add(sb, line, linelen);
+			append_line_with_color(sb, opt, line, linelen,
+					       pp->color, GREP_CONTEXT_BODY,
+					       GREP_HEADER_FIELD_MAX);
 		}
 		strbuf_addch(sb, '\n');
 	}
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 9dfead936b..72138fa7a0 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -449,6 +449,48 @@ test_expect_success !FAIL_PREREQS 'log with various grep.patternType configurati
 	)
 '
 
+cat > expect << EOF
+Author: ^[[1;31mA U^[[m Thor <author@example.com>
+Author: ^[[1;31mA U^[[m Thor <author@example.com>
+Author: ^[[1;31mA U^[[m Thor <author@example.com>
+Author: ^[[1;31mA U^[[m Thor <author@example.com>
+Author: ^[[1;31mA U^[[m Thor <author@example.com>
+Author: ^[[1;31mA U^[[m Thor <author@example.com>
+Author: ^[[1;31mA U^[[m Thor <author@example.com>
+EOF
+
+test_expect_success 'log --author' '
+	git log --color=always --author="A U" >log &&
+	grep Author log >actual &&
+	test_cmp expect actual
+'
+
+cat > expect << EOF
+Commit:     C O ^[[1;31mMitter^[[m <committer@example.com>
+Commit:     C O ^[[1;31mMitter^[[m <committer@example.com>
+Commit:     C O ^[[1;31mMitter^[[m <committer@example.com>
+Commit:     C O ^[[1;31mMitter^[[m <committer@example.com>
+Commit:     C O ^[[1;31mMitter^[[m <committer@example.com>
+Commit:     C O ^[[1;31mMitter^[[m <committer@example.com>
+Commit:     C O ^[[1;31mMitter^[[m <committer@example.com>
+EOF
+
+test_expect_success 'log --committer' '
+	git log --color=always --pretty=fuller --committer="Mitter" >log &&
+	grep "Commit:" log >actual &&
+	test_cmp expect actual
+'
+
+cat > expect << EOF
+    ^[[1;31msec^[[mond
+EOF
+
+test_expect_success 'log --grep color' '
+	git log --color=always --grep=sec >log &&
+	grep sec log >actual &&
+	test_cmp expect actual
+'
+
 cat > expect <<EOF
 * Second
 * sixth
-- 
2.33.0

Re: [PATCH] pretty: colorize pattern matches in commit messages

[Eric Sunshine]

On Sun, Sep 5, 2021 at 8:05 AM Hamza Mahfooz
<someguy@effective-light.com> wrote:
> [...]
> Teach the pretty-printer code to highlight matches from the
> "--grep=<pattern>", "--author=<pattern>" and "--committer=<pattern>"
> options (to view the last one, you may have to ask for --pretty=fuller).
>
> Signed-off-by: Hamza Mahfooz <someguy@effective-light.com>
> ---
> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> @@ -232,6 +232,14 @@ notes.displayRef::
>         or `GIT_NOTES_REF`, to read notes from when showing commit
>         messages with the `log` family of commands.  See
>         linkgit:git-notes[1].
> +
> +color.grep.selected::
> +       Determines the non matching text (background) color of selected lines,
> +       when `--grep`, `--author` or `--committer` are used.
> +
> +color.grep.matchSelected::
> +       Determines the matching text (foreground) color of selected lines, when
> +       `--grep`, `--author` or `--committer` are used.
>  +
>  May be an unabbreviated ref name or a glob and may be specified
>  multiple times.  A warning will be issued for refs that do not exist,

The paragraph which begins "May be an unabbreviated..." belongs to the
`notes.displayRef` item (which is indicated by the line containing
only a `+` just before the paragraph), so this newly-inserted text
incorrectly divorces the item from the rest of its description. The
two new `color.grep.*` items which this patch adds should be placed
after _all_ the paragraphs connected by `+` lines.