Re: [PATCH] format-patch: use raw format for notes

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 18, 2025, 21:17 UTC
Message-ID: <xmqqy0x2yr6b.fsf@gitster.g>
In-Reply-To: <20250318180251.3712-1-taahol@utu.fi>

Tuomas Ahola <taahol@utu.fi> writes:

> The default formatting of commit notes by git format-patch --notes
> doesn't make a very good fit.  It would be more beneficial to use the
> raw format for CMIT_FMT_EMAIL and CMIT_FMT_MBOXRD.

Hmph. That is unfortunately quite subjective. "doesn't make a very good fit" why? "more benefitial" why?

And it turns out that using "raw" is not a good choice in the context of e-mailed patches. Read on.

Show 17 quoted lines

> Signed-off-by: Tuomas Ahola <taahol@utu.fi>
> ---
>  log-tree.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/log-tree.c b/log-tree.c
> index 8b184d6776..c40a7599d0 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -857,7 +857,9 @@ void show_log(struct rev_info *opt)
>  		int raw;
>  		struct strbuf notebuf = STRBUF_INIT;
>  
> -		raw = (opt->commit_format == CMIT_FMT_USERFORMAT);
> +		raw = (opt->commit_format == CMIT_FMT_USERFORMAT ||
> +		       opt->commit_format == CMIT_FMT_EMAIL ||
> +		       opt->commit_format == CMIT_FMT_MBOXRD);

After applying this patch and running

    $ git format-patch --notes=amlog -1

(where refs/notes/amlog holds commit to original e-mail mapping), I get this:

    ...
    Subject: [PATCH] format-patch: use raw format for notes

    ...
    Signed-off-by: Tuomas Ahola <taahol@utu.fi>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
    ---

    Notes (amlog):
        Message-Id: <20250318180251.3712-1-taahol@utu.fi>

     log-tree.c | 4 +++-
     1 file changed, 3 insertions(+), 1 deletion(-)
    ...

But with this patch in place, I instead get this:

    ...
    Subject: [PATCH] format-patch: use raw format for notes

    ...
    Signed-off-by: Tuomas Ahola <taahol@utu.fi>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
    ---
    Message-Id: <20250318180251.3712-1-taahol@utu.fi>

     log-tree.c | 4 +++-
     1 file changed, 3 insertions(+), 1 deletion(-)
    ...

There is no indication where the note came from, and more importantly, the contents of the note loses its crucial leading spaces that makes sure that any random lines in the note that happen to begin with "diff", "---", etc. are not mistaken as the beginning of the first patch.

So, no, this change is not a good thing to do, at least in its current form. Besides, unconditional change like this will break existing users.