Subject: Re: [PATCH] format-patch: assume --cover-letter for diff in multi-patch series

From: Junio C Hamano <[email-masked]>
Date: Wed, 05 Jun 2024 11:17:46 -0700

Rubén Justo <[email-masked]> writes:

> diff --git a/builtin/log.c b/builtin/log.c
> index c8ce0c0d88..8032909d4f 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -2277,6 +2277,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  	if (cover_letter == -1) {
>  		if (config_cover_letter == COVER_AUTO)
>  			cover_letter = (total > 1);
> +		else if ((idiff_prev.nr || rdiff_prev) && (total > 1))
> +			cover_letter = (config_cover_letter != COVER_OFF);
>  		else
>  			cover_letter = (config_cover_letter == COVER_ON);
>  	}

Interesting.  So those who really really hate cover letters can set
the configuration explicitly to 'off' and giving an --interdiff
option would still have the sanity check kick in.  Makes sense.

> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
> index 7b05bf3961..5af155805d 100755
> --- a/t/t3206-range-diff.sh
> +++ b/t/t3206-range-diff.sh
> @@ -545,6 +545,12 @@ do
>  	'
>  done
>  
> +test_expect_success "format-patch --range-diff, implicit --cover-letter" '
> +	git format-patch -v2 --range-diff=topic main..unmodified &&
> +	test_when_finished "rm v2-000?-*" &&

I was about to make the follwoing:

    Swap these two.  Arrange things we are going to create to be
    removed at end, and then start creating them.  That way, we will
    clean them up even if we fail after creating some but before the
    end of the command.

but this one is littered with exiting breakage, so I'll let it pass.
Somebody will later go in and fix them all (#leftoverbits).

> +	test_grep "^Range-diff against v1:$" v2-0000-*
> +'

Isn't the name of the cover letter file always cover-letter.patch
unless you configure a custom --suffix (which is not the case here)?

> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index ba85b582c5..c844fbfe47 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -2492,6 +2492,12 @@ test_expect_success 'interdiff: solo-patch' '
>  	test_cmp expect actual
>  '
>  
> +test_expect_success 'interdiff: multi-patch, implicit --cover-letter' '
> +	git format-patch --interdiff=boop~2 -2 -v23 &&
> +	test_grep "^Interdiff against v22:$" v23-0000-cover-letter.patch &&
> +	test_cmp expect actual
> +'

OK.

>  test_expect_success 'format-patch does not respect diff.noprefix' '
>  	git -c diff.noprefix format-patch -1 --stdout >actual &&
>  	grep "^--- a/blorp" actual

Looking good.