On 09/29/2016 02:56 PM, Junio C Hamano wrote:
Jonathan Tan <jonathantanmy@xxxxxxxxxx> writes:
This is somewhat of a follow-up to my previous e-mail with subject
"[PATCH] sequencer: support folding in rfc2822 footer" [1], in which I
proposed relaxing the definition of a commit message footer to allow
multiple-line field bodies (as described in RFC2822), but its strictness
was deemed deliberate.
It does not necessarily mean we can never change it when we did
something deliberately, though. With a good enough justification,
and with a transitition plan if the backward incompatibility is
severe enough to warrant one, we can change things.
I vaguely recall that there were some discussion on the definition
of "what's a trailer line" with folks from the kernel land, perhaps
while discussing the interpret-trailers topic. IIRC, when somebody
passes an improved version along, the resulting message's trailer
block may look like this:
Signed-off-by: Original Author <original@xxxxxxxxx>
[fixed typo in the variable names]
Signed-off-by: Somebhody Else <somebody@xxxxxxx>
and an obvious "wish" of theirs was to treat not just RFC2822-like
"a line that begins with token followed by a colon" but also these
short comments as part of the trailer block. Your original wish in
[*1*] is to also treat "a line that begin with a whitespace that
follows a line that begins with token followed by a colon" as part
of the trailer block and I personally think that is a reasonable
thing to wish for, too.
If we allowed arbitrary lines in the trailer block, this would solve my
original problem, yes.
I recall that I was somewhat surprised and dissapointed to see no
change to interpret-trailers when you tried [*1*], which was really
about improving the definition of what the trailer block is, by the
way.
Sorry, I had missed that.
Looking at that, it seems that sequencer.c started interpreting the last
paragraph of the commit message as a footer and adding an exception for
"cherry picked from" in commit b971e04 ("sequencer.c: always separate
"(cherry picked from" from commit body", 2013-02-12). So the
interpretations of sequencer.c and interpret-trailers were already
divergent, but I should have probably at least discussed that.
Below is a patch set that allows placing the "cherry picked from" line
without taking into account the definition of a commit message footer.
For example, "git cherry-pick -x" (with the appropriate configuration
variable or argument) would, to this commit message:
commit title
This is an explanatory paragraph.
Footer: foo
place the "(cherry picked from ...)" line below "commit title".
Would this be better?
It is not immediately obvious what such a change buys us. Wouldn't
the current code place that line below "Footer: foo"? I cannot
think of any reason why anybody would want to place "cherry-picked
from" immediately below the title and before the first line of the
body.
Yes, the current code would place it below "Footer: foo" without a blank
line before it, but if it thinks that the so-called footer is not
actually a footer, it would insert a blank line before that line.
As for a reason:
1) I do not have a specific reason for placing it in that exact
position, but I would like to be able to place the "cherry picked from"
line without affecting the last paragraph (specifically, without making
the "cherry picked from" line the only line in the last paragraph). It
seems to me that placing it below the title was the most straightforward
way to do that - this way, Git can have its own idea of what a footer
constitutes, and the user can treat it as completely separate from the
"cherry picked from" line mechanism.
1a) (Avoiding the footer might also be a good way of more clearly
defining what the footer is. For example, currently, "cherry picked
from" is treated as a special case in sequencer.c but not in trailer.c,
as far as I can tell. If we consistently avoided the footer, we wouldn't
need such a special case anywhere.)
2) The Linux kernel's repository has some "commit ... upstream." lines
in this position (below the commit title) - for example, in commit
dacc0987fd2e.
[1]
https://kernel.googlesource.com/pub/scm/linux/kernel/git/stable/linux-stable/+/dacc0987fd2e25a8b4b8c19778862ba12ce76d0a