[PATCH] blame: add color

[PATCH] blame: add color

[Chris J Arges]

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

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

Re: [PATCH] blame: add color

[Stefan Beller]

[-- Attachment #1: Type: text/plain, Size: 930 bytes --]

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.

>  
> +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?

> +
> +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



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

Re: [PATCH] blame: add color

[Junio C Hamano]

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.

> +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.

Re: [PATCH] blame: add color

[Chris J Arges]

On 10/08/2013 10:16 AM, Junio C Hamano wrote:
> 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.

>> +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.

> 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