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

From: Patrick Steinhardt <ps@pks.im>
Date: Mar 28, 2025, 07:00 UTC
Message-ID: <Z-ZI-M2jBf5pqMoA@pks.im>
In-Reply-To: <20250326-514-git-blame-1-s-porcelain-output-does-not-emit-unblamable-and-ignored-markers-v2-1-79037e17a74b@gmail.com>

On Wed, Mar 26, 2025 at 10:06:10PM +0100, Karthik Nayak wrote:

Show 14 quoted lines

> 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?

Show 7 quoted lines

> +{
> +	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.

Show 18 quoted lines

> 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