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

From: Junio C Hamano <[email-masked]>
Date: Wed, 06 Sep 2017 12:34:48 +0900

Sam Bobroff <[email-masked]> writes:

> If "--notes=..." is used with "git format-patch", the notes are
> prefixed with the ref's local name and indented, which looks odd and
> exposes the path of the ref.
>
> Extend the test that suppresses this behaviour so that it also catches
> this case, causing the notes to be included without additional
> processing.
>
> Signed-off-by: Sam Bobroff <[email-masked]>
> ---
>
> Notes (foo):
>     Hi,
>     
>     I've noticed what appears to be a small cosmetic bug in git format-patch, as
>     I've described in the commit message.
>     
>     I'm not sure if this patch is the right way to fix it (or perhaps it's not even
>     a bug), but it should at least help to explain what I'm talking about.
>     
>     I've used "git format-patch --notes=foo" to prepare this email so that it is an
>     example of the issue :-)
>     
>     Cheers,
>     Sam.

Is the above addition from your 'foo' notes with or without this
patch?  I think the answer is "without", and the above "example"
looks just fine to me.

 - It is very much intended to allow The "(foo)" after the "Notes"
   label to show which notes ref the note comes from, because there
   can be more than one notes refs that annotate the same commit.

 - And the contents are indented, just like the diffstat and other
   stuff we place after "---" but before the first "diff", to ensure
   no matter what text appears there it will not be mistaken as part
   sure that the contents from the notes will not be mistaken as part
   of the patch.

I do not think an unconditional change of the established format,
like your patch does, is acceptable, as existing users have relied
on, and expect to be able to continue relying on, the above two
aspect of the current format.

But I am somewhat curious what your use case that wants to insert
the raw contents there is.  We may be able to construct a valid
argument to add such an output as an optional feature if there is a
good use case for it.

Thanks.

>  log-tree.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/log-tree.c b/log-tree.c
> index 410ab4f02..26bc21ad3 100644
> --- a/log-tree.c
> +++ b/log-tree.c
> @@ -655,7 +655,8 @@ 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);
>  		format_display_notes(&commit->object.oid, &notebuf,
>  				     get_log_output_encoding(), raw);
>  		ctx.notes_message = notebuf.len