Re: [PATCH] cherry-pick: Append -x line on separate paragraph

[Junio C Hamano <gitster@pobox.com>]

Robin Stocker <robin@nibor.org> writes:

>  		if (opts->record_origin) {
> +			/* Some implementations don't terminate message with final \n, so add it */
> +			if (msg.message[strlen(msg.message)-1] != '\n')
> +				strbuf_addch(&msgbuf, '\n');

I can agree that this is a good change.

> +			strbuf_addch(&msgbuf, '\n');

But this is somewhat dubious.  Even if what we are adding is merely
an extra LF, that changes the mechanically generated output format
and can break existing hooks that read from these generated commit
log template.

Is there a reason better than "having an empty line there look
better to _me_" to justify this change?

>  			strbuf_addstr(&msgbuf, "(cherry picked from commit ");
>  			strbuf_addstr(&msgbuf, sha1_to_hex(commit->object.sha1));
>  			strbuf_addstr(&msgbuf, ")\n");

Having said that, I've seen proposals to update this message to
format more like the other trailers, so that we would see this:

	The title of the original commit

	The log message taken from the original
        commit comes here.

	Signed-off-by: First person who signed off the original
        Signed-off-by: Another person who signed off the original
        Cherry-picked-from: a9bbc121ea850e49d52ba3cb5a6b7f8077d195d2
        
in the editor, to allow you to add your own Sign-off at the end to
make it look like this:

	The title of the original commit

	The log message taken from the original
        commit comes here.

	Signed-off-by: First person who signed off the original
        Signed-off-by: Another person who signed off the original
        Cherry-picked-from: a9bbc121ea850e49d52ba3cb5a6b7f8077d195d2
	Signed-off-by: Me who did the cherry-pick

I think that might be a worthwhile thing to do perhaps as an
optional behaviour (e.g. perhaps triggered with a new option
"--trailer", or with the same "-x" but only when "cherry-pick.origin
= trailer" configuration is set, or something).  At that point, the
output will look vastly different to existing hooks and those who
care how this field looks like are forced to be updated, but as long
as it is an opt-in feature, it may be worth it.