Re: [PATCH v2] blame: print unblamable and ignored commits in porcelain mode

[Patrick Steinhardt <ps@pks.im>]

On Wed, Mar 26, 2025 at 10:06:10PM +0100, Karthik Nayak wrote:
> diff --git a/builtin/blame.c b/builtin/blame.c
> index c470654c7e..528bfef249 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -351,6 +351,19 @@ static void emit_porcelain_details(struct blame_origin *suspect, int repeat)
>  		write_filename_info(suspect);
>  }
>  
> +/*
> + * Information which needs to be printed per-line goes here. Any
> + * information which can be clubbed on a commit/file level, should
> + * be printed via 'emit_one_suspect_detail()'.
> + */
> +static void emit_per_line_details(struct blame_entry *ent)

Tiny nit, feel free to ignore: should this something like
`emit_porcelain_per_line_details()` to highlight that this is part of
the porcelain format?

> +{
> +	if (mark_unblamable_lines && ent->unblamable)
> +		printf("unblamable\n");
> +	if (mark_ignored_lines && ent->ignored)
> +		printf("ignored\n");
> +}
> +

Another tiny nit: you may use `puts()` instead of `printf()`. I don't
mind it much though, both versions work equally well.

> diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh
> index 370b768149..306fc61057 100755
> --- a/t/t8013-blame-ignore-revs.sh
> +++ b/t/t8013-blame-ignore-revs.sh
> @@ -158,6 +158,16 @@ test_expect_success mark_unblamable_lines '
>  	test_cmp expect actual
>  '
>  
> +for opt in --porcelain --line-porcelain
> +do
> +	test_expect_success 'mark_unblamable_lines with $opt' '
> +		sha=$(git rev-parse Y) &&
> +
> +		git -c blame.markUnblamableLines=true blame $opt --ignore-rev Y file >actual &&
> +		test $(grep ^unblamable actual | wc -l) -eq 2
> +	'
> +done
> +

Okay, makes sense. We cannot batch the information on the first time
we've seen the commit here because both the "unblamable" and "ignored"
properties are properties of the line, not of the commit. So we'd expect
to see the information per line in both modes.

Patrick