{"thread":{"id":"35082","subject":"[PATCH] blame: add color","startedAt":"2013-10-08T14:18:49Z","lastAt":"2013-10-08T16:03:01Z","messageCount":4,"participants":["Chris J Arges","Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"228610","messageId":"1381241929-23378-1-git-send-email-christopherarges@gmail.com","threadId":"35082","inReplyTo":null,"subject":"[PATCH] blame: add color","fromName":"Chris J Arges","fromEmail":"christopherarges@gmail.com","sentAt":"2013-10-08T14:18:49Z","receivedAt":"2013-10-08T14:18:49Z","isPatch":true,"sender":{"key":"christopherarges@gmail.com","avatar":null},"body":"Add colorized text for git blame when color.interactive is enabled.\nThis work is based on the colorization code in builtin/clean.c.\n\nSigned-off-by: Chris J Arges \n---\n builtin/blame.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++--\n 1 file changed, 49 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 6da7233..fbff437 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -23,6 +23,7 @@\n #include \"userdiff.h\"\n #include \"line-range.h\"\n #include \"line-log.h\"\n+#include \"color.h\"\n \n static char blame_usage[] = N_(\"git blame [options] [rev-opts] [rev] [--] file\");\n \n@@ -51,6 +52,24 @@ static size_t blame_date_width;\n \n static struct string_list mailmap;\n \n+static int blame_use_color = -1;\n+static char blame_colors[][COLOR_MAXLEN] = {\n+\tGIT_COLOR_RESET,\n+\tGIT_COLOR_NORMAL,\t/* PLAIN */\n+\tGIT_COLOR_YELLOW,\t/* COMMIT */\n+\tGIT_COLOR_BOLD,\t\t/* NAME */\n+\tGIT_COLOR_CYAN,\t\t/* LINE */\n+\tGIT_COLOR_GREEN,\t/* TIME */\n+};\n+enum color_blame {\n+\tBLAME_COLOR_RESET = 0,\n+\tBLAME_COLOR_PLAIN = 1,\n+\tBLAME_COLOR_COMMIT = 2,\n+\tBLAME_COLOR_NAME = 3,\n+\tBLAME_COLOR_LINE = 4,\n+\tBLAME_COLOR_TIME = 5,\n+};\n+\n #ifndef DEBUG\n #define DEBUG 0\n #endif\n@@ -1575,6 +1594,18 @@ static void assign_blame(struct scoreboard *sb, int opt)\n \t}\n }\n \n+static const char *blame_get_color(enum color_blame ix)\n+{\n+\tif (want_color(blame_use_color))\n+\t\treturn blame_colors[ix];\n+\treturn \"\";\n+}\n+\n+static void blame_print_color(enum color_blame ix)\n+{\n+\tprintf(\"%s\", blame_get_color(ix));\n+}\n+\n static const char *format_time(unsigned long time, const char *tz_str,\n \t\t\t int show_raw_time)\n {\n@@ -1680,7 +1711,9 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)\n \t\t\t}\n \t\t}\n \n+\t\tblame_print_color(BLAME_COLOR_COMMIT);\n \t\tprintf(\"%.*s\", length, hex);\n+\t\tblame_print_color(BLAME_COLOR_RESET);\n \t\tif (opt & OUTPUT_ANNOTATE_COMPAT) {\n \t\t\tconst char *name;\n \t\t\tif (opt & OUTPUT_SHOW_EMAIL)\n@@ -1711,14 +1744,22 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)\n \t\t\t\telse\n \t\t\t\t\tname = ci.author.buf;\n \t\t\t\tpad = longest_author - utf8_strwidth(name);\n-\t\t\t\tprintf(\" (%s%*s %10s\",\n-\t\t\t\t name, pad, \"\",\n+\t\t\t\tblame_print_color(BLAME_COLOR_NAME);\n+\t\t\t\tprintf(\" (%s%*s \",\n+\t\t\t\t name, pad, \"\");\n+\t\t\t\tblame_print_color(BLAME_COLOR_RESET);\n+\t\t\t\tblame_print_color(BLAME_COLOR_TIME);\n+\t\t\t\tprintf(\" (%10s\",\n \t\t\t\t format_time(ci.author_time,\n \t\t\t\t\t\t ci.author_tz.buf,\n \t\t\t\t\t\t show_raw_time));\n+\n+\t\t\t\tblame_print_color(BLAME_COLOR_RESET);\n \t\t\t}\n+\t\t\tblame_print_color(BLAME_COLOR_LINE);\n \t\t\tprintf(\" %*d) \",\n \t\t\t max_digits, ent->lno + 1 + cnt);\n+\t\t\tblame_print_color(BLAME_COLOR_RESET);\n \t\t}\n \t\tdo {\n \t\t\tch = *cp++;\n@@ -1948,6 +1989,12 @@ static int git_blame_config(const char *var, const char *value, void *cb)\n \t\tblame_date_mode = parse_date_format(value);\n \t\treturn 0;\n \t}\n+\t/* honors the color.interactive* config variables which also\n+\t applied in git-add--interactive and git-stash */\n+\tif (!strcmp(var, \"color.interactive\")) {\n+\t\tblame_use_color = git_config_colorbool(var, value);\n+\t\treturn 0;\n+\t}\n \n \tif (userdiff_config(var, value) < 0)\n \t\treturn -1;\n-- \n1.7.9.5\n"},{"id":"228613","messageId":"52541927.5010206@googlemail.com","threadId":"35082","inReplyTo":"1381241929-23378-1-git-send-email-christopherarges@gmail.com","subject":"Re: [PATCH] blame: add color","fromName":"Stefan Beller","fromEmail":"stefanbeller@googlemail.com","sentAt":"2013-10-08T14:39:35Z","receivedAt":"2013-10-08T14:39:35Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On 10/08/2013 04:18 PM, Chris J Arges wrote:\n> Add colorized text for git blame when color.interactive is enabled.\n> This work is based on the colorization code in builtin/clean.c.\n> \n\nAnother way to color blame, would be by commit, not by row.\nTry \"git gui blame \", to see what I mean. The colorisation\nwould be per commit and not by line.\nThough I guess that's a lot of more work, but I'd personally find it\nbetter, for what it's worth.\n\n> \n> +static const char *blame_get_color(enum color_blame ix)\n> +{\n> +\tif (want_color(blame_use_color))\n> +\t\treturn blame_colors[ix];\n> +\treturn \"\";\n> +}\nThe only occurence of blame_get_color is in blame_print_color, so why\nnot move it in there?\n\n> +\n> +static void blame_print_color(enum color_blame ix)\n> +{\n> +\tprintf(\"%s\", blame_get_color(ix));\n> +}\n> +\n> static const char *format_time(unsigned long time, const char *tz_str,\n\nThanks,\nStefan\n\n\n"},{"id":"228615","messageId":"xmqqob6zn67d.fsf@gitster.dls.corp.google.com","threadId":"35082","inReplyTo":"1381241929-23378-1-git-send-email-christopherarges@gmail.com","subject":"Re: [PATCH] blame: add color","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-10-08T15:16:38Z","receivedAt":"2013-10-08T15:16:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Chris J Arges writes:\n\n> Add colorized text for git blame when color.interactive is enabled.\n\nIt does not make any sense to tie this to color.interactive at all,\nat least to me. The \"check color.blame and if absent fall back to\ncolor.ui\", which is the usual pattern, would be more appropriate.\n\n> +static char blame_colors[][COLOR_MAXLEN] = {\n> +\tGIT_COLOR_RESET,\n> +\tGIT_COLOR_NORMAL,\t/* PLAIN */\n> +\tGIT_COLOR_YELLOW,\t/* COMMIT */\n> +\tGIT_COLOR_BOLD,\t\t/* NAME */\n> +\tGIT_COLOR_CYAN,\t\t/* LINE */\n> +\tGIT_COLOR_GREEN,\t/* TIME */\n> +};\n\nUnlike \"git grep\", where some pieces of a single line are more\ninteresting (i.e. the exact text matching the pattern given) than\nothers (i.e. other text on the same line), and \"git diff\", where\nsome lines have different meanings from others (i.e. hunk header,\ndeleted lines, added lines, context lines), the output from \"git\nblame\" is already columnar and it is obvious to the reader of the\noutput that everything on the leftmost part of all lines is commit\nobject name, without distraction of extra colours; I do not see much\npoint in painting the output into vertical stripes of colours.\n\nIt may make more sense to assign one colour to each blame origin\n(i.e. pair), to make it clear that first five lines\ncame from the same origin that is different from the two lines that\nfollow, etc., showing horizontal stripes of colours. If we were to\ngo that route, I suspect that it would be too distracting to paint\nthe whole line (like \"git diff\" does for added or deleted\nlines). Perhaps paint only the commit object name part in different\ncolors to show which lines form a group that came from the same\norigin? The way \"git show-branch\" paints its output might give an\ninspiration.\n"},{"id":"228616","messageId":"52542CB5.3050005@gmail.com","threadId":"35082","inReplyTo":"xmqqob6zn67d.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] blame: add color","fromName":"Chris J Arges","fromEmail":"christopherarges@gmail.com","sentAt":"2013-10-08T16:03:01Z","receivedAt":"2013-10-08T16:03:01Z","isPatch":true,"sender":{"key":"christopherarges@gmail.com","avatar":null},"body":"On 10/08/2013 10:16 AM, Junio C Hamano wrote:\n> Chris J Arges writes:\n> \n>> Add colorized text for git blame when color.interactive is enabled.\n> \n> It does not make any sense to tie this to color.interactive at all,\n> at least to me. The \"check color.blame and if absent fall back to\n> color.ui\", which is the usual pattern, would be more appropriate.\n> \nOk, I wasn't entirely sure of convention, but I could modify it to do this.\n\n>> +static char blame_colors[][COLOR_MAXLEN] = {\n>> +\tGIT_COLOR_RESET,\n>> +\tGIT_COLOR_NORMAL,\t/* PLAIN */\n>> +\tGIT_COLOR_YELLOW,\t/* COMMIT */\n>> +\tGIT_COLOR_BOLD,\t\t/* NAME */\n>> +\tGIT_COLOR_CYAN,\t\t/* LINE */\n>> +\tGIT_COLOR_GREEN,\t/* TIME */\n>> +};\n> \n> Unlike \"git grep\", where some pieces of a single line are more\n> interesting (i.e. the exact text matching the pattern given) than\n> others (i.e. other text on the same line), and \"git diff\", where\n> some lines have different meanings from others (i.e. hunk header,\n> deleted lines, added lines, context lines), the output from \"git\n> blame\" is already columnar and it is obvious to the reader of the\n> output that everything on the leftmost part of all lines is commit\n> object name, without distraction of extra colours; I do not see much\n> point in painting the output into vertical stripes of colours.\n> \nI mainly did this because it makes it easier for me to see where the\ndifferent columns start and stop.\n\n> It may make more sense to assign one colour to each blame origin\n> (i.e. pair), to make it clear that first five lines\n> came from the same origin that is different from the two lines that\n> follow, etc., showing horizontal stripes of colours. If we were to\n> go that route, I suspect that it would be too distracting to paint\n> the whole line (like \"git diff\" does for added or deleted\n> lines). Perhaps paint only the commit object name part in different\n> colors to show which lines form a group that came from the same\n> origin? The way \"git show-branch\" paints its output might give an\n> inspiration.\n> \nThis could provide a more useful colorization; in addition if something\nas simple as the line number was colorized it would provide a easy way\nfor me to see where the code column starts.\n\n--chris j arges\n"}]}