{"thread":{"id":"4955","subject":"[RFC] Colorize 'commit' lines in log ui","startedAt":"2006-07-23T09:24:18Z","lastAt":"2006-07-24T07:06:22Z","messageCount":4,"participants":["Jeff King","Jan-Benedict Glaw","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"24014","messageId":"20060723092417.GA7547@coredump.intra.peff.net","threadId":"4955","inReplyTo":null,"subject":"[RFC] Colorize 'commit' lines in log ui","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2006-07-23T09:24:18Z","receivedAt":"2006-07-23T09:24:18Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When paging through the output of git-whatchanged, the color cues help to\nvisually navigate within a diff. However, it is difficult to notice when a\nnew commit starts, because the commit and log are shown in the \"normal\"\ncolor. This patch colorizes the 'commit' line, customizable through\ndiff.colors.commit and defaulting to yellow.\n\nAs a side effect, some of the diff color engine (slot enum, get_color) has\nbecome accessible outside of diff.c.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n\nI think the visual cue makes the git-whatchanged -p output much easier\nto read.\n\nI originally thought of making a \"colorized commit\" patch where you\ncould independently set commit.color and commit.color.*. However, it was\nclearly much simpler to start by using the diff engine's code.\n  1. Do people really want to colorize the commit outside of colorizing\n     the diffs, anyway? It makes sense for the code to go together.\n  2. Should we call it something other than diff_colors (since now it's\n     handling the commit log, too)?\n\nIt currently just colorizes the 'commit' line. We could also do the\n'Author' and 'Date' lines (either as the same color, or independently),\nas well as the log text.\n\nIt colorizes all formats given to show_log() except CMIT_FMT_EMAIL (so,\ne.g., the sha1's in CMIT_FMT_ONELINE will be yellow). Is that what we\nwant?\n\n diff.c     |   28 +++++++++++-----------------\n diff.h     |   11 +++++++++++\n log-tree.c |    5 ++++-\n 3 files changed, 26 insertions(+), 18 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 8b44756..6a71376 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -17,15 +17,6 @@ static int diff_detect_rename_default = \n static int diff_rename_limit_default = -1;\n static int diff_use_color_default = 0;\n \n-enum color_diff {\n-\tDIFF_RESET = 0,\n-\tDIFF_PLAIN = 1,\n-\tDIFF_METAINFO = 2,\n-\tDIFF_FRAGINFO = 3,\n-\tDIFF_FILE_OLD = 4,\n-\tDIFF_FILE_NEW = 5,\n-};\n-\n /* \"\\033[1;38;5;2xx;48;5;2xxm\\0\" is 23 bytes */\n static char diff_colors[][24] = {\n \t\"\\033[m\",\t/* reset */\n@@ -33,7 +24,8 @@ static char diff_colors[][24] = {\n \t\"\\033[1m\",\t/* bold */\n \t\"\\033[36m\",\t/* cyan */\n \t\"\\033[31m\",\t/* red */\n-\t\"\\033[32m\"\t/* green */\n+\t\"\\033[32m\",\t/* green */\n+\t\"\\033[33m\"\t/* yellow */\n };\n \n static int parse_diff_color_slot(const char *var, int ofs)\n@@ -48,6 +40,8 @@ static int parse_diff_color_slot(const c\n \t\treturn DIFF_FILE_OLD;\n \tif (!strcasecmp(var+ofs, \"new\"))\n \t\treturn DIFF_FILE_NEW;\n+\tif (!strcasecmp(var+ofs, \"commit\"))\n+\t\treturn DIFF_COMMIT;\n \tdie(\"bad config variable '%s'\", var);\n }\n \n@@ -370,7 +364,7 @@ struct emit_callback {\n \tconst char **label_path;\n };\n \n-static inline const char *get_color(int diff_use_color, enum color_diff ix)\n+const char *diff_get_color(int diff_use_color, enum color_diff ix)\n {\n \tif (diff_use_color)\n \t\treturn diff_colors[ix];\n@@ -381,8 +375,8 @@ static void fn_out_consume(void *priv, c\n {\n \tint i;\n \tstruct emit_callback *ecbdata = priv;\n-\tconst char *set = get_color(ecbdata->color_diff, DIFF_METAINFO);\n-\tconst char *reset = get_color(ecbdata->color_diff, DIFF_RESET);\n+\tconst char *set = diff_get_color(ecbdata->color_diff, DIFF_METAINFO);\n+\tconst char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);\n \n \tif (ecbdata->label_path[0]) {\n \t\tprintf(\"%s--- %s%s\\n\", set, ecbdata->label_path[0], reset);\n@@ -397,7 +391,7 @@ static void fn_out_consume(void *priv, c\n \t\t;\n \tif (2 <= i && i < len && line[i] == ' ') {\n \t\tecbdata->nparents = i - 1;\n-\t\tset = get_color(ecbdata->color_diff, DIFF_FRAGINFO);\n+\t\tset = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO);\n \t}\n \telse if (len < ecbdata->nparents)\n \t\tset = reset;\n@@ -410,7 +404,7 @@ static void fn_out_consume(void *priv, c\n \t\t\telse if (line[i] == '+')\n \t\t\t\tcolor = DIFF_FILE_NEW;\n \t\t}\n-\t\tset = get_color(ecbdata->color_diff, color);\n+\t\tset = diff_get_color(ecbdata->color_diff, color);\n \t}\n \tif (len > 0 && line[len-1] == '\\n')\n \t\tlen--;\n@@ -767,8 +761,8 @@ static void builtin_diff(const char *nam\n \tmmfile_t mf1, mf2;\n \tconst char *lbl[2];\n \tchar *a_one, *b_two;\n-\tconst char *set = get_color(o->color_diff, DIFF_METAINFO);\n-\tconst char *reset = get_color(o->color_diff, DIFF_RESET);\n+\tconst char *set = diff_get_color(o->color_diff, DIFF_METAINFO);\n+\tconst char *reset = diff_get_color(o->color_diff, DIFF_RESET);\n \n \ta_one = quote_two(\"a/\", name_a);\n \tb_two = quote_two(\"b/\", name_b);\ndiff --git a/diff.h b/diff.h\nindex a06f959..2cced53 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -69,6 +69,17 @@ struct diff_options {\n \tadd_remove_fn_t add_remove;\n };\n \n+enum color_diff {\n+\tDIFF_RESET = 0,\n+\tDIFF_PLAIN = 1,\n+\tDIFF_METAINFO = 2,\n+\tDIFF_FRAGINFO = 3,\n+\tDIFF_FILE_OLD = 4,\n+\tDIFF_FILE_NEW = 5,\n+\tDIFF_COMMIT = 6,\n+};\n+const char *diff_get_color(int diff_use_color, enum color_diff ix);\n+\n extern const char mime_boundary_leader[];\n \n extern void diff_tree_setup_paths(const char **paths, struct diff_options *);\ndiff --git a/log-tree.c b/log-tree.c\nindex 9d8d46f..b71cf9b 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -129,7 +129,8 @@ void show_log(struct rev_info *opt, cons\n \t\t\topt->diffopt.stat_sep = buffer;\n \t\t}\n \t} else {\n-\t\tprintf(\"%s%s\",\n+\t\tprintf(\"%s%s%s\",\n+\t\t       diff_get_color(opt->diffopt.color_diff, DIFF_COMMIT),\n \t\t       opt->commit_format == CMIT_FMT_ONELINE ? \"\" : \"commit \",\n \t\t       diff_unique_abbrev(commit->object.sha1, abbrev_commit));\n \t\tif (opt->parents)\n@@ -139,6 +140,8 @@ void show_log(struct rev_info *opt, cons\n \t\t\t       diff_unique_abbrev(parent->object.sha1,\n \t\t\t\t\t\t  abbrev_commit));\n \t\tputchar(opt->commit_format == CMIT_FMT_ONELINE ? ' ' : '\\n');\n+\t\tprintf(\"%s\",\n+\t\t       diff_get_color(opt->diffopt.color_diff, DIFF_RESET));\n \t}\n \n \t/*\n-- \n1.4.2.rc1.gc470-dirty\n"},{"id":"24015","messageId":"20060723112422.GB27825@lug-owl.de","threadId":"4955","inReplyTo":"20060723092417.GA7547@coredump.intra.peff.net","subject":"Re: [RFC] Colorize 'commit' lines in log ui","fromName":"Jan-Benedict Glaw","fromEmail":"jbglaw@lug-owl.de","sentAt":"2006-07-23T11:24:22Z","receivedAt":"2006-07-23T11:24:22Z","isPatch":false,"sender":{"key":"jbglaw@lug-owl.de","avatar":null},"body":"On Sun, 2006-07-23 05:24:18 -0400, Jeff King <peff@peff.net> wrote:\n> @@ -33,7 +24,8 @@ static char diff_colors[][24] = {\n>  \t\"\\033[1m\",\t/* bold */\n>  \t\"\\033[36m\",\t/* cyan */\n>  \t\"\\033[31m\",\t/* red */\n> -\t\"\\033[32m\"\t/* green */\n> +\t\"\\033[32m\",\t/* green */\n> +\t\"\\033[33m\"\t/* yellow */\n>  };\n>  \n>  static int parse_diff_color_slot(const char *var, int ofs)\n\nIf people only were used to *always* place a comma behind the last\narray element, the diff would be shorter by one line each time...\n\nMfG, JBG\n\n-- \n       Jan-Benedict Glaw       jbglaw@lug-owl.de                +49-172-7608481\n Signature of:                     ...und wenn Du denkst, es geht nicht mehr,\n the second  :                            kommt irgendwo ein Lichtlein her.\n"},{"id":"24031","messageId":"20060723220133.GA26390@coredump.intra.peff.net","threadId":"4955","inReplyTo":"20060723112422.GB27825@lug-owl.de","subject":"Re: [RFC] Colorize 'commit' lines in log ui","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2006-07-23T22:01:33Z","receivedAt":"2006-07-23T22:01:33Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jul 23, 2006 at 01:24:22PM +0200, Jan-Benedict Glaw wrote:\n\n> If people only were used to *always* place a comma behind the last\n> array element, the diff would be shorter by one line each time...\n\nI agree that it's much more convenient; I left it off because it wasn't\nthere before and I thought there were some standardization issues.\n\nAs it turns out, the situation is quite ridiculous. C89 allowed trailing\ncommas in initialization lists but not in enums. Most compilers (gcc,\nsun) accepted it anyway, but some (aix xlc) did not.\n\nDo we care? I think probably not, since the color_diff enum already had\na trailing comma. It seems like the primary non-gcc compiler that has\nbeen mentioned is sun cc. I don't know what other compilers are being\nused to compile git.\n\n-Peff\n"},{"id":"24050","messageId":"7vwta3eb2p.fsf@assigned-by-dhcp.cox.net","threadId":"4955","inReplyTo":"20060723092417.GA7547@coredump.intra.peff.net","subject":"Re: [RFC] Colorize 'commit' lines in log ui","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-24T07:06:22Z","receivedAt":"2006-07-24T07:06:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I think the visual cue makes the git-whatchanged -p output much easier\n> to read.\n\nAlthough I personally do not do much colors (instead I do\n\"/^commit [0-9a-f]*\" under less), I think this makes sense and I\ndo not think of any obvious downside.\n"}]}