threads / patch / 35082

patchblame: add color

Subject: [PATCH] blame: add color

## tl;dr

4 messages between Oct 8, 2013 and Oct 8, 2013. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Chris J Arges· Oct 8, 2013, 14:18 UTC · lore

Add colorized text for git blame when color.interactive is enabled. This work is based on the colorization code in builtin/clean.c.

Signed-off-by: Chris J Arges <christopherarges@gmail.com>
---
 builtin/blame.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 2 deletions(-)
Show changes to builtin/blame.c +49 −2
diff --git a/builtin/blame.c b/builtin/blame.c
index 6da7233..fbff437 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -23,6 +23,7 @@
 #include "userdiff.h"
 #include "line-range.h"
 #include "line-log.h"
+#include "color.h"
 
 static char blame_usage[] = N_("git blame [options] [rev-opts] [rev] [--] file");
 
@@ -51,6 +52,24 @@ static size_t blame_date_width;
 
 static struct string_list mailmap;
 
+static int blame_use_color = -1;
+static char blame_colors[][COLOR_MAXLEN] = {
+	GIT_COLOR_RESET,
+	GIT_COLOR_NORMAL,	/* PLAIN */
+	GIT_COLOR_YELLOW,	/* COMMIT */
+	GIT_COLOR_BOLD,		/* NAME */
+	GIT_COLOR_CYAN,		/* LINE */
+	GIT_COLOR_GREEN,	/* TIME */
+};
+enum color_blame {
+	BLAME_COLOR_RESET = 0,
+	BLAME_COLOR_PLAIN = 1,
+	BLAME_COLOR_COMMIT = 2,
+	BLAME_COLOR_NAME = 3,
+	BLAME_COLOR_LINE = 4,
+	BLAME_COLOR_TIME = 5,
+};
+
 #ifndef DEBUG
 #define DEBUG 0
 #endif
@@ -1575,6 +1594,18 @@ static void assign_blame(struct scoreboard *sb, int opt)
 	}
 }
 
+static const char *blame_get_color(enum color_blame ix)
+{
+	if (want_color(blame_use_color))
+		return blame_colors[ix];
+	return "";
+}
+
+static void blame_print_color(enum color_blame ix)
+{
+	printf("%s", blame_get_color(ix));
+}
+
 static const char *format_time(unsigned long time, const char *tz_str,
 			       int show_raw_time)
 {
@@ -1680,7 +1711,9 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 			}
 		}
 
+		blame_print_color(BLAME_COLOR_COMMIT);
 		printf("%.*s", length, hex);
+		blame_print_color(BLAME_COLOR_RESET);
 		if (opt & OUTPUT_ANNOTATE_COMPAT) {
 			const char *name;
 			if (opt & OUTPUT_SHOW_EMAIL)
@@ -1711,14 +1744,22 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 				else
 					name = ci.author.buf;
 				pad = longest_author - utf8_strwidth(name);
-				printf(" (%s%*s %10s",
-				       name, pad, "",
+				blame_print_color(BLAME_COLOR_NAME);
+				printf(" (%s%*s ",
+				       name, pad, "");
+				blame_print_color(BLAME_COLOR_RESET);
+				blame_print_color(BLAME_COLOR_TIME);
+				printf(" (%10s",
 				       format_time(ci.author_time,
 						   ci.author_tz.buf,
 						   show_raw_time));
+
+				blame_print_color(BLAME_COLOR_RESET);
 			}
+			blame_print_color(BLAME_COLOR_LINE);
 			printf(" %*d) ",
 			       max_digits, ent->lno + 1 + cnt);
+			blame_print_color(BLAME_COLOR_RESET);
 		}
 		do {
 			ch = *cp++;
@@ -1948,6 +1989,12 @@ static int git_blame_config(const char *var, const char *value, void *cb)
 		blame_date_mode = parse_date_format(value);
 		return 0;
 	}
+	/* honors the color.interactive* config variables which also
+	   applied in git-add--interactive and git-stash */
+	if (!strcmp(var, "color.interactive")) {
+		blame_use_color = git_config_colorbool(var, value);
+		return 0;
+	}
 
 	if (userdiff_config(var, value) < 0)
 		return -1;

-- 
1.7.9.5
Stefan Beller· Oct 8, 2013, 14:39 UTC · re: Chris J Arges · lore

Re: [PATCH] blame: add color

On 10/08/2013 04:18 PM, Chris J Arges wrote:
> Add colorized text for git blame when color.interactive is enabled.
> This work is based on the colorization code in builtin/clean.c.
>

Another way to color blame, would be by commit, not by row. Try "git gui blame <file>", to see what I mean. The colorisation would be per commit and not by line. Though I guess that's a lot of more work, but I'd personally find it better, for what it's worth.

Show 7 quoted lines
>  
> +static const char *blame_get_color(enum color_blame ix)
> +{
> +	if (want_color(blame_use_color))
> +		return blame_colors[ix];
> +	return "";
> +}

The only occurence of blame_get_color is in blame_print_color, so why not move it in there?

Show 7 quoted lines
> +
> +static void blame_print_color(enum color_blame ix)
> +{
> +	printf("%s", blame_get_color(ix));
> +}
> +
>  static const char *format_time(unsigned long time, const char *tz_str,

Thanks, Stefan

Junio C Hamano· Oct 8, 2013, 15:16 UTC · re: Chris J Arges · lore

Re: [PATCH] blame: add color

Chris J Arges <christopherarges@gmail.com> writes:
> Add colorized text for git blame when color.interactive is enabled.

It does not make any sense to tie this to color.interactive at all, at least to me. The "check color.blame and if absent fall back to color.ui", which is the usual pattern, would be more appropriate.

Show 8 quoted lines
> +static char blame_colors[][COLOR_MAXLEN] = {
> +	GIT_COLOR_RESET,
> +	GIT_COLOR_NORMAL,	/* PLAIN */
> +	GIT_COLOR_YELLOW,	/* COMMIT */
> +	GIT_COLOR_BOLD,		/* NAME */
> +	GIT_COLOR_CYAN,		/* LINE */
> +	GIT_COLOR_GREEN,	/* TIME */
> +};

Unlike "git grep", where some pieces of a single line are more interesting (i.e. the exact text matching the pattern given) than others (i.e. other text on the same line), and "git diff", where some lines have different meanings from others (i.e. hunk header, deleted lines, added lines, context lines), the output from "git blame" is already columnar and it is obvious to the reader of the output that everything on the leftmost part of all lines is commit object name, without distraction of extra colours; I do not see much point in painting the output into vertical stripes of colours.

It may make more sense to assign one colour to each blame origin (i.e. <commit, path> pair), to make it clear that first five lines came from the same origin that is different from the two lines that follow, etc., showing horizontal stripes of colours. If we were to go that route, I suspect that it would be too distracting to paint the whole line (like "git diff" does for added or deleted lines). Perhaps paint only the commit object name part in different colors to show which lines form a group that came from the same origin? The way "git show-branch" paints its output might give an inspiration.

Chris J Arges· Oct 8, 2013, 16:03 UTC · re: Junio C Hamano · lore

Re: [PATCH] blame: add color

On 10/08/2013 10:16 AM, Junio C Hamano wrote:
Show 8 quoted lines
> Chris J Arges <christopherarges@gmail.com> writes:
> 
>> Add colorized text for git blame when color.interactive is enabled.
> 
> It does not make any sense to tie this to color.interactive at all,
> at least to me.  The "check color.blame and if absent fall back to
> color.ui", which is the usual pattern, would be more appropriate.
> 
Ok, I wasn't entirely sure of convention, but I could modify it to do this.
Show 19 quoted lines
>> +static char blame_colors[][COLOR_MAXLEN] = {
>> +	GIT_COLOR_RESET,
>> +	GIT_COLOR_NORMAL,	/* PLAIN */
>> +	GIT_COLOR_YELLOW,	/* COMMIT */
>> +	GIT_COLOR_BOLD,		/* NAME */
>> +	GIT_COLOR_CYAN,		/* LINE */
>> +	GIT_COLOR_GREEN,	/* TIME */
>> +};
> 
> Unlike "git grep", where some pieces of a single line are more
> interesting (i.e. the exact text matching the pattern given) than
> others (i.e. other text on the same line), and "git diff", where
> some lines have different meanings from others (i.e. hunk header,
> deleted lines, added lines, context lines), the output from "git
> blame" is already columnar and it is obvious to the reader of the
> output that everything on the leftmost part of all lines is commit
> object name, without distraction of extra colours; I do not see much
> point in painting the output into vertical stripes of colours.
>

I mainly did this because it makes it easier for me to see where the different columns start and stop.

Show 11 quoted lines
> It may make more sense to assign one colour to each blame origin
> (i.e. <commit, path> pair), to make it clear that first five lines
> came from the same origin that is different from the two lines that
> follow, etc., showing horizontal stripes of colours. If we were to
> go that route, I suspect that it would be too distracting to paint
> the whole line (like "git diff" does for added or deleted
> lines). Perhaps paint only the commit object name part in different
> colors to show which lines form a group that came from the same
> origin?  The way "git show-branch" paints its output might give an
> inspiration.
>

This could provide a more useful colorization; in addition if something as simple as the line number was colorized it would provide a easy way for me to see where the code column starts.

--chris j arges

← back to recent threads