blame: (dim rep. metadata lines or fields, decay date coloring)
cover blame: (dim rep. metadata lines or fields, decay date coloring)
This is picking up [1], but presenting it in another approach, as I realized these are orthogonal features:
* dimming repeated lines/fields of information
* giving a quick visual information how old (as a proxy for 'well tested') a line of code is.
Both features are configurable.
Any feedback welcome.
Thanks,
Stefan
[1] https://public-inbox.org/git/[email protected]/
Stefan Beller (4):
color.h: modernize header
builtin/blame: dim uninteresting metadata builtin/blame: add option to color metadata fields separately builtin/blame: highlight recently changed lines
Documentation/config.txt | 25 ++++++ builtin/blame.c | 216 ++++++++++++++++++++++++++++++++++++++++++----- color.c | 2 - color.h | 49 ++++++++--- t/t8012-blame-colors.sh | 56 ++++++++++++ 5 files changed, 313 insertions(+), 35 deletions(-) create mode 100755 t/t8012-blame-colors.sh
Signature · 1 line
2.15.0.128.gcadd42da22
PATCH 1/4 color.h: modernize header
--- color.c | 2 -- color.h | 48 +++++++++++++++++++++++++++++++++++------------- 2 files changed, 35 insertions(+), 15 deletions(-)
color.c +0 -2
@@ -400,8 +400,6 @@ static int color_vfprintf(FILE *fp, const char *color, const char *fmt,return r; } - -int color_fprintf (FILE *fp,const char *color,const char *fmt, ...) {va_list args;
color.h +35 -13
61 lines not on this page · raw message
Signature · 1 line
2.15.0.128.gcadd42da22
PATCH 2/4 builtin/blame: dim uninteresting metadata
In reply to Stefan BellerWhen using git-blame lots of lines contain redundant information, for example in hunks that consist of multiple lines, the metadata (commit name, author, date) are repeated. A reader may not be interested in those, so offer an option to color the information that is repeated from the previous line differently.
--- Documentation/config.txt | 5 ++++ builtin/blame.c | 68 ++++++++++++++++++++++++++++++++++++++---------- color.h | 1 + t/t8012-blame-colors.sh | 18 +++++++++++++ 4 files changed, 78 insertions(+), 14 deletions(-) create mode 100755 t/t8012-blame-colors.sh
Documentation/config.txt +5 -0
@@ -1189,6 +1189,11 @@ color.status.<slot>:: status short-format), or `unmerged` (files which have unmerged changes). +color.blame.repeatedMeta:: + Use the customized color for the part of git-blame output that + is repeated meta information per line (such as commit id, + author name, date and timezone). Defaults to dark gray. + color.ui:: This variable determines the default value for variables such as `color.diff` and `color.grep` that control the use of color
builtin/blame.c +54 -14
142 lines not on this page · raw message
color.h +1 -0
@@ -30,6 +30,7 @@ struct strbuf;#define GIT_COLOR_BLUE "\033[34m"#define GIT_COLOR_MAGENTA "\033[35m"#define GIT_COLOR_CYAN "\033[36m" +#define GIT_COLOR_DARK "\033[1;30m"#define GIT_COLOR_BOLD_RED "\033[1;31m"#define GIT_COLOR_BOLD_GREEN "\033[1;32m"#define GIT_COLOR_BOLD_YELLOW "\033[1;33m"
t/t8012-blame-colors.sh new file +18 -0
@@ -0,0 +1,18 @@ +#!/bin/sh + +test_description='colored git blame' +. ./test-lib.sh + +PROG='git blame -c' +. "$TEST_DIRECTORY"/annotate-tests.sh + +test_expect_success 'colored blame colors continuous lines' ' + git blame --abbrev=12 --color-lines hello.c >actual.raw && + test_decode_color <actual.raw >actual && + grep "<BOLD;BLACK>(F" actual > F.expect && + grep "<BOLD;BLACK>(H" actual > H.expect && + test_line_count = 2 F.expect && + test_line_count = 3 H.expect +' + +test_done
Signature · 1 line
2.15.0.128.gcadd42da22
PATCH 3/4 builtin/blame: add option to color metadata fields separately
In reply to Stefan BellerUnlike the previous commit, this dims colors for each metadata field individually.
--- builtin/blame.c | 82 +++++++++++++++++++++++++++++++++++++++++++------ t/t8012-blame-colors.sh | 38 +++++++++++++++++++++++ 2 files changed, 111 insertions(+), 9 deletions(-)
builtin/blame.c +73 -9
@@ -323,6 +323,7 @@ static const char *format_time(timestamp_t time, const char *tz_str,#define OUTPUT_SHOW_EMAIL 0400#define OUTPUT_LINE_PORCELAIN 01000#define OUTPUT_COLOR_LINE 02000 +#define OUTPUT_COLOR_FIELDS 04000static void emit_porcelain_details (struct blame_origin *suspect,int repeat) {
@@ -370,6 +371,33 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,putchar ('\n' ); } +static int had_same_field_previously (int opt,int field, +struct blame_entry *ent, +struct blame_entry *prev) +{ +struct commit_info ci, prev_ci; + +switch (field) { +case OUTPUT_SHOW_SCORE : +return ent->score == prev->score; +case OUTPUT_SHOW_NAME : +return prev->suspect && + !strcmp (ent->suspect->path, prev->suspect->path); +case OUTPUT_SHOW_NUMBER : +return ent->s_lno == prev->s_lno + prev->num_lines -1 ; + +case OUTPUT_NO_AUTHOR : +get_commit_info (ent->suspect->commit, &ci,1 ); +get_commit_info (prev->suspect->commit, &prev_ci,1 ); +return ((opt &OUTPUT_SHOW_EMAIL ) && + !strcmp (ci.author_mail.buf, prev_ci.author_mail.buf)) || + !strcmp (ci.author.buf, prev_ci.author.buf); +default : +BUG ("unknown field" ); + } +return 0 ; +} +static inline void colors_unset (const char **use_color,const char **reset_color) { *use_color ="" ;
@@ -382,17 +410,36 @@ static inline void colors_set(const char **use_color, const char **reset_color) *reset_color =GIT_COLOR_RESET ; } +static void setup_field_color (int opt,int cnt,int field, +struct blame_entry *ent, +struct blame_entry *prev, +const char **use_color, +const char **reset_color) +{ +if (!(opt &OUTPUT_COLOR_FIELDS )) +return ; + +if ((cnt >0 || + (prev &&had_same_field_previously (opt, field, ent, prev)))) +colors_set (use_color, reset_color); +else +colors_unset (use_color, reset_color); +} +static void setup_line_color (int opt,int cnt,const char **use_color,const char **reset_color) {colors_unset (use_color, reset_color); -if ((opt &OUTPUT_COLOR_LINE ) && cnt >0 ) +if ((opt & (OUTPUT_COLOR_LINE |OUTPUT_COLOR_FIELDS )) && cnt >0 )colors_set (use_color, reset_color); } -static void emit_other (struct blame_scoreboard *sb,struct blame_entry *ent,int opt) +static void emit_other (struct blame_scoreboard *sb, +struct blame_entry *ent, +struct blame_entry *prev, +int opt) {int cnt;const char *cp;
@@ -432,18 +479,27 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int show_raw_time), ent->lno + 1 + cnt); }else { -if (opt &OUTPUT_SHOW_SCORE ) +if (opt &OUTPUT_SHOW_SCORE ) { +setup_field_color (opt, cnt,OUTPUT_SHOW_SCORE , + ent, prev, &col, &rcol);printf (" %s%*d %02d%s" , col, max_score_digits, ent->score, ent->suspect->refcnt, rcol); -if (opt &OUTPUT_SHOW_NAME ) + } +if (opt &OUTPUT_SHOW_NAME ) { +setup_field_color (opt, cnt,OUTPUT_SHOW_NAME , + ent, prev, &col, &rcol);printf (" %s%-*.*s%s" , col, longest_file, longest_file, suspect->path, rcol); -if (opt &OUTPUT_SHOW_NUMBER ) + } +if (opt &OUTPUT_SHOW_NUMBER ) { +setup_field_color (opt, cnt,OUTPUT_SHOW_NUMBER , + ent, prev, &col, &rcol);printf (" %s%*d%s" , col, max_orig_digits, ent->s_lno +1 + cnt, rcol); + }if (!(opt &OUTPUT_NO_AUTHOR )) {const char *name;int pad;
@@ -452,6 +508,8 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, intelse name = ci.author.buf; pad = longest_author -utf8_strwidth (name); +setup_field_color (opt, cnt,OUTPUT_NO_AUTHOR , + ent, prev, &col, &rcol); printf(" %s(%s%*s %10s%s", col, name, pad, "", format_time(ci.author_time,
@@ -477,7 +535,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, intstatic void output (struct blame_scoreboard *sb,int option) { -struct blame_entry *ent; +struct blame_entry *ent, *prev =NULL ;if (option &OUTPUT_PORCELAIN ) {for (ent = sb->ent; ent; ent = ent->next) {
@@ -499,7 +557,8 @@ static void output(struct blame_scoreboard *sb, int option)if (option &OUTPUT_PORCELAIN )emit_porcelain (sb, ent, option);else { -emit_other (sb, ent, option); +emit_other (sb, ent, prev, option); + prev = ent; } } }
@@ -717,6 +776,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)OPT_BIT ('e' ,"show-email" , &output_option,N_ ("Show author email instead of name (Default: off)" ),OUTPUT_SHOW_EMAIL ),OPT_BIT ('w' ,NULL , &xdl_opts,N_ ("Ignore whitespace differences" ),XDF_IGNORE_WHITESPACE ),OPT_BIT (0 ,"color-lines" , &output_option,N_ ("color redundant metadata from previous line" ),OUTPUT_COLOR_LINE ), + OPT_BIT(0, "color-fields", &output_option, N_("color redundant metadata fields from previous line"), OUTPUT_COLOR_FIELDS),/* * The following two options are parsed by parse_revision_opt()
@@ -775,6 +835,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix)DIFF_OPT_CLR (&revs.diffopt,FOLLOW_RENAMES ); argc =parse_options_end (&ctx); +if ((output_option &OUTPUT_COLOR_LINE ) && + (output_option &OUTPUT_COLOR_FIELDS )) +die (_ ("cannot ask for colored lines and fields at the same time" )); +if (incremental || (output_option &OUTPUT_PORCELAIN )) {if (show_progress >0 )die (_ ("--progress can't be used with --incremental or porcelain formats" ));
@@ -978,8 +1042,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)if (!(output_option &OUTPUT_PORCELAIN )) {find_alignment (&sb, &output_option); -if ((output_option &OUTPUT_COLOR_LINE ) && - !repeated_meta_color) +if (!repeated_meta_color && + (output_option & (OUTPUT_COLOR_LINE |OUTPUT_COLOR_FIELDS ))) repeated_meta_color =GIT_COLOR_DARK ; }
t/t8012-blame-colors.sh +38 -0
@@ -15,4 +15,42 @@ test_expect_success 'colored blame colors continuous lines' ' test_line_count = 3 H.expect ' +test_expect_success 'colored blame colors continuous fields' ' + + git mv hello.c world.c && + git commit -a -m "moved file" && + cat <<-EOF >> world.c && + void world() + { + puts("world"); + } + EOF + git add world.c && + GIT_AUTHOR_NAME="F" GIT_AUTHOR_EMAIL="[email protected]" \ + git commit -m "forgot to add changes to moved file" && + + git blame --abbrev=12 --color-fields world.c >actual.raw && + test_decode_color <actual.raw >actual && + + grep "<BOLD;BLACK>hello.c" actual > colored_hello.expect && + grep "hello.c" actual > all_hello.expect && + test_line_count = 9 colored_hello.expect && + test_line_count = 10 all_hello.expect && + + grep "<BOLD;BLACK>world.c" actual > colored_world.expect && + grep "world.c" actual > all_world.expect && + test_line_count = 3 colored_world.expect && + test_line_count = 4 all_world.expect && + + grep "(F" actual > all_F.expect && + grep "<BOLD;BLACK>(F" actual > colored_F.expect && + test_line_count = 8 all_F.expect && + test_line_count = 5 colored_F.expect && + + grep "(H" actual > all_H.expect && + grep "<BOLD;BLACK>(H" actual > colored_H.expect && + test_line_count = 5 all_H.expect && + test_line_count = 3 colored_H.expect +' + test_done
Signature · 1 line
2.15.0.128.gcadd42da22
PATCH 4/4 builtin/blame: highlight recently changed lines
In reply to Stefan BellerChoose a different color for dates and imitate a 'temperature cool down' for the dates.
--- Documentation/config.txt | 20 +++++++++++++ builtin/blame.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 93 insertions(+), 1 deletion(-)
Documentation/config.txt +20 -0
@@ -1194,6 +1194,26 @@ color.blame.repeatedMeta:: is repeated meta information per line (such as commit id, author name, date and timezone). Defaults to dark gray. +color.blame.highlightRecent:: + This can be used to color the author and date of a blame line. + This overrides `color.blame.repeatedMeta` setting, which colors + repetitions. ++ + This setting should be set to a comma-separated list of + color and date settings, starting and ending with a color, the dates + should be set from oldest to newest. + The metadata will be colored given the colors if the the line was + introduced before the given timestamp, overwriting older timestamped + colors. ++ + Instead of an absolute timestamp relative timestamps work as + well, e.g. 2.weeks.ago is valid to address anything older than 2 weeks. ++ + It defaults to "blue,12 month ago,white,1 month ago,red", + which colors everything older than one year blue, recent changes between + one month and one year old are kept white, and lines introduced within + the last month are colored red. + color.ui:: This variable determines the default value for variables such as `color.diff` and `color.grep` that control the use of color
builtin/blame.c +73 -1
134 lines not on this page · raw message
Signature · 1 line
2.15.0.128.gcadd42da22
v3 cover blame: (dim rep. metadata lines or fields, decay date coloring)
In reply to Stefan Bellerv3:
Thanks Eric for feedback, I implemented all of the suggestions. Specifically I dropped the abstractions in patch2 but keep around a similar abstraction in patch 3 as that still looks like it benefits (the condition is just growing large).
Thanks,
Stefan
v2:
This is picking up [1], but presenting it in another approach, as I realized these are orthogonal features:
* dimming repeated lines/fields of information
* giving a quick visual information how old (as a proxy for 'well tested') a line of code is.
Both features are configurable.
Changes from sending it out in November:
* better commit messages
* rebased on master
Any feedback welcome.
Thanks,
Stefan
[1] https://public-inbox.org/git/[email protected]/
Stefan Beller (4):
color.h: document and modernize header builtin/blame: dim uninteresting metadata builtin/blame: add option to color metadata fields separately builtin/blame: highlight recently changed lines
Documentation/config.txt | 23 ++++++ builtin/blame.c | 201 ++++++++++++++++++++++++++++++++++++++++++----- color.c | 2 - color.h | 59 ++++++++++---- t/t8012-blame-colors.sh | 56 +++++++++++++ 5 files changed, 305 insertions(+), 36 deletions(-) create mode 100755 t/t8012-blame-colors.sh
Signature · 1 line
2.16.0.rc0.223.g4a4ac83678-goog
PATCH v3 1/4 color.h: document and modernize header
Add documentation explaining the functions in color.h. While at it, mark them extern.
--- color.c | 2 -- color.h | 58 ++++++++++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 44 insertions(+), 16 deletions(-)
color.c +0 -2
@@ -399,8 +399,6 @@ static int color_vfprintf(FILE *fp, const char *color, const char *fmt,return r; } - -int color_fprintf (FILE *fp,const char *color,const char *fmt, ...) {va_list args;
color.h +44 -14
70 lines not on this page · raw message
Signature · 1 line
2.16.0.rc0.223.g4a4ac83678-goog
Re: [PATCHv3 1/4] color.h: document and modernize header
color.h extern int color_stdout_is_tty; · line 82
Quoted 9 lines · Stefan Beller
/*
- * Set the color buffer (which must beCOLOR_MAXLENbytes)
- * to the raw color bytes; this is useful for initializing
- * default color variables.
+ * NEEDSWWORK: document this function or refactorgrep.cto stop using this
+ * function.
*/color.h +1
-1-void color_set (char *dst,const char *color_bytes); +extern void color_set (char *dst,const char *color_bytes);
The original that is removed by the patch documents the function well enough; as long as the NEEDSWORK comment is followed through in a later step in the series, it's alright, though ;-)
color.h extern int color_stdout_is_tty; · line 91
Quoted 12 lines
color.h +8
-4-int git_config_colorbool (const char *var,const char *value); -int want_color (int var); -int color_parse (const char *value,char *dst); -int color_parse_mem (const char *value,int len,char *dst); +/* +* Parse a config option, which can be a boolean or one of +* "never", "auto", "always". Return a constant of +* GIT_COLOR_NEVER for "never" or negative boolean, +* GIT_COLOR_ALWAYS for "always" or a positive boolean, +* and GIT_COLOR_AUTO for "auto". +*/ +extern int git_config_colorbool (const char *var,const char *value);
"never" and "always" not being part of usual boolean vocabulary makes it a bit awkward to explain.
Quoted 7 lines
+/*
+ * Output the formatted string in the specified color (and then reset to normal
+ * color so subsequent output is uncolored). Omits the color encapsulation if
+ *coloris NULL. Thecolor_fprintf_lnprints a new line after resetting
+ * the color. Thecolor_print_strbufprints the given pre-formatted strbuf
+ * instead, up to its first NUL character.
+ */
Obviously, it does not have to be part of this step nor series, but the above observation makes us realize that color_print_strbuf() would probably be an unreasonably narrow interface. It is not too much to ask the caller to dereference and pass only the .buf component of the strbuf to an alternative helper that takes "const char *" and by doing so would allow us to let other callers that do not have a strbuf but just a plain string use it, too.
Looks good.
PATCH v3 2/4 builtin/blame: dim uninteresting metadata
In reply to Stefan BellerWhen using git-blame lots of lines contain redundant information, for example in hunks that consist of multiple lines, the metadata (commit name, author, date) are repeated. A reader may not be interested in those, so offer an option to color the information that is repeated from the previous line differently.
--- Documentation/config.txt | 5 +++++ builtin/blame.c | 50 ++++++++++++++++++++++++++++++++++-------------- color.h | 1 + t/t8012-blame-colors.sh | 18 +++++++++++++++++ 4 files changed, 60 insertions(+), 14 deletions(-) create mode 100755 t/t8012-blame-colors.sh
Documentation/config.txt +5 -0
@@ -1216,6 +1216,11 @@ color.status.<slot>:: status short-format), or `unmerged` (files which have unmerged changes). +color.blame.repeatedMeta:: + Use the customized color for the part of git-blame output that + is repeated meta information per line (such as commit id, + author name, date and timezone). Defaults to dark gray. + color.ui:: This variable determines the default value for variables such as `color.diff` and `color.grep` that control the use of color
builtin/blame.c +36 -14
118 lines not on this page · raw message
color.h +1 -0
@@ -30,6 +30,7 @@ struct strbuf;#define GIT_COLOR_BLUE "\033[34m"#define GIT_COLOR_MAGENTA "\033[35m"#define GIT_COLOR_CYAN "\033[36m" +#define GIT_COLOR_DARK "\033[1;30m"#define GIT_COLOR_BOLD_RED "\033[1;31m"#define GIT_COLOR_BOLD_GREEN "\033[1;32m"#define GIT_COLOR_BOLD_YELLOW "\033[1;33m"
t/t8012-blame-colors.sh new file +18 -0
@@ -0,0 +1,18 @@ +#!/bin/sh + +test_description='colored git blame' +. ./test-lib.sh + +PROG='git blame -c' +. "$TEST_DIRECTORY"/annotate-tests.sh + +test_expect_success 'colored blame colors contiguous lines' ' + git blame --abbrev=12 --color-lines hello.c >actual.raw && + test_decode_color <actual.raw >actual && + grep "<BOLD;BLACK>(F" actual > F.expect && + grep "<BOLD;BLACK>(H" actual > H.expect && + test_line_count = 2 F.expect && + test_line_count = 3 H.expect +' + +test_done
Signature · 1 line
2.16.0.rc0.223.g4a4ac83678-goog
Re: [PATCHv3 2/4] builtin/blame: dim uninteresting metadata
Documentation/config.txt color.status.<slot>:: · line 1223
Quoted 6 lines · Stefan Beller
Documentation/config.txt +5
-0+color.blame.repeatedMeta:: + Use the customized color for the part of git-blame output that + is repeated meta information per line (such as commit id, + author name, date and timezone). Defaults to dark gray. +
"Dark gray on default background" may alleviate worrries from those who prefer black ink on white paper display by hinting that both foreground and background colors can be configured.
Do we want to make this overridable from the command line, i.e. --color-repeated-meta=gray?
Quoted 1 line
+#define
OUTPUT_COLOR_LINE02000
The name of the macro implies that this is (or at least can be) a lot more generic UI request than merely "paint the same metadata on adjacent lines in a different color".
builtin/blame.c int cmd_blame(int argc, const char **argv, const char *prefix) · line 701
Quoted 1 line
builtin/blame.c +1
-0+ OPT_BIT(0, "color-lines", &output_option, N_("color redundant metadata from previous line differently"), OUTPUT_COLOR_LINE),
Should this eventually become "--color=<yes,no,auto>" that is more usual and generic, possibly defaulting to "auto" in the future, I wonder?
color.h struct strbuf; · line 36
Quoted 12 lines
color.h +1
-0@@ -30,6 +30,7 @@ struct strbuf;#define GIT_COLOR_BLUE "\033[34m"#define GIT_COLOR_MAGENTA "\033[35m"#define GIT_COLOR_CYAN "\033[36m" +#define GIT_COLOR_DARK "\033[1;30m"#define GIT_COLOR_BOLD_RED "\033[1;31m"#define GIT_COLOR_BOLD_GREEN "\033[1;32m"#define GIT_COLOR_BOLD_YELLOW "\033[1;33m"
How about using CYAN just like "diff" output uses it to paint the least interesting lines? That way we will keep the "uninteresting is cyan" consistency for the default settings without adding a new color to the palette.
Re: [PATCHv3 2/4] builtin/blame: dim uninteresting metadata
Quoted 22 lines · Junio C Hamano
Documentation/config.txt color.status.<slot>:: · line 1223
Quoted 6 lines · Stefan Beller
Documentation/config.txt +5
-0+color.blame.repeatedMeta:: + Use the customized color for the part of git-blame output that + is repeated meta information per line (such as commit id, + author name, date and timezone). Defaults to dark gray. +"Dark gray on default background" may alleviate worrries from those who prefer black ink on white paper display by hinting that both foreground and background colors can be configured.
Do we want to make this overridable from the command line, i.e. --color-repeated-meta=gray?
Quoted 1 line
+#define
OUTPUT_COLOR_LINE02000The name of the macro implies that this is (or at least can be) a lot more generic UI request than merely "paint the same metadata on adjacent lines in a different color".
Quoted 5 lines
Quoted 1 line
+ OPT_BIT(0, "color-lines",
&output_option,N_("color redundant metadata from previous line differently"),OUTPUT_COLOR_LINE),Should this eventually become "--color=<yes,no,auto>" that is more usual and generic, possibly defaulting to "auto" in the future, I wonder?
This sounds like a good change to make, though what does "yes" mean here? The following patches introduce other modes of colorization, such that --color=<no, auto, lines, fields, decayed> would make sense to me, with "lines" as the mode of this patch, "fields" implemented in the next patch and "decayed" having the meaning of the heating algorithm presented in the last patch.
However each of these (except no and auto) would want to have extra parameters, which as mentioned above could go into its own separate parameters, such that
git blame --color=line --color-repeated-meta=cyan
would seem like a good UI. This might look like it could be a good idea to have --color-repeated-meta imply --color=<line,auto> if no --color is given. But as different coloring modes will have different arguments to provide extra color information, I wonder what we would do with
git blame --color="mode1" --extra-color-arg-for-mode-2
So I don't think the idea of having separate options would be a good idea as we'd have to think about the implications as mentioned above. So maybe
git blame "--color=lines,cyan yellow"
would work, if you want "cyan yellow" as the color configuration in the "lines" mode? (bad choice of a background color btw)
Quoted 9 lines
Quoted 9 lines
color.h +1
-0@@ -30,6 +30,7 @@ struct strbuf;#define GIT_COLOR_BLUE "\033[34m"#define GIT_COLOR_MAGENTA "\033[35m"#define GIT_COLOR_CYAN "\033[36m" +#define GIT_COLOR_DARK "\033[1;30m"
Quoted 8 lines
color.h struct strbuf; · line 36
Quoted 3 lines
color.h +0
-0#define GIT_COLOR_BOLD_RED "\033[1;31m"#define GIT_COLOR_BOLD_GREEN "\033[1;32m"#define GIT_COLOR_BOLD_YELLOW "\033[1;33m"How about using CYAN just like "diff" output uses it to paint the least interesting lines? That way we will keep the "uninteresting is cyan" consistency for the default settings without adding a new color to the palette.
sounds good, I'll take that suggestion once we reach consensus on the UI for the user.
Thanks,
Stefan
PATCH v3 3/4 builtin/blame: add option to color metadata fields separately
In reply to Stefan BellerUnlike the previous commit, this dims colors for each metadata field individually.
--- builtin/blame.c | 83 ++++++++++++++++++++++++++++++++++++++++++++----- t/t8012-blame-colors.sh | 38 ++++++++++++++++++++++ 2 files changed, 113 insertions(+), 8 deletions(-)
builtin/blame.c +75 -8
@@ -323,6 +323,7 @@ static const char *format_time(timestamp_t time, const char *tz_str,#define OUTPUT_SHOW_EMAIL 0400#define OUTPUT_LINE_PORCELAIN 01000#define OUTPUT_COLOR_LINE 02000 +#define OUTPUT_COLOR_FIELDS 04000static void emit_porcelain_details (struct blame_origin *suspect,int repeat) {
@@ -370,7 +371,56 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent,putchar ('\n' ); } -static void emit_other (struct blame_scoreboard *sb,struct blame_entry *ent,int opt) +static int had_same_field_previously (int opt,int field, +struct blame_entry *ent, +struct blame_entry *prev) +{ +struct commit_info ci, prev_ci; + +switch (field) { +case OUTPUT_SHOW_SCORE : +return ent->score == prev->score; +case OUTPUT_SHOW_NAME : +return prev->suspect && + !strcmp (ent->suspect->path, prev->suspect->path); +case OUTPUT_SHOW_NUMBER : +return ent->s_lno == prev->s_lno + prev->num_lines -1 ; + +case OUTPUT_NO_AUTHOR : +get_commit_info (ent->suspect->commit, &ci,1 ); +get_commit_info (prev->suspect->commit, &prev_ci,1 ); +return ((opt &OUTPUT_SHOW_EMAIL ) && + !strcmp (ci.author_mail.buf, prev_ci.author_mail.buf)) || + !strcmp (ci.author.buf, prev_ci.author.buf); +default : +BUG ("unknown field" ); + } +return 0 ; +} + +static void setup_field_color (int opt,int cnt,int field, +struct blame_entry *ent, +struct blame_entry *prev, +const char **use_color, +const char **reset_color) +{ +if (!(opt &OUTPUT_COLOR_FIELDS )) +return ; + +if ((cnt >0 || + (prev &&had_same_field_previously (opt, field, ent, prev)))) { + *use_color = repeated_meta_color; + *reset_color =GIT_COLOR_RESET ; + }else { + *use_color ="" ; + *reset_color ="" ; + } +} + +static void emit_other (struct blame_scoreboard *sb, +struct blame_entry *ent, +struct blame_entry *prev, +int opt) {int cnt;const char *cp;
@@ -414,18 +464,27 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int show_raw_time), ent->lno + 1 + cnt); }else { -if (opt &OUTPUT_SHOW_SCORE ) +if (opt &OUTPUT_SHOW_SCORE ) { +setup_field_color (opt, cnt,OUTPUT_SHOW_SCORE , + ent, prev, &color, &reset);printf (" %s%*d %02d%s" , color, max_score_digits, ent->score, ent->suspect->refcnt, reset); -if (opt &OUTPUT_SHOW_NAME ) + } +if (opt &OUTPUT_SHOW_NAME ) { +setup_field_color (opt, cnt,OUTPUT_SHOW_NAME , + ent, prev, &color, &reset);printf (" %s%-*.*s%s" , color, longest_file, longest_file, suspect->path, reset); -if (opt &OUTPUT_SHOW_NUMBER ) + } +if (opt &OUTPUT_SHOW_NUMBER ) { +setup_field_color (opt, cnt,OUTPUT_SHOW_NUMBER , + ent, prev, &color, &reset);printf (" %s%*d%s" , color, max_orig_digits, ent->s_lno +1 + cnt, reset); + }if (!(opt &OUTPUT_NO_AUTHOR )) {const char *name;int pad;
@@ -434,6 +493,8 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, intelse name = ci.author.buf; pad = longest_author -utf8_strwidth (name); +setup_field_color (opt, cnt,OUTPUT_NO_AUTHOR , + ent, prev, &color, &reset); printf(" %s(%s%*s %10s%s", color, name, pad, "", format_time(ci.author_time,
@@ -459,7 +520,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, intstatic void output (struct blame_scoreboard *sb,int option) { -struct blame_entry *ent; +struct blame_entry *ent, *prev =NULL ;if (option &OUTPUT_PORCELAIN ) {for (ent = sb->ent; ent; ent = ent->next) {
@@ -481,7 +542,8 @@ static void output(struct blame_scoreboard *sb, int option)if (option &OUTPUT_PORCELAIN )emit_porcelain (sb, ent, option);else { -emit_other (sb, ent, option); +emit_other (sb, ent, prev, option); + prev = ent; } } }
@@ -699,6 +761,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)OPT_BIT ('e' ,"show-email" , &output_option,N_ ("Show author email instead of name (Default: off)" ),OUTPUT_SHOW_EMAIL ),OPT_BIT ('w' ,NULL , &xdl_opts,N_ ("Ignore whitespace differences" ),XDF_IGNORE_WHITESPACE ),OPT_BIT (0 ,"color-lines" , &output_option,N_ ("color redundant metadata from previous line differently" ),OUTPUT_COLOR_LINE ), + OPT_BIT(0, "color-fields", &output_option, N_("color redundant metadata fields from previous line differently"), OUTPUT_COLOR_FIELDS),/* * The following two options are parsed by parse_revision_opt()
@@ -757,6 +820,10 @@ int cmd_blame(int argc, const char **argv, const char *prefix) revs.diffopt.flags.follow_renames =0 ; argc =parse_options_end (&ctx); +if ((output_option &OUTPUT_COLOR_LINE ) && + (output_option &OUTPUT_COLOR_FIELDS )) +die (_ ("cannot ask for colored lines and fields at the same time" )); +if (incremental || (output_option &OUTPUT_PORCELAIN )) {if (show_progress >0 )die (_ ("--progress can't be used with --incremental or porcelain formats" ));
@@ -960,8 +1027,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)if (!(output_option &OUTPUT_PORCELAIN )) {find_alignment (&sb, &output_option); -if ((output_option &OUTPUT_COLOR_LINE ) && - !repeated_meta_color) +if (!repeated_meta_color && + (output_option & (OUTPUT_COLOR_LINE |OUTPUT_COLOR_FIELDS ))) repeated_meta_color =GIT_COLOR_DARK ; }
t/t8012-blame-colors.sh +38 -0
@@ -15,4 +15,42 @@ test_expect_success 'colored blame colors contiguous lines' ' test_line_count = 3 H.expect ' +test_expect_success 'colored blame colors continuous fields' ' + + git mv hello.c world.c && + git commit -a -m "moved file" && + cat <<-EOF >> world.c && + void world() + { + puts("world"); + } + EOF + git add world.c && + GIT_AUTHOR_NAME="F" GIT_AUTHOR_EMAIL="[email protected]" \ + git commit -m "forgot to add changes to moved file" && + + git blame --abbrev=12 --color-fields world.c >actual.raw && + test_decode_color <actual.raw >actual && + + grep "<BOLD;BLACK>hello.c" actual > colored_hello.expect && + grep "hello.c" actual > all_hello.expect && + test_line_count = 9 colored_hello.expect && + test_line_count = 10 all_hello.expect && + + grep "<BOLD;BLACK>world.c" actual > colored_world.expect && + grep "world.c" actual > all_world.expect && + test_line_count = 3 colored_world.expect && + test_line_count = 4 all_world.expect && + + grep "(F" actual > all_F.expect && + grep "<BOLD;BLACK>(F" actual > colored_F.expect && + test_line_count = 8 all_F.expect && + test_line_count = 5 colored_F.expect && + + grep "(H" actual > all_H.expect && + grep "<BOLD;BLACK>(H" actual > colored_H.expect && + test_line_count = 5 all_H.expect && + test_line_count = 3 colored_H.expect +' + test_done
Signature · 1 line
2.16.0.rc0.223.g4a4ac83678-goog
PATCH v3 4/4 builtin/blame: highlight recently changed lines
In reply to Stefan BellerChoose a different color for dates and imitate a 'temperature cool down' depending upon age.
Originally I had planned to have the temperature cooldown dependent on the age of the project or file for example, as that might scale better, but that can be added on top of this commit, e.g. instead of giving a date, you could imagine giving a percentage that would be the linearly interpolated between now and the beginning of the file.
--- Documentation/config.txt | 18 ++++++++++++ builtin/blame.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 1 deletion(-)
Documentation/config.txt +18 -0
@@ -1221,6 +1221,24 @@ color.blame.repeatedMeta:: is repeated meta information per line (such as commit id, author name, date and timezone). Defaults to dark gray. +color.blame.highlightRecent:: + This can be used to color the author and date of a blame line. + This overrides `color.blame.repeatedMeta` setting, which colors + repetitions. ++ +This setting should be set to a comma-separated list of color and date settings, +starting and ending with a color, the dates should be set from oldest to newest. +The metadata will be colored given the colors if the the line was introduced +before the given timestamp, overwriting older timestamped colors. ++ +Instead of an absolute timestamp relative timestamps work as well, e.g. +2.weeks.ago is valid to address anything older than 2 weeks. ++ +It defaults to "blue,12 month ago,white,1 month ago,red", which colors +everything older than one year blue, recent changes between one month and +one year old are kept white, and lines introduced within the last month are +colored red. + color.ui:: This variable determines the default value for variables such as `color.diff` and `color.grep` that control the use of color
builtin/blame.c +73 -1
134 lines not on this page · raw message
Signature · 1 line
2.16.0.rc0.223.g4a4ac83678-goog
Re: [PATCHv3 4/4] builtin/blame: highlight recently changed lines
Quoted 14 lines · Stefan Beller
builtin/blame.c +14
-0+static struct color_field { +timestamp_t hop; +char col[COLOR_MAXLEN ]; +} *colorfield; +static int colorfield_nr, colorfield_alloc; + +static void parse_color_fields (const char *s) +{ +struct string_list l =STRING_LIST_INIT_DUP ; +struct string_list_item *item; +enum {EXPECT_DATE ,EXPECT_COLOR } next =EXPECT_COLOR ; + +/* Ideally this would be stripped and split at the same time? */ +string_list_split (&l, s,',' ,-1 );
builtin/blame.c static void setup_field_color(int opt, int cnt, int field, · line 445
Quoted 10 lines
builtin/blame.c +10
-0+ALLOC_GROW (colorfield, colorfield_nr +1 , colorfield_alloc); + +for_each_string_list_item (item, &l) { +switch (next) { +case EXPECT_DATE : + colorfield[colorfield_nr].hop =approxidate (item->string); + next =EXPECT_COLOR ; + colorfield_nr++; +ALLOC_GROW (colorfield, colorfield_nr +1 , colorfield_alloc); +break ;
This should make sure cf[i].hop is monotonically increasing to avoid end-user mistakes, I would think (what's 'hop' by the way?).
builtin/blame.c static void setup_field_color(int opt, int cnt, int field, · line 448
Quoted 3 lines
builtin/blame.c +3
-0+case EXPECT_COLOR : +if (color_parse (item->string, colorfield[colorfield_nr].col)) +die (_ ("expecting a color: %s" ), item->string);
When you have a typo in one of your configuration files, say "[color "blame"] highlightrecent = 1,week,blue,...", you'd want to see a bit more than just "expecting a color: week" to help you diagnose and resolve the issue. Giving the name of the variable and the file the wrong definition was found in would be needed, givin that this is called from the config callback git_blame_config() below.
builtin/blame.c static void setup_field_color(int opt, int cnt, int field, · line 455
Quoted 7 lines
builtin/blame.c +7
-0+ next =EXPECT_DATE ; +break ; + } + } + +if (next ==EXPECT_COLOR ) +die (_ ("must end with a color" ));
Same here.
builtin/blame.c int cmd_blame(int argc, const char **argv, const char *prefix) · line 835
Quoted 3 lines
builtin/blame.c +1
-0OPT_BIT (0 ,"color-lines" , &output_option,N_ ("color redundant metadata from previous line differently" ),OUTPUT_COLOR_LINE ),OPT_BIT (0 ,"color-fields" , &output_option,N_ ("color redundant metadata fields from previous line differently" ),OUTPUT_COLOR_FIELDS ), + OPT_BIT(0, "heated-lines", &output_option, N_("color lines by age"), OUTPUT_HEATED_LINES),
These options may be useful while having fun experimenting, but my gut feeling is that these are too fine-grained for end-users to tweak per invocation basis (which is what command line options are for).
But perhaps I am biased (as anybody else), as personally I find anything beyond step 2/4 uninteresting, and not adding too many of these options is consistent with that viewpoint ;-)
In any case, thanks for a fun read.
Re: [PATCHv3 4/4] builtin/blame: highlight recently changed lines
Quoted 55 lines · Junio C Hamano
Quoted 14 lines · Stefan Beller
builtin/blame.c +14
-0+static struct color_field { +timestamp_t hop; +char col[COLOR_MAXLEN ]; +} *colorfield; +static int colorfield_nr, colorfield_alloc; + +static void parse_color_fields (const char *s) +{ +struct string_list l =STRING_LIST_INIT_DUP ; +struct string_list_item *item; +enum {EXPECT_DATE ,EXPECT_COLOR } next =EXPECT_COLOR ; + +/* Ideally this would be stripped and split at the same time? */ +string_list_split (&l, s,',' ,-1 );builtin/blame.c static void setup_field_color(int opt, int cnt, int field, · line 445
Quoted 10 lines
builtin/blame.c +10
-0+ALLOC_GROW (colorfield, colorfield_nr +1 , colorfield_alloc); + +for_each_string_list_item (item, &l) { +switch (next) { +case EXPECT_DATE : + colorfield[colorfield_nr].hop =approxidate (item->string); + next =EXPECT_COLOR ; + colorfield_nr++; +ALLOC_GROW (colorfield, colorfield_nr +1 , colorfield_alloc); +break ;This should make sure cf[i].hop is monotonically increasing to avoid end-user mistakes, I would think (what's 'hop' by the way?).
builtin/blame.c static void setup_field_color(int opt, int cnt, int field, · line 448
Quoted 3 lines
builtin/blame.c +3
-0+case EXPECT_COLOR : +if (color_parse (item->string, colorfield[colorfield_nr].col)) +die (_ ("expecting a color: %s" ), item->string);When you have a typo in one of your configuration files, say "[color "blame"] highlightrecent = 1,week,blue,...", you'd want to see a bit more than just "expecting a color: week" to help you diagnose and resolve the issue. Giving the name of the variable and the file the wrong definition was found in would be needed, givin that this is called from the config callback
git_blame_config()below.builtin/blame.c static void setup_field_color(int opt, int cnt, int field, · line 455
Quoted 7 lines
builtin/blame.c +7
-0+ next =EXPECT_DATE ; +break ; + } + } + +if (next ==EXPECT_COLOR ) +die (_ ("must end with a color" ));Same here.
Quoted 3 lines
OPT_BIT(0, "color-lines",
&output_option,N_("color redundant metadata from previous line differently"),OUTPUT_COLOR_LINE), OPT_BIT(0, "color-fields",&output_option,N_("color redundant metadata fields from previous line differently"),OUTPUT_COLOR_FIELDS),
+OPT_BIT(0, "heated-lines", &output_option, N_("color lines by age"), OUTPUT_HEATED_LINES),
Quoted 8 lines
These options may be useful while having fun experimenting, but my gut feeling is that these are too fine-grained for end-users to tweak per invocation basis (which is what command line options are for).
But perhaps I am biased (as anybody else), as personally I find anything beyond step 2/4 uninteresting, and not adding too many of these options is consistent with that viewpoint ;-)
See, I find 2 and 3 uninteresting and just did it 'because someone else hinted at that is what they want'. Maybe I was a bad listener.
4 (maybe with 2 in combination) would be all I need as that allows me to quickly judge the trustworthiness of code (old code is better, just like most liquors? ;)
Quoted 1 line
In any case, thanks for a fun read.
Thanks, I'll reread the comments and see if I can remove some options to make it useful for upstream consumption.
Thanks,
Stefan
On Thu, Jan 04 2018, Stefan Beller jotted:
Quoted 5 lines
Stefan Beller (4):
color.h: document and modernize header builtin/blame: dim uninteresting metadata builtin/blame: add option to color metadata fields separately builtin/blame: highlight recently changed lines
I like this feature in principle, but I can't get it to work. Building pu and:
./git -c color.ui=always --exec-path=$PWD blame Makefile
Shows no colors. Neither does:
./git -c color.ui=always --exec-path=$PWD -c color.blame.highlightRecent="red,12 month ago,blue" blame Makefile
And there's a bug, it segfaults on any custom value to the other config option:
./git -c color.ui=always --exec-path=$PWD -c color.blame.repeatedMeta=red blame Makefile
0x00000000004c312b in color_parse_mem (value=0x8f6520 "red", value_len=3, dst=0x1 <Address 0x1 out of bounds>) at color.c:272
272 OUT('\033');The repeated_meta_color variable is NULL when passed to color_parse_mem(). Didn't dig further.
Quoted 11 lines · Ævar Arnfjörð Bjarmason
On Thu, Jan 04 2018, Stefan Beller jotted:
Quoted 5 lines
Stefan Beller (4):
color.h: document and modernize header builtin/blame: dim uninteresting metadata builtin/blame: add option to color metadata fields separately builtin/blame: highlight recently changed linesI like this feature in principle, but I can't get it to work. Building pu and:
Thanks for testing the feature!
Please use the flag --color-lines, --color-fields or --heated-lines for experimentation.
In the future we may decide that one of them becomes the default (which one?) and is triggered by the color.ui=always setting as well.
Quoted 9 lines
./git -c color.ui=always --exec-path=$PWD blame MakefileShows no colors. Neither does:
./git -c color.ui=always --exec-path=$PWD -c color.blame.highlightRecent="red,12 month ago,blue" blame MakefileAnd there's a bug, it segfaults on any custom value to the other config option:
Quoted 7 lines
./git -c color.ui=always --exec-path=$PWD -c color.blame.repeatedMeta=red blame Makefile 0x00000000004c312b in color_parse_mem (value=0x8f6520 "red", value_len=3, dst=0x1 <Address 0x1 out of bounds>) at color.c:272 272 OUT('\033');The
repeated_meta_colorvariable is NULL when passed tocolor_parse_mem(). Didn't dig further.
Thanks for noticing.
Fix below (white space mangled)
builtin/blame.c +1 -1
@@ -48,7 +48,7 @@ static int xdl_opts;static int abbrev =-1 ;static int no_whole_file_rename;static int show_progress; -static char *repeated_meta_color; +static char repeated_meta_color[COLOR_MAXLEN ];static struct date_mode blame_date_mode = {DATE_ISO8601 };static size_t blame_date_width;
@@ -1099,9 +1099,9 @@ int cmd_blame(int argc, const char **argv, const
char *prefix)
if (!(output_option & OUTPUT_PORCELAIN)) {
find_alignment(&sb, &output_option);- if (!repeated_meta_color &&
+ if (!*repeated_meta_color &&
(output_option & (OUTPUT_COLOR_LINE | OUTPUT_COLOR_FIELDS)))
- repeated_meta_color = GIT_COLOR_DARK;
+ strcpy(repeated_meta_color, GIT_COLOR_DARK);
}