# [RFC] Colorize 'commit' lines in log ui 4 messages from 2006-07-23 to 2006-07-24. Participants: Jeff King, Jan-Benedict Glaw, Junio C Hamano. Thread: https://gitlist.dev/t/4955 ## Jeff King, 2006-07-23 09:24 Subject: [RFC] Colorize 'commit' lines in log ui Message-ID: <20060723092417.GA7547@coredump.intra.peff.net> URL: https://gitlist.dev/e/20060723092417.GA7547%40coredump.intra.peff.net ``` When paging through the output of git-whatchanged, the color cues help to visually navigate within a diff. However, it is difficult to notice when a new commit starts, because the commit and log are shown in the "normal" color. This patch colorizes the 'commit' line, customizable through diff.colors.commit and defaulting to yellow. As a side effect, some of the diff color engine (slot enum, get_color) has become accessible outside of diff.c. Signed-off-by: Jeff King --- I think the visual cue makes the git-whatchanged -p output much easier to read. I originally thought of making a "colorized commit" patch where you could independently set commit.color and commit.color.*. However, it was clearly much simpler to start by using the diff engine's code. 1. Do people really want to colorize the commit outside of colorizing the diffs, anyway? It makes sense for the code to go together. 2. Should we call it something other than diff_colors (since now it's handling the commit log, too)? It currently just colorizes the 'commit' line. We could also do the 'Author' and 'Date' lines (either as the same color, or independently), as well as the log text. It colorizes all formats given to show_log() except CMIT_FMT_EMAIL (so, e.g., the sha1's in CMIT_FMT_ONELINE will be yellow). Is that what we want? diff.c | 28 +++++++++++----------------- diff.h | 11 +++++++++++ log-tree.c | 5 ++++- 3 files changed, 26 insertions(+), 18 deletions(-) diff --git a/diff.c b/diff.c index 8b44756..6a71376 100644 --- a/diff.c +++ b/diff.c @@ -17,15 +17,6 @@ static int diff_detect_rename_default = static int diff_rename_limit_default = -1; static int diff_use_color_default = 0; -enum color_diff { - DIFF_RESET = 0, - DIFF_PLAIN = 1, - DIFF_METAINFO = 2, - DIFF_FRAGINFO = 3, - DIFF_FILE_OLD = 4, - DIFF_FILE_NEW = 5, -}; - /* "\033[1;38;5;2xx;48;5;2xxm\0" is 23 bytes */ static char diff_colors[][24] = { "\033[m", /* reset */ @@ -33,7 +24,8 @@ static char diff_colors[][24] = { "\033[1m", /* bold */ "\033[36m", /* cyan */ "\033[31m", /* red */ - "\033[32m" /* green */ + "\033[32m", /* green */ + "\033[33m" /* yellow */ }; static int parse_diff_color_slot(const char *var, int ofs) @@ -48,6 +40,8 @@ static int parse_diff_color_slot(const c return DIFF_FILE_OLD; if (!strcasecmp(var+ofs, "new")) return DIFF_FILE_NEW; + if (!strcasecmp(var+ofs, "commit")) + return DIFF_COMMIT; die("bad config variable '%s'", var); } @@ -370,7 +364,7 @@ struct emit_callback { const char **label_path; }; -static inline const char *get_color(int diff_use_color, enum color_diff ix) +const char *diff_get_color(int diff_use_color, enum color_diff ix) { if (diff_use_color) return diff_colors[ix]; @@ -381,8 +375,8 @@ static void fn_out_consume(void *priv, c { int i; struct emit_callback *ecbdata = priv; - const char *set = get_color(ecbdata->color_diff, DIFF_METAINFO); - const char *reset = get_color(ecbdata->color_diff, DIFF_RESET); + const char *set = diff_get_color(ecbdata->color_diff, DIFF_METAINFO); + const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET); if (ecbdata->label_path[0]) { printf("%s--- %s%s\n", set, ecbdata->label_path[0], reset); @@ -397,7 +391,7 @@ static void fn_out_consume(void *priv, c ; if (2 <= i && i < len && line[i] == ' ') { ecbdata->nparents = i - 1; - set = get_color(ecbdata->color_diff, DIFF_FRAGINFO); + set = diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO); } else if (len < ecbdata->nparents) set = reset; @@ -410,7 +404,7 @@ static void fn_out_consume(void *priv, c else if (line[i] == '+') color = DIFF_FILE_NEW; } - set = get_color(ecbdata->color_diff, color); + set = diff_get_color(ecbdata->color_diff, color); } if (len > 0 && line[len-1] == '\n') len--; @@ -767,8 +761,8 @@ static void builtin_diff(const char *nam mmfile_t mf1, mf2; const char *lbl[2]; char *a_one, *b_two; - const char *set = get_color(o->color_diff, DIFF_METAINFO); - const char *reset = get_color(o->color_diff, DIFF_RESET); + const char *set = diff_get_color(o->color_diff, DIFF_METAINFO); + const char *reset = diff_get_color(o->color_diff, DIFF_RESET); a_one = quote_two("a/", name_a); b_two = quote_two("b/", name_b); diff --git a/diff.h b/diff.h index a06f959..2cced53 100644 --- a/diff.h +++ b/diff.h @@ -69,6 +69,17 @@ struct diff_options { add_remove_fn_t add_remove; }; +enum color_diff { + DIFF_RESET = 0, + DIFF_PLAIN = 1, + DIFF_METAINFO = 2, + DIFF_FRAGINFO = 3, + DIFF_FILE_OLD = 4, + DIFF_FILE_NEW = 5, + DIFF_COMMIT = 6, +}; +const char *diff_get_color(int diff_use_color, enum color_diff ix); + extern const char mime_boundary_leader[]; extern void diff_tree_setup_paths(const char **paths, struct diff_options *); diff --git a/log-tree.c b/log-tree.c index 9d8d46f..b71cf9b 100644 --- a/log-tree.c +++ b/log-tree.c @@ -129,7 +129,8 @@ void show_log(struct rev_info *opt, cons opt->diffopt.stat_sep = buffer; } } else { - printf("%s%s", + printf("%s%s%s", + diff_get_color(opt->diffopt.color_diff, DIFF_COMMIT), opt->commit_format == CMIT_FMT_ONELINE ? "" : "commit ", diff_unique_abbrev(commit->object.sha1, abbrev_commit)); if (opt->parents) @@ -139,6 +140,8 @@ void show_log(struct rev_info *opt, cons diff_unique_abbrev(parent->object.sha1, abbrev_commit)); putchar(opt->commit_format == CMIT_FMT_ONELINE ? ' ' : '\n'); + printf("%s", + diff_get_color(opt->diffopt.color_diff, DIFF_RESET)); } /* -- 1.4.2.rc1.gc470-dirty ``` ## Jan-Benedict Glaw, 2006-07-23 11:24 Subject: Re: [RFC] Colorize 'commit' lines in log ui Message-ID: <20060723112422.GB27825@lug-owl.de> URL: https://gitlist.dev/e/20060723112422.GB27825%40lug-owl.de In-Reply-To: <20060723092417.GA7547@coredump.intra.peff.net> ``` On Sun, 2006-07-23 05:24:18 -0400, Jeff King wrote: > @@ -33,7 +24,8 @@ static char diff_colors[][24] = { > "\033[1m", /* bold */ > "\033[36m", /* cyan */ > "\033[31m", /* red */ > - "\033[32m" /* green */ > + "\033[32m", /* green */ > + "\033[33m" /* yellow */ > }; > > static int parse_diff_color_slot(const char *var, int ofs) If people only were used to *always* place a comma behind the last array element, the diff would be shorter by one line each time... MfG, JBG -- Jan-Benedict Glaw jbglaw@lug-owl.de +49-172-7608481 Signature of: ...und wenn Du denkst, es geht nicht mehr, the second : kommt irgendwo ein Lichtlein her. ``` ## Jeff King, 2006-07-23 22:01 Subject: Re: [RFC] Colorize 'commit' lines in log ui Message-ID: <20060723220133.GA26390@coredump.intra.peff.net> URL: https://gitlist.dev/e/20060723220133.GA26390%40coredump.intra.peff.net In-Reply-To: <20060723112422.GB27825@lug-owl.de> ``` On Sun, Jul 23, 2006 at 01:24:22PM +0200, Jan-Benedict Glaw wrote: > If people only were used to *always* place a comma behind the last > array element, the diff would be shorter by one line each time... I agree that it's much more convenient; I left it off because it wasn't there before and I thought there were some standardization issues. As it turns out, the situation is quite ridiculous. C89 allowed trailing commas in initialization lists but not in enums. Most compilers (gcc, sun) accepted it anyway, but some (aix xlc) did not. Do we care? I think probably not, since the color_diff enum already had a trailing comma. It seems like the primary non-gcc compiler that has been mentioned is sun cc. I don't know what other compilers are being used to compile git. -Peff ``` ## Junio C Hamano, 2006-07-24 07:06 Subject: Re: [RFC] Colorize 'commit' lines in log ui Message-ID: <7vwta3eb2p.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vwta3eb2p.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20060723092417.GA7547@coredump.intra.peff.net> ``` Jeff King writes: > I think the visual cue makes the git-whatchanged -p output much easier > to read. Although I personally do not do much colors (instead I do "/^commit [0-9a-f]*" under less), I think this makes sense and I do not think of any obvious downside. ```