* [PATCH 0/2] blame: color line by commit
@ 2018-04-16 22:09 Stefan Beller
2018-04-16 22:09 ` [PATCH 1/2] builtin/blame: dim uninteresting metadata lines Stefan Beller
2018-04-16 22:09 ` [PATCH 2/2] builtin/blame: highlight recently changed lines Stefan Beller
0 siblings, 2 replies; 18+ messages in thread
From: Stefan Beller @ 2018-04-16 22:09 UTC (permalink / raw)
To: git; +Cc: avarab, Stefan Beller
This is a resend of sb/blame-color, dropping the patch
"add option to color metadata fields separately" as I think it is not needed.
> Expecting a reroll.
> cf. https://public-inbox.org/git/20171110011002.10179-1-sbeller@google.com/#t
> error messages are funny, can segfault, ...
The error messages have been addressed. However a default mode that is enabled
by color.ui has not been added yet.
Thanks,
Stefan
Stefan Beller (2):
builtin/blame: dim uninteresting metadata lines
builtin/blame: highlight recently changed lines
Documentation/config.txt | 23 ++++++++
builtin/blame.c | 118 +++++++++++++++++++++++++++++++++++++--
color.h | 1 +
t/t8012-blame-colors.sh | 54 ++++++++++++++++++
4 files changed, 192 insertions(+), 4 deletions(-)
create mode 100755 t/t8012-blame-colors.sh
--
2.17.0.484.g0c8726318c-goog
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] builtin/blame: dim uninteresting metadata lines 2018-04-16 22:09 [PATCH 0/2] blame: color line by commit Stefan Beller @ 2018-04-16 22:09 ` Stefan Beller 2018-04-17 3:17 ` Junio C Hamano 2018-04-16 22:09 ` [PATCH 2/2] builtin/blame: highlight recently changed lines Stefan Beller 1 sibling, 1 reply; 18+ messages in thread From: Stefan Beller @ 2018-04-16 22:09 UTC (permalink / raw) To: git; +Cc: avarab, Stefan Beller When 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. Both the command line option '--color-lines' as well as a config option 'color.blame.colorLines' is provided. Signed-off-by: Stefan Beller <sbeller@google.com> --- Documentation/config.txt | 5 +++++ builtin/blame.c | 42 ++++++++++++++++++++++++++++++++++++---- color.h | 1 + t/t8012-blame-colors.sh | 29 +++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 4 deletions(-) create mode 100755 t/t8012-blame-colors.sh diff --git a/Documentation/config.txt b/Documentation/config.txt index 2659153cb3..a223232263 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1218,6 +1218,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 diff --git a/builtin/blame.c b/builtin/blame.c index db38c0b307..b49fee70a9 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -7,6 +7,7 @@ #include "cache.h" #include "config.h" +#include "color.h" #include "builtin.h" #include "commit.h" #include "diff.h" @@ -46,6 +47,7 @@ static int xdl_opts; static int abbrev = -1; static int no_whole_file_rename; static int show_progress; +static char repeated_meta_color[COLOR_MAXLEN]; static struct date_mode blame_date_mode = { DATE_ISO8601 }; static size_t blame_date_width; @@ -316,10 +318,11 @@ static const char *format_time(timestamp_t time, const char *tz_str, #define OUTPUT_PORCELAIN 010 #define OUTPUT_SHOW_NAME 020 #define OUTPUT_SHOW_NUMBER 040 -#define OUTPUT_SHOW_SCORE 0100 -#define OUTPUT_NO_AUTHOR 0200 +#define OUTPUT_SHOW_SCORE 0100 +#define OUTPUT_NO_AUTHOR 0200 #define OUTPUT_SHOW_EMAIL 0400 -#define OUTPUT_LINE_PORCELAIN 01000 +#define OUTPUT_LINE_PORCELAIN 01000 +#define OUTPUT_COLOR_LINE 02000 static void emit_porcelain_details(struct blame_origin *suspect, int repeat) { @@ -375,6 +378,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int struct commit_info ci; char hex[GIT_MAX_HEXSZ + 1]; int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP); + const char *color = "", *reset = ""; get_commit_info(suspect->commit, &ci, 1); oid_to_hex_r(hex, &suspect->commit->object.oid); @@ -384,6 +388,19 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int char ch; int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev; + if (!(opt & OUTPUT_ANNOTATE_COMPAT)) { + if (opt & OUTPUT_COLOR_LINE) { + if (cnt > 0) { + color = repeated_meta_color; + reset = GIT_COLOR_RESET; + } else { + color =""; + reset = ""; + } + } + fputs(color, stdout); + } + if (suspect->commit->object.flags & UNINTERESTING) { if (blank_boundary) memset(hex, ' ', length); @@ -433,6 +450,9 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int printf(" %*d) ", max_digits, ent->lno + 1 + cnt); } + if (!(opt & OUTPUT_ANNOTATE_COMPAT) && + (opt & OUTPUT_COLOR_LINE)) + fputs(reset, stdout); do { ch = *cp++; putchar(ch); @@ -585,6 +605,8 @@ static const char *add_prefix(const char *prefix, const char *path) static int git_blame_config(const char *var, const char *value, void *cb) { + int *output_option = cb; + if (!strcmp(var, "blame.showroot")) { show_root = git_config_bool(var, value); return 0; @@ -607,6 +629,13 @@ static int git_blame_config(const char *var, const char *value, void *cb) parse_date_format(value, &blame_date_mode); return 0; } + if (!strcmp(var, "color.blame.repeatedlines")) { + if (color_parse_mem(value, strlen(value), repeated_meta_color)) + warning(_("invalid color '%s' in color.blame.repeatedLines"), + value); + *output_option |= OUTPUT_COLOR_LINE; + return 0; + } if (git_diff_heuristic_config(var, value, cb) < 0) return -1; @@ -690,6 +719,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) OPT_BIT('s', NULL, &output_option, N_("Suppress author name and timestamp (Default: off)"), OUTPUT_NO_AUTHOR), 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), /* * The following two options are parsed by parse_revision_opt() @@ -949,8 +979,12 @@ int cmd_blame(int argc, const char **argv, const char *prefix) blame_coalesce(&sb); - if (!(output_option & OUTPUT_PORCELAIN)) + if (!(output_option & OUTPUT_PORCELAIN)) { find_alignment(&sb, &output_option); + if (!*repeated_meta_color && + (output_option & OUTPUT_COLOR_LINE)) + strcpy(repeated_meta_color, GIT_COLOR_DARK); + } output(&sb, output_option); free((void *)sb.final_buf); diff --git a/color.h b/color.h index cd0bcedd08..196be16058 100644 --- a/color.h +++ b/color.h @@ -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" diff --git a/t/t8012-blame-colors.sh b/t/t8012-blame-colors.sh new file mode 100755 index 0000000000..bb0fe20e76 --- /dev/null +++ b/t/t8012-blame-colors.sh @@ -0,0 +1,29 @@ +#!/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>" <actual >darkened && + grep "(F" darkened > F.expect && + grep "(H" darkened > H.expect && + test_line_count = 2 F.expect && + test_line_count = 3 H.expect +' + +test_expect_success 'colored blame colors contiguous lines via config' ' + git -c color.blame.repeatedLines=yellow blame --abbrev=12 hello.c >actual.raw && + test_decode_color <actual.raw >actual && + grep "<YELLOW>" <actual >darkened && + grep "(F" darkened > F.expect && + grep "(H" darkened > H.expect && + test_line_count = 2 F.expect && + test_line_count = 3 H.expect +' + +test_done -- 2.17.0.484.g0c8726318c-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] builtin/blame: dim uninteresting metadata lines 2018-04-16 22:09 ` [PATCH 1/2] builtin/blame: dim uninteresting metadata lines Stefan Beller @ 2018-04-17 3:17 ` Junio C Hamano 2018-04-17 19:04 ` Stefan Beller 0 siblings, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2018-04-17 3:17 UTC (permalink / raw) To: Stefan Beller; +Cc: git, avarab Stefan Beller <sbeller@google.com> writes: > @@ -316,10 +318,11 @@ static const char *format_time(timestamp_t time, const char *tz_str, > #define OUTPUT_PORCELAIN 010 > #define OUTPUT_SHOW_NAME 020 > #define OUTPUT_SHOW_NUMBER 040 > -#define OUTPUT_SHOW_SCORE 0100 > -#define OUTPUT_NO_AUTHOR 0200 > +#define OUTPUT_SHOW_SCORE 0100 > +#define OUTPUT_NO_AUTHOR 0200 I wondered what these are about; they are about aligning with HT assuming 8-place tab stop like the other lines. > #define OUTPUT_SHOW_EMAIL 0400 > -#define OUTPUT_LINE_PORCELAIN 01000 > +#define OUTPUT_LINE_PORCELAIN 01000 But then this line has SP plus HT here; it should have been just a single HT (i.e. replace a single SP with HT), to be consistent. > +#define OUTPUT_COLOR_LINE 02000 > > static void emit_porcelain_details(struct blame_origin *suspect, int repeat) > { > @@ -375,6 +378,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int > struct commit_info ci; > char hex[GIT_MAX_HEXSZ + 1]; > int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP); > + const char *color = "", *reset = ""; > > get_commit_info(suspect->commit, &ci, 1); > oid_to_hex_r(hex, &suspect->commit->object.oid); > @@ -384,6 +388,19 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int > char ch; > int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev; > > + if (!(opt & OUTPUT_ANNOTATE_COMPAT)) { > + if (opt & OUTPUT_COLOR_LINE) { > + if (cnt > 0) { > + color = repeated_meta_color; > + reset = GIT_COLOR_RESET; > + } else { > + color =""; You need a SP after '=' that assigns an empty string to 'color'. > + reset = ""; > + } > + } > + fputs(color, stdout); > + } This fputs() should be hidden to the case where color is *NOT* an empty string, no? In any case, it should be inside "if color-line is in effect" block, I would think. Users of "git annotate" would not pass the --color option, so I do not think the outer if () block is worth the loss of readability due to increased indent level. I would say that it is a job of the caller of git_config() to make sure color.blame.lines would not take effect when the command is annotate, if what you are worried about is the configuration in this code. > @@ -433,6 +450,9 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int > printf(" %*d) ", > max_digits, ent->lno + 1 + cnt); > } > + if (!(opt & OUTPUT_ANNOTATE_COMPAT) && > + (opt & OUTPUT_COLOR_LINE)) > + fputs(reset, stdout); Likewise. > @@ -949,8 +979,12 @@ int cmd_blame(int argc, const char **argv, const char *prefix) > > blame_coalesce(&sb); > > - if (!(output_option & OUTPUT_PORCELAIN)) > + if (!(output_option & OUTPUT_PORCELAIN)) { > find_alignment(&sb, &output_option); > + if (!*repeated_meta_color && > + (output_option & OUTPUT_COLOR_LINE)) > + strcpy(repeated_meta_color, GIT_COLOR_DARK); > + } This code does not check OUTPUT_ANNOTATE_COMPAT because it assumes that OUTPUT_COLOR_LINE won't be in output_option when we are working in annotate compatibility mode. The deeper codepaths we saw above should do the same. It should be enough to drop color-line when anno-compat is set, or something like that, immediately after reading the config and overriding them from the command line. > diff --git a/color.h b/color.h > index cd0bcedd08..196be16058 100644 > --- a/color.h > +++ b/color.h > @@ -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" I wonder if it is worth adding a new color only to give this a different default. Traditionally, we use CYAN for lines that are less interesting than others (e.g. hunk header), so reusing it might make the life easier to the users, especially because I envision that we may want to introduce another "logical" level to give another redirection between the configuration keys like color.diff.frag and color.blame.repeatedlines and the actual ANSI sequence like "\033[36m". I.e. we update the system so that these two configuration keys take "uninteresting" (which is one of the "logical" colors) by default, and then map "uninteresting" to "\033[36m" at the physical level by default. The users could then change the mapping from "uninteresting" to "\033[1;30m" and consistently modify both diff.frag and blame.repeated if they wanted to. Of course, if they want them to be different, they can come up with a different "logical" color and split the two. And from that point of view, adding new colors to the default set we have above will make life harder for the end users. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] builtin/blame: dim uninteresting metadata lines 2018-04-17 3:17 ` Junio C Hamano @ 2018-04-17 19:04 ` Stefan Beller 0 siblings, 0 replies; 18+ messages in thread From: Stefan Beller @ 2018-04-17 19:04 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ævar Arnfjörð Bjarmason Hi Junio, >> -#define OUTPUT_SHOW_SCORE 0100 >> -#define OUTPUT_NO_AUTHOR 0200 >> +#define OUTPUT_SHOW_SCORE 0100 >> +#define OUTPUT_NO_AUTHOR 0200 > > I wondered what these are about; they are about aligning with HT > assuming 8-place tab stop like the other lines. correct. >> -#define OUTPUT_LINE_PORCELAIN 01000 >> +#define OUTPUT_LINE_PORCELAIN 01000 > > But then this line has SP plus HT here; it should have been just a > single HT (i.e. replace a single SP with HT), to be consistent. fixed >> @@ -384,6 +388,19 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int >> char ch; >> int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev; >> >> + if (!(opt & OUTPUT_ANNOTATE_COMPAT)) { >> + if (opt & OUTPUT_COLOR_LINE) { >> + if (cnt > 0) { >> + color = repeated_meta_color; >> + reset = GIT_COLOR_RESET; >> + } else { >> + color =""; > > You need a SP after '=' that assigns an empty string to 'color'. > >> + reset = ""; >> + } >> + } >> + fputs(color, stdout); >> + } > > This fputs() should be hidden to the case where color is *NOT* an > empty string, no? In any case, it should be inside "if color-line > is in effect" block, I would think. > > Users of "git annotate" would not pass the --color option, so I do > not think the outer if () block is worth the loss of readability due > to increased indent level. > > I would say that it is a job of the caller of git_config() to make > sure color.blame.lines would not take effect when the command is > annotate, if what you are worried about is the configuration in this > code. ok, so we'll have to correct these mis aligned configs before hand and here we just go by the bits set in the flags. >> @@ -949,8 +979,12 @@ int cmd_blame(int argc, const char **argv, const char *prefix) >> >> blame_coalesce(&sb); >> >> - if (!(output_option & OUTPUT_PORCELAIN)) >> + if (!(output_option & OUTPUT_PORCELAIN)) { >> find_alignment(&sb, &output_option); >> + if (!*repeated_meta_color && >> + (output_option & OUTPUT_COLOR_LINE)) >> + strcpy(repeated_meta_color, GIT_COLOR_DARK); >> + } > > This code does not check OUTPUT_ANNOTATE_COMPAT because it assumes > that OUTPUT_COLOR_LINE won't be in output_option when we are working > in annotate compatibility mode. The deeper codepaths we saw above > should do the same. It should be enough to drop color-line when > anno-compat is set, or something like that, immediately after > reading the config and overriding them from the command line. Makes sense. > >> diff --git a/color.h b/color.h >> index cd0bcedd08..196be16058 100644 >> --- a/color.h >> +++ b/color.h >> @@ -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" > > I wonder if it is worth adding a new color only to give this a > different default. > > Traditionally, we use CYAN for lines that are less interesting than > others (e.g. hunk header), so reusing it might make the life easier > to the users, especially because I envision that we may want to > introduce another "logical" level to give another redirection > between the configuration keys like color.diff.frag and > color.blame.repeatedlines and the actual ANSI sequence like > "\033[36m". I.e. we update the system so that these two > configuration keys take "uninteresting" (which is one of the > "logical" colors) by default, and then map "uninteresting" to > "\033[36m" at the physical level by default. The users could then > change the mapping from "uninteresting" to "\033[1;30m" and > consistently modify both diff.frag and blame.repeated if they wanted > to. Of course, if they want them to be different, they can come up > with a different "logical" color and split the two. And from that > point of view, adding new colors to the default set we have above > will make life harder for the end users. That is a good longer term vision. I'll switch to cyan for now. Thanks, Stefan ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2] builtin/blame: highlight recently changed lines 2018-04-16 22:09 [PATCH 0/2] blame: color line by commit Stefan Beller 2018-04-16 22:09 ` [PATCH 1/2] builtin/blame: dim uninteresting metadata lines Stefan Beller @ 2018-04-16 22:09 ` Stefan Beller 2018-04-17 3:23 ` Junio C Hamano 2018-04-17 3:29 ` Junio C Hamano 1 sibling, 2 replies; 18+ messages in thread From: Stefan Beller @ 2018-04-16 22:09 UTC (permalink / raw) To: git; +Cc: avarab, Stefan Beller Choose 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. Signed-off-by: Stefan Beller <sbeller@google.com> --- Documentation/config.txt | 18 ++++++++++ builtin/blame.c | 78 +++++++++++++++++++++++++++++++++++++++- t/t8012-blame-colors.sh | 25 +++++++++++++ 3 files changed, 120 insertions(+), 1 deletion(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index a223232263..acc456e1fd 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1223,6 +1223,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 diff --git a/builtin/blame.c b/builtin/blame.c index b49fee70a9..9b1021f10d 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -24,6 +24,7 @@ #include "dir.h" #include "progress.h" #include "blame.h" +#include "string-list.h" static char blame_usage[] = N_("git blame [<options>] [<rev-opts>] [<rev>] [--] <file>"); @@ -323,6 +324,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_HEATED_LINES 04000 static void emit_porcelain_details(struct blame_origin *suspect, int repeat) { @@ -370,6 +372,63 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent, putchar('\n'); } +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; + + colorfield_nr = 0; + + /* Ideally this would be stripped and split at the same time? */ + string_list_split(&l, s, ',', -1); + 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; + case EXPECT_COLOR: + if (color_parse(item->string, colorfield[colorfield_nr].col)) + die(_("expecting a color: %s"), item->string); + next = EXPECT_DATE; + break; + } + } + + if (next == EXPECT_COLOR) + die (_("must end with a color")); + + colorfield[colorfield_nr].hop = TIME_MAX; +} + +static void setup_default_colors_heated_lines(void) +{ + parse_color_fields("blue,12 month ago,white,1 month ago,red"); +} + +static void determine_line_heat(struct blame_entry *ent, const char **dest_color) +{ + int i = 0; + struct commit_info ci; + get_commit_info(ent->suspect->commit, &ci, 1); + + while (i < colorfield_nr && ci.author_time > colorfield[i].hop) + i++; + + *dest_color = colorfield[i].col; +} + static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int opt) { int cnt; @@ -384,6 +443,12 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int oid_to_hex_r(hex, &suspect->commit->object.oid); cp = blame_nth_line(sb, ent->lno); + + if (opt & OUTPUT_HEATED_LINES) { + determine_line_heat(ent, &color); + reset = GIT_COLOR_RESET; + } + for (cnt = 0; cnt < ent->num_lines; cnt++) { char ch; int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev; @@ -451,7 +516,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int max_digits, ent->lno + 1 + cnt); } if (!(opt & OUTPUT_ANNOTATE_COMPAT) && - (opt & OUTPUT_COLOR_LINE)) + (opt & (OUTPUT_COLOR_LINE | OUTPUT_HEATED_LINES))) fputs(reset, stdout); do { ch = *cp++; @@ -636,6 +701,11 @@ static int git_blame_config(const char *var, const char *value, void *cb) *output_option |= OUTPUT_COLOR_LINE; return 0; } + if (!strcmp(var, "color.blame.highlightrecent")) { + parse_color_fields(value); + *output_option |= OUTPUT_HEATED_LINES; + return 0; + } if (git_diff_heuristic_config(var, value, cb) < 0) return -1; @@ -720,6 +790,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, "heated-lines", &output_option, N_("color lines by age"), OUTPUT_HEATED_LINES), /* * The following two options are parsed by parse_revision_opt() @@ -744,6 +815,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) unsigned int range_i; long anchor; + setup_default_colors_heated_lines(); git_config(git_blame_config, &output_option); init_revisions(&revs, NULL); revs.date_mode = blame_date_mode; @@ -779,6 +851,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_HEATED_LINES)) + die(_("cannot ask for colored lines and heated lines 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")); diff --git a/t/t8012-blame-colors.sh b/t/t8012-blame-colors.sh index bb0fe20e76..d6b2932bf8 100755 --- a/t/t8012-blame-colors.sh +++ b/t/t8012-blame-colors.sh @@ -26,4 +26,29 @@ test_expect_success 'colored blame colors contiguous lines via config' ' test_line_count = 3 H.expect ' +test_expect_success 'heated line coloring for old code' ' + git blame --heated-lines hello.c >actual.raw && + test_decode_color <actual.raw >actual && + grep "<BLUE>" <actual >colored && + test_line_count = 10 colored +' + +test_expect_success 'heated line coloring' ' + cat >>hello.c <<-EOF && + void qfunc(); + EOF + git add hello.c && + GIT_AUTHOR_DATE="" git commit -m "new commit" && + + git blame --heated-lines hello.c >actual.raw && + test_decode_color <actual.raw >actual && + + grep "<BLUE>" <actual >colored && + test_line_count = 10 colored && + + grep "<RED>" <actual >colored && + test_line_count = 1 colored && + grep qfunc colored +' + test_done -- 2.17.0.484.g0c8726318c-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] builtin/blame: highlight recently changed lines 2018-04-16 22:09 ` [PATCH 2/2] builtin/blame: highlight recently changed lines Stefan Beller @ 2018-04-17 3:23 ` Junio C Hamano 2018-04-17 3:29 ` Junio C Hamano 1 sibling, 0 replies; 18+ messages in thread From: Junio C Hamano @ 2018-04-17 3:23 UTC (permalink / raw) To: Stefan Beller; +Cc: git, avarab Stefan Beller <sbeller@google.com> writes: > Choose 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. > ... > @@ -323,6 +324,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_HEATED_LINES 04000 How about calling it OUTPUT_SHOW_AGE_WITH_COLOR or something like that instead? Anything with "AGE" in it, if that is what you are trying to indicate, would be more appropriate than "HEATED", which does not convey much meaning to readers unless you explain what determines the temperature in your mind. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] builtin/blame: highlight recently changed lines 2018-04-16 22:09 ` [PATCH 2/2] builtin/blame: highlight recently changed lines Stefan Beller 2018-04-17 3:23 ` Junio C Hamano @ 2018-04-17 3:29 ` Junio C Hamano 2018-04-17 19:31 ` Stefan Beller 1 sibling, 1 reply; 18+ messages in thread From: Junio C Hamano @ 2018-04-17 3:29 UTC (permalink / raw) To: Stefan Beller; +Cc: git, avarab It seems that this $ git -c color.blame.repeatedlines=cyan blame --heated-lines builtin/blame.c refuses to run. Would it work if the configuration is in .git/config instead, or would it forever disable --heated-lines once somebody choses to use --color-lines feature by default by configuring it in? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] builtin/blame: highlight recently changed lines 2018-04-17 3:29 ` Junio C Hamano @ 2018-04-17 19:31 ` Stefan Beller 2018-04-17 19:35 ` Stefan Beller 2018-04-18 0:12 ` [PATCH 2/2] builtin/blame: highlight recently changed lines Junio C Hamano 0 siblings, 2 replies; 18+ messages in thread From: Stefan Beller @ 2018-04-17 19:31 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ævar Arnfjörð Bjarmason On Mon, Apr 16, 2018 at 8:29 PM, Junio C Hamano <gitster@pobox.com> wrote: > It seems that this > > $ git -c color.blame.repeatedlines=cyan blame --heated-lines builtin/blame.c > > refuses to run. > > Would it work if the configuration is in .git/config instead, or > would it forever disable --heated-lines once somebody choses to use > --color-lines feature by default by configuring it in? That is the unfortunate part of this series, I have not figured out how to treat these two options at the same time. One could take the approach to check the config first and see if there are conflicts and then overlay it with the command line options (and resolve conflicts there, but CLI taking precedence over config). Or we'd need to introduce another config blame.coloring={none, repeatedlines, highlightrecent} which breaks the tie. Thanks, Stefan ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] builtin/blame: highlight recently changed lines 2018-04-17 19:31 ` Stefan Beller @ 2018-04-17 19:35 ` Stefan Beller 2018-04-17 21:30 ` [PATCH 1/2] builtin/blame: dim uninteresting metadata lines Stefan Beller 2018-04-18 0:12 ` [PATCH 2/2] builtin/blame: highlight recently changed lines Junio C Hamano 1 sibling, 1 reply; 18+ messages in thread From: Stefan Beller @ 2018-04-17 19:35 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Ævar Arnfjörð Bjarmason On Tue, Apr 17, 2018 at 12:31 PM, Stefan Beller <sbeller@google.com> wrote: > On Mon, Apr 16, 2018 at 8:29 PM, Junio C Hamano <gitster@pobox.com> wrote: >> It seems that this >> >> $ git -c color.blame.repeatedlines=cyan blame --heated-lines builtin/blame.c >> >> refuses to run. >> >> Would it work if the configuration is in .git/config instead, or >> would it forever disable --heated-lines once somebody choses to use >> --color-lines feature by default by configuring it in? > > That is the unfortunate part of this series, I have not figured out how to > treat these two options at the same time. > > One could take the approach to check the config first and see if there > are conflicts and then overlay it with the command line options > (and resolve conflicts there, but CLI taking precedence over config). > > Or we'd need to introduce another config > blame.coloring={none, repeatedlines, highlightrecent} > which breaks the tie. Nevermind, we can overlay these color schemes just fine, which I'll do in a resend. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] builtin/blame: dim uninteresting metadata lines 2018-04-17 19:35 ` Stefan Beller @ 2018-04-17 21:30 ` Stefan Beller 2018-04-17 21:30 ` [PATCH 2/2] builtin/blame: highlight recently changed lines Stefan Beller 2018-04-18 0:34 ` [PATCH 1/2] builtin/blame: dim uninteresting metadata lines Junio C Hamano 0 siblings, 2 replies; 18+ messages in thread From: Stefan Beller @ 2018-04-17 21:30 UTC (permalink / raw) To: sbeller; +Cc: avarab, git, gitster When 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. Both the command line option '--color-lines' as well as a config option 'color.blame.colorLines' is provided. Signed-off-by: Stefan Beller <sbeller@google.com> --- Documentation/config.txt | 5 +++++ builtin/blame.c | 42 ++++++++++++++++++++++++++++++++++++---- t/t8012-blame-colors.sh | 29 +++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 4 deletions(-) create mode 100755 t/t8012-blame-colors.sh diff --git a/Documentation/config.txt b/Documentation/config.txt index 2659153cb3..8128eee4f9 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1218,6 +1218,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 cyan. + color.ui:: This variable determines the default value for variables such as `color.diff` and `color.grep` that control the use of color diff --git a/builtin/blame.c b/builtin/blame.c index db38c0b307..5190ff5df3 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -7,6 +7,7 @@ #include "cache.h" #include "config.h" +#include "color.h" #include "builtin.h" #include "commit.h" #include "diff.h" @@ -46,6 +47,7 @@ static int xdl_opts; static int abbrev = -1; static int no_whole_file_rename; static int show_progress; +static char repeated_meta_color[COLOR_MAXLEN]; static struct date_mode blame_date_mode = { DATE_ISO8601 }; static size_t blame_date_width; @@ -316,10 +318,11 @@ static const char *format_time(timestamp_t time, const char *tz_str, #define OUTPUT_PORCELAIN 010 #define OUTPUT_SHOW_NAME 020 #define OUTPUT_SHOW_NUMBER 040 -#define OUTPUT_SHOW_SCORE 0100 -#define OUTPUT_NO_AUTHOR 0200 +#define OUTPUT_SHOW_SCORE 0100 +#define OUTPUT_NO_AUTHOR 0200 #define OUTPUT_SHOW_EMAIL 0400 -#define OUTPUT_LINE_PORCELAIN 01000 +#define OUTPUT_LINE_PORCELAIN 01000 +#define OUTPUT_COLOR_LINE 02000 static void emit_porcelain_details(struct blame_origin *suspect, int repeat) { @@ -375,6 +378,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int struct commit_info ci; char hex[GIT_MAX_HEXSZ + 1]; int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP); + const char *color = NULL, *reset = NULL; get_commit_info(suspect->commit, &ci, 1); oid_to_hex_r(hex, &suspect->commit->object.oid); @@ -384,6 +388,18 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int char ch; int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev; + if (opt & OUTPUT_COLOR_LINE) { + if (cnt > 0) { + color = repeated_meta_color; + reset = GIT_COLOR_RESET; + } else { + color = ""; + reset = ""; + } + } + if (color) + fputs(color, stdout); + if (suspect->commit->object.flags & UNINTERESTING) { if (blank_boundary) memset(hex, ' ', length); @@ -433,6 +449,8 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int printf(" %*d) ", max_digits, ent->lno + 1 + cnt); } + if (reset) + fputs(reset, stdout); do { ch = *cp++; putchar(ch); @@ -585,6 +603,8 @@ static const char *add_prefix(const char *prefix, const char *path) static int git_blame_config(const char *var, const char *value, void *cb) { + int *output_option = cb; + if (!strcmp(var, "blame.showroot")) { show_root = git_config_bool(var, value); return 0; @@ -607,6 +627,13 @@ static int git_blame_config(const char *var, const char *value, void *cb) parse_date_format(value, &blame_date_mode); return 0; } + if (!strcmp(var, "color.blame.repeatedlines")) { + if (color_parse_mem(value, strlen(value), repeated_meta_color)) + warning(_("invalid color '%s' in color.blame.repeatedLines"), + value); + *output_option |= OUTPUT_COLOR_LINE; + return 0; + } if (git_diff_heuristic_config(var, value, cb) < 0) return -1; @@ -690,6 +717,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) OPT_BIT('s', NULL, &output_option, N_("Suppress author name and timestamp (Default: off)"), OUTPUT_NO_AUTHOR), 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), /* * The following two options are parsed by parse_revision_opt() @@ -949,8 +977,14 @@ int cmd_blame(int argc, const char **argv, const char *prefix) blame_coalesce(&sb); - if (!(output_option & OUTPUT_PORCELAIN)) + if (!(output_option & OUTPUT_PORCELAIN)) { find_alignment(&sb, &output_option); + if (!*repeated_meta_color && + (output_option & OUTPUT_COLOR_LINE)) + strcpy(repeated_meta_color, GIT_COLOR_CYAN); + } + if (output_option & OUTPUT_ANNOTATE_COMPAT) + output_option &= ~OUTPUT_COLOR_LINE; output(&sb, output_option); free((void *)sb.final_buf); diff --git a/t/t8012-blame-colors.sh b/t/t8012-blame-colors.sh new file mode 100755 index 0000000000..9fc32ff813 --- /dev/null +++ b/t/t8012-blame-colors.sh @@ -0,0 +1,29 @@ +#!/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 "<CYAN>" <actual >darkened && + grep "(F" darkened > F.expect && + grep "(H" darkened > H.expect && + test_line_count = 2 F.expect && + test_line_count = 3 H.expect +' + +test_expect_success 'colored blame colors contiguous lines via config' ' + git -c color.blame.repeatedLines=yellow blame --abbrev=12 hello.c >actual.raw && + test_decode_color <actual.raw >actual && + grep "<YELLOW>" <actual >darkened && + grep "(F" darkened > F.expect && + grep "(H" darkened > H.expect && + test_line_count = 2 F.expect && + test_line_count = 3 H.expect +' + +test_done -- 2.17.0.484.g0c8726318c-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] builtin/blame: highlight recently changed lines 2018-04-17 21:30 ` [PATCH 1/2] builtin/blame: dim uninteresting metadata lines Stefan Beller @ 2018-04-17 21:30 ` Stefan Beller 2018-04-17 21:54 ` Eric Sunshine 2018-06-09 11:26 ` René Scharfe 2018-04-18 0:34 ` [PATCH 1/2] builtin/blame: dim uninteresting metadata lines Junio C Hamano 1 sibling, 2 replies; 18+ messages in thread From: Stefan Beller @ 2018-04-17 21:30 UTC (permalink / raw) To: sbeller; +Cc: avarab, git, gitster Choose 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. Signed-off-by: Stefan Beller <sbeller@google.com> # Conflicts: # builtin/blame.c --- Documentation/config.txt | 17 +++++++++ builtin/blame.c | 81 ++++++++++++++++++++++++++++++++++++++-- t/t8012-blame-colors.sh | 25 +++++++++++++ 3 files changed, 119 insertions(+), 4 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 8128eee4f9..eae88be662 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -1223,6 +1223,23 @@ color.blame.repeatedMeta:: is repeated meta information per line (such as commit id, author name, date and timezone). Defaults to cyan. +color.blame.highlightRecent:: + This can be used to color the metadata of a blame line depending + on age of the line. ++ +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 diff --git a/builtin/blame.c b/builtin/blame.c index 5190ff5df3..53999df511 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -24,6 +24,7 @@ #include "dir.h" #include "progress.h" #include "blame.h" +#include "string-list.h" static char blame_usage[] = N_("git blame [<options>] [<rev-opts>] [<rev>] [--] <file>"); @@ -323,6 +324,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_SHOW_AGE_WITH_COLOR 04000 static void emit_porcelain_details(struct blame_origin *suspect, int repeat) { @@ -370,6 +372,63 @@ static void emit_porcelain(struct blame_scoreboard *sb, struct blame_entry *ent, putchar('\n'); } +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; + + colorfield_nr = 0; + + /* Ideally this would be stripped and split at the same time? */ + string_list_split(&l, s, ',', -1); + 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; + case EXPECT_COLOR: + if (color_parse(item->string, colorfield[colorfield_nr].col)) + die(_("expecting a color: %s"), item->string); + next = EXPECT_DATE; + break; + } + } + + if (next == EXPECT_COLOR) + die (_("must end with a color")); + + colorfield[colorfield_nr].hop = TIME_MAX; +} + +static void setup_default_color_by_age(void) +{ + parse_color_fields("blue,12 month ago,white,1 month ago,red"); +} + +static void determine_line_heat(struct blame_entry *ent, const char **dest_color) +{ + int i = 0; + struct commit_info ci; + get_commit_info(ent->suspect->commit, &ci, 1); + + while (i < colorfield_nr && ci.author_time > colorfield[i].hop) + i++; + + *dest_color = colorfield[i].col; +} + static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int opt) { int cnt; @@ -378,12 +437,19 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int struct commit_info ci; char hex[GIT_MAX_HEXSZ + 1]; int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP); - const char *color = NULL, *reset = NULL; + const char *default_color = NULL, *color = NULL, *reset = NULL; get_commit_info(suspect->commit, &ci, 1); oid_to_hex_r(hex, &suspect->commit->object.oid); cp = blame_nth_line(sb, ent->lno); + + if (opt & OUTPUT_SHOW_AGE_WITH_COLOR) { + determine_line_heat(ent, &default_color); + color = default_color; + reset = GIT_COLOR_RESET; + } + for (cnt = 0; cnt < ent->num_lines; cnt++) { char ch; int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev; @@ -393,8 +459,8 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int color = repeated_meta_color; reset = GIT_COLOR_RESET; } else { - color = ""; - reset = ""; + color = default_color ? default_color : ""; + reset = default_color ? GIT_COLOR_RESET : ""; } } if (color) @@ -634,6 +700,11 @@ static int git_blame_config(const char *var, const char *value, void *cb) *output_option |= OUTPUT_COLOR_LINE; return 0; } + if (!strcmp(var, "color.blame.highlightrecent")) { + parse_color_fields(value); + *output_option |= OUTPUT_SHOW_AGE_WITH_COLOR; + return 0; + } if (git_diff_heuristic_config(var, value, cb) < 0) return -1; @@ -718,6 +789,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-by-age", &output_option, N_("color lines by age"), OUTPUT_SHOW_AGE_WITH_COLOR), /* * The following two options are parsed by parse_revision_opt() @@ -742,6 +814,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) unsigned int range_i; long anchor; + setup_default_color_by_age(); git_config(git_blame_config, &output_option); init_revisions(&revs, NULL); revs.date_mode = blame_date_mode; @@ -984,7 +1057,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix) strcpy(repeated_meta_color, GIT_COLOR_CYAN); } if (output_option & OUTPUT_ANNOTATE_COMPAT) - output_option &= ~OUTPUT_COLOR_LINE; + output_option &= ~(OUTPUT_COLOR_LINE | OUTPUT_SHOW_AGE_WITH_COLOR); output(&sb, output_option); free((void *)sb.final_buf); diff --git a/t/t8012-blame-colors.sh b/t/t8012-blame-colors.sh index 9fc32ff813..6f6be6c5dc 100755 --- a/t/t8012-blame-colors.sh +++ b/t/t8012-blame-colors.sh @@ -26,4 +26,29 @@ test_expect_success 'colored blame colors contiguous lines via config' ' test_line_count = 3 H.expect ' +test_expect_success 'color by age consistently colors old code' ' + git blame --color-by-age hello.c >actual.raw && + test_decode_color <actual.raw >actual && + grep "<BLUE>" <actual >colored && + test_line_count = 10 colored +' + +test_expect_success 'blame color by age: new code is different' ' + cat >>hello.c <<-EOF && + void qfunc(); + EOF + git add hello.c && + GIT_AUTHOR_DATE="" git commit -m "new commit" && + + git blame --color-by-age hello.c >actual.raw && + test_decode_color <actual.raw >actual && + + grep "<BLUE>" <actual >colored && + test_line_count = 10 colored && + + grep "<RED>" <actual >colored && + test_line_count = 1 colored && + grep qfunc colored +' + test_done -- 2.17.0.484.g0c8726318c-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] builtin/blame: highlight recently changed lines 2018-04-17 21:30 ` [PATCH 2/2] builtin/blame: highlight recently changed lines Stefan Beller @ 2018-04-17 21:54 ` Eric Sunshine 2018-04-18 0:39 ` Junio C Hamano 2018-06-09 11:26 ` René Scharfe 1 sibling, 1 reply; 18+ messages in thread From: Eric Sunshine @ 2018-04-17 21:54 UTC (permalink / raw) To: Stefan Beller Cc: Ævar Arnfjörð Bjarmason, Git List, Junio C Hamano On Tue, Apr 17, 2018 at 5:30 PM, Stefan Beller <sbeller@google.com> wrote: > Choose 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. > > Signed-off-by: Stefan Beller <sbeller@google.com> > > # Conflicts: > # builtin/blame.c Meh. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] builtin/blame: highlight recently changed lines 2018-04-17 21:54 ` Eric Sunshine @ 2018-04-18 0:39 ` Junio C Hamano 0 siblings, 0 replies; 18+ messages in thread From: Junio C Hamano @ 2018-04-18 0:39 UTC (permalink / raw) To: Eric Sunshine Cc: Stefan Beller, Ævar Arnfjörð Bjarmason, Git List Eric Sunshine <sunshine@sunshineco.com> writes: > On Tue, Apr 17, 2018 at 5:30 PM, Stefan Beller <sbeller@google.com> wrote: >> Choose 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. >> >> Signed-off-by: Stefan Beller <sbeller@google.com> >> >> # Conflicts: >> # builtin/blame.c > > Meh. Sloppy. Will edit it out. Thanks. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] builtin/blame: highlight recently changed lines 2018-04-17 21:30 ` [PATCH 2/2] builtin/blame: highlight recently changed lines Stefan Beller 2018-04-17 21:54 ` Eric Sunshine @ 2018-06-09 11:26 ` René Scharfe 2018-06-11 23:56 ` Stefan Beller 2018-06-14 17:37 ` Junio C Hamano 1 sibling, 2 replies; 18+ messages in thread From: René Scharfe @ 2018-06-09 11:26 UTC (permalink / raw) To: Stefan Beller; +Cc: avarab, git, gitster Am 17.04.2018 um 23:30 schrieb Stefan Beller: > +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; > + > + colorfield_nr = 0; > + > + /* Ideally this would be stripped and split at the same time? */ Why? Both approxidate() and color_parse() handle spaces. > + string_list_split(&l, s, ',', -1); > + 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; > + case EXPECT_COLOR: > + if (color_parse(item->string, colorfield[colorfield_nr].col)) > + die(_("expecting a color: %s"), item->string); > + next = EXPECT_DATE; > + break; > + } > + } > + > + if (next == EXPECT_COLOR) > + die (_("must end with a color")); > + > + colorfield[colorfield_nr].hop = TIME_MAX; > +} This adds a minor memory leak; fix below. -- >8 -- Subject: [PATCH] blame: release string_list after use in parse_color_fields() Signed-off-by: Rene Scharfe <l.s.r@web.de> --- builtin/blame.c | 1 + 1 file changed, 1 insertion(+) diff --git a/builtin/blame.c b/builtin/blame.c index 4202584f97..3295718841 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -411,6 +411,7 @@ static void parse_color_fields(const char *s) die (_("must end with a color")); colorfield[colorfield_nr].hop = TIME_MAX; + string_list_clear(&l, 0); } static void setup_default_color_by_age(void) -- 2.17.1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] builtin/blame: highlight recently changed lines 2018-06-09 11:26 ` René Scharfe @ 2018-06-11 23:56 ` Stefan Beller 2018-06-14 17:37 ` Junio C Hamano 1 sibling, 0 replies; 18+ messages in thread From: Stefan Beller @ 2018-06-11 23:56 UTC (permalink / raw) To: René Scharfe Cc: Ævar Arnfjörð Bjarmason, git, Junio C Hamano On Sat, Jun 9, 2018 at 4:26 AM René Scharfe <l.s.r@web.de> wrote: > > Am 17.04.2018 um 23:30 schrieb Stefan Beller: > > +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; > > + > > + colorfield_nr = 0; > > + > > + /* Ideally this would be stripped and split at the same time? */ > > Why? Both approxidate() and color_parse() handle spaces. I think that comment is stale; I remember experimenting with different internal structs to write a parser until I came up with this implementation. It is a left over. > This adds a minor memory leak; fix below. Thanks! > -- >8 -- > Subject: [PATCH] blame: release string_list after use in parse_color_fields() > > Signed-off-by: Rene Scharfe <l.s.r@web.de> > --- > builtin/blame.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/builtin/blame.c b/builtin/blame.c > index 4202584f97..3295718841 100644 > --- a/builtin/blame.c > +++ b/builtin/blame.c > @@ -411,6 +411,7 @@ static void parse_color_fields(const char *s) > die (_("must end with a color")); > > colorfield[colorfield_nr].hop = TIME_MAX; > + string_list_clear(&l, 0); > } > > static void setup_default_color_by_age(void) > -- > 2.17.1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] builtin/blame: highlight recently changed lines 2018-06-09 11:26 ` René Scharfe 2018-06-11 23:56 ` Stefan Beller @ 2018-06-14 17:37 ` Junio C Hamano 1 sibling, 0 replies; 18+ messages in thread From: Junio C Hamano @ 2018-06-14 17:37 UTC (permalink / raw) To: René Scharfe; +Cc: Stefan Beller, avarab, git René Scharfe <l.s.r@web.de> writes: > > This adds a minor memory leak; fix below. > > -- >8 -- > Subject: [PATCH] blame: release string_list after use in parse_color_fields() > > Signed-off-by: Rene Scharfe <l.s.r@web.de> > --- Thanks. Will apply. > builtin/blame.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/builtin/blame.c b/builtin/blame.c > index 4202584f97..3295718841 100644 > --- a/builtin/blame.c > +++ b/builtin/blame.c > @@ -411,6 +411,7 @@ static void parse_color_fields(const char *s) > die (_("must end with a color")); > > colorfield[colorfield_nr].hop = TIME_MAX; > + string_list_clear(&l, 0); > } > > static void setup_default_color_by_age(void) ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] builtin/blame: dim uninteresting metadata lines 2018-04-17 21:30 ` [PATCH 1/2] builtin/blame: dim uninteresting metadata lines Stefan Beller 2018-04-17 21:30 ` [PATCH 2/2] builtin/blame: highlight recently changed lines Stefan Beller @ 2018-04-18 0:34 ` Junio C Hamano 1 sibling, 0 replies; 18+ messages in thread From: Junio C Hamano @ 2018-04-18 0:34 UTC (permalink / raw) To: Stefan Beller; +Cc: avarab, git Stefan Beller <sbeller@google.com> writes: > @@ -375,6 +378,7 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int > struct commit_info ci; > char hex[GIT_MAX_HEXSZ + 1]; > int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP); > + const char *color = NULL, *reset = NULL; > > get_commit_info(suspect->commit, &ci, 1); > oid_to_hex_r(hex, &suspect->commit->object.oid); > @@ -384,6 +388,18 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int > char ch; > int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev; > > + if (opt & OUTPUT_COLOR_LINE) { > + if (cnt > 0) { > + color = repeated_meta_color; > + reset = GIT_COLOR_RESET; > + } else { > + color = ""; > + reset = ""; Shouldn't these be NULL as you do not want to fputs() these when not in painting mode anyway? Which would make it consistent with ... > + } > + } > + if (color) > + fputs(color, stdout); > + ... this thing which otherwise needs to be "if (color && *color)", but if you do NULL, can be left as-is ;-) > @@ -433,6 +449,8 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int > printf(" %*d) ", > max_digits, ent->lno + 1 + cnt); > } > + if (reset) > + fputs(reset, stdout); Likewise. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] builtin/blame: highlight recently changed lines 2018-04-17 19:31 ` Stefan Beller 2018-04-17 19:35 ` Stefan Beller @ 2018-04-18 0:12 ` Junio C Hamano 1 sibling, 0 replies; 18+ messages in thread From: Junio C Hamano @ 2018-04-18 0:12 UTC (permalink / raw) To: Stefan Beller; +Cc: git, Ævar Arnfjörð Bjarmason Stefan Beller <sbeller@google.com> writes: > On Mon, Apr 16, 2018 at 8:29 PM, Junio C Hamano <gitster@pobox.com> wrote: >> It seems that this >> >> $ git -c color.blame.repeatedlines=cyan blame --heated-lines builtin/blame.c >> >> refuses to run. >> >> Would it work if the configuration is in .git/config instead, or >> would it forever disable --heated-lines once somebody choses to use >> --color-lines feature by default by configuring it in? > > That is the unfortunate part of this series, I have not figured out how to > treat these two options at the same time. Perhaps I wasn't clear enough, but I did not want to use both at the same time. "git blame --color-lines --heated-lines" that errors out saying these cannot be used at the same time is an acceptable limitation. My sole complaint was that just like command line is used to override (weaker) configs, the wish to use "repeatedlines" painting by default expressed in the configuration form should be overriden when there is an explicit command line --heated-lines option that is incompatible with it. In this particular case, you might be able to come up with a scheme where both can be made effective at the same time, but the principle still stands, and that is the more important lesson I'd like to see you learn. Thanks. ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2018-06-14 17:37 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-16 22:09 [PATCH 0/2] blame: color line by commit Stefan Beller 2018-04-16 22:09 ` [PATCH 1/2] builtin/blame: dim uninteresting metadata lines Stefan Beller 2018-04-17 3:17 ` Junio C Hamano 2018-04-17 19:04 ` Stefan Beller 2018-04-16 22:09 ` [PATCH 2/2] builtin/blame: highlight recently changed lines Stefan Beller 2018-04-17 3:23 ` Junio C Hamano 2018-04-17 3:29 ` Junio C Hamano 2018-04-17 19:31 ` Stefan Beller 2018-04-17 19:35 ` Stefan Beller 2018-04-17 21:30 ` [PATCH 1/2] builtin/blame: dim uninteresting metadata lines Stefan Beller 2018-04-17 21:30 ` [PATCH 2/2] builtin/blame: highlight recently changed lines Stefan Beller 2018-04-17 21:54 ` Eric Sunshine 2018-04-18 0:39 ` Junio C Hamano 2018-06-09 11:26 ` René Scharfe 2018-06-11 23:56 ` Stefan Beller 2018-06-14 17:37 ` Junio C Hamano 2018-04-18 0:34 ` [PATCH 1/2] builtin/blame: dim uninteresting metadata lines Junio C Hamano 2018-04-18 0:12 ` [PATCH 2/2] builtin/blame: highlight recently changed lines Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).