Re: [PATCH v2 3/3] blame: add a config option to mark ignored lines

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Barret,

On Thu, 17 Jan 2019, Barret Rhoden wrote:

> When ignoring commits, the commit that is blamed might not be
> responsible for the change.  Users might want to know when a particular
> line has a potentially inaccurate blame.
> 
> By specifying blame.markIgnoredFiles, each blame line is marked with an
> '*'.  For example:
> 
> 278b6158d6fdb (Barret Rhoden  2016-04-11 13:57:54 -0400 26)
> 
> appears as:
> 
> *278b6158d6fd (Barret Rhoden  2016-04-11 13:57:54 -0400 26)
> 
> where the '*' is placed before the commit, and the hash has one fewer
> characters.
> 
> Signed-off-by: Barret Rhoden <brho@google.com>

Again, I cannot comment on blame.c, there are more competent people Cc:ed.
But I do have to point out that Git prefers commit messages in the
imperative form rather than the present tense (reading the commit message
could leave the inclined reader wondering what the patch changes, if Git
already does all that).

Ciao,
Johannes

> ---
>  Documentation/blame-options.txt | 4 +++-
>  blame.c                         | 8 +++++++-
>  blame.h                         | 1 +
>  builtin/blame.c                 | 9 +++++++++
>  4 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
> index 424a63f0b45c..92787ae951ac 100644
> --- a/Documentation/blame-options.txt
> +++ b/Documentation/blame-options.txt
> @@ -115,7 +115,9 @@ take effect.
>  	change never happened.  Lines that were changed or added by an ignored
>  	commit will be blamed on the previous commit that changed that line or
>  	nearby lines.  This option may be specified multiple times to ignore
> -	more than one revision.
> +	more than one revision.  If the `blame.markIgnoredLines` config option
> +	is set, then lines that were changed by an ignored commit will be
> +	marked with a `*` in the blame output.
>  
>  --ignore-revs-file <file>::
>  	Ignore revisions listed in `file`, one full SHA-1 hash per line.
> diff --git a/blame.c b/blame.c
> index 0b91fba2d04c..b1805633fb23 100644
> --- a/blame.c
> +++ b/blame.c
> @@ -474,7 +474,8 @@ void blame_coalesce(struct blame_scoreboard *sb)
>  
>  	for (ent = sb->ent; ent && (next = ent->next); ent = next) {
>  		if (ent->suspect == next->suspect &&
> -		    ent->s_lno + ent->num_lines == next->s_lno) {
> +		    ent->s_lno + ent->num_lines == next->s_lno &&
> +		    ent->ignored == next->ignored) {
>  			ent->num_lines += next->num_lines;
>  			ent->next = next->next;
>  			blame_origin_decref(next->suspect);
> @@ -726,6 +727,8 @@ static void split_overlap(struct blame_entry *split,
>  	int chunk_end_lno;
>  	memset(split, 0, sizeof(struct blame_entry [3]));
>  
> +	split[0].ignored = split[1].ignored = split[2].ignored = e->ignored;
> +
>  	if (e->s_lno < tlno) {
>  		/* there is a pre-chunk part not blamed on parent */
>  		split[0].suspect = blame_origin_incref(e->suspect);
> @@ -862,6 +865,7 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
>  			int len = tlno - e->s_lno;
>  			struct blame_entry *n = xcalloc(1, sizeof (struct blame_entry));
>  			n->suspect = e->suspect;
> +			n->ignored = e->ignored;
>  			n->lno = e->lno + len;
>  			n->s_lno = e->s_lno + len;
>  			n->num_lines = e->num_lines - len;
> @@ -916,6 +920,7 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
>  			int len = same - e->s_lno;
>  			struct blame_entry *n = xcalloc(1, sizeof (struct blame_entry));
>  			n->suspect = blame_origin_incref(e->suspect);
> +			n->ignored = e->ignored;
>  			n->lno = e->lno + len;
>  			n->s_lno = e->s_lno + len;
>  			n->num_lines = e->num_lines - len;
> @@ -930,6 +935,7 @@ static void blame_chunk(struct blame_entry ***dstq, struct blame_entry ***srcq,
>  			blame_origin_decref(e->suspect);
>  			e->suspect = blame_origin_incref(parent);
>  			e->s_lno += offset;
> +			e->ignored = 1;
>  			e->next = ignoredp;
>  			ignoredp = e;
>  		} else {
> diff --git a/blame.h b/blame.h
> index 086b92915e4b..56aeff582b01 100644
> --- a/blame.h
> +++ b/blame.h
> @@ -92,6 +92,7 @@ struct blame_entry {
>  	 * scanning the lines over and over.
>  	 */
>  	unsigned score;
> +	int ignored;
>  };
>  
>  /*
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 2f9183fb5fbd..8c3c5e435c9c 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -53,6 +53,7 @@ static int show_progress;
>  static char repeated_meta_color[COLOR_MAXLEN];
>  static int coloring_mode;
>  static const char *ignore_revs_file;
> +static int mark_ignored_lines;
>  
>  static struct date_mode blame_date_mode = { DATE_ISO8601 };
>  static size_t blame_date_width;
> @@ -480,6 +481,10 @@ static void emit_other(struct blame_scoreboard *sb, struct blame_entry *ent, int
>  			}
>  		}
>  
> +		if (mark_ignored_lines && ent->ignored) {
> +			length--;
> +			putchar('*');
> +		}
>  		printf("%.*s", length, hex);
>  		if (opt & OUTPUT_ANNOTATE_COMPAT) {
>  			const char *name;
> @@ -698,6 +703,10 @@ static int git_blame_config(const char *var, const char *value, void *cb)
>  	}
>  	if (!strcmp(var, "blame.ignorerevsfile"))
>  		return git_config_pathname(&ignore_revs_file, var, value);
> +	if (!strcmp(var, "blame.markignoredlines")) {
> +		mark_ignored_lines = git_config_bool(var, value);
> +		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"),
> -- 
> 2.20.1.321.g9e740568ce-goog
> 
>