Re: [PATCH v2] trailer: allow spaces in tokens

[Jonathan Tan <jonathantanmy@google.com>]

Junio C Hamano <gitster@pobox.com> writes:
> "Max Bernstein via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
> > Range-diff vs v1:
> >
> >  1:  404a6d1b193 ! 1:  4d490851ac2 trailer: allow spaces in tokens
> >      @@ Commit message
> >       
> >           Signed-off-by: Max Bernstein <max@bernsteinbear.com>
> >       
> >      + ## t/t4014-format-patch.sh ##
> >      +@@ t/t4014-format-patch.sh: test_expect_success 'signoff: not really a signoff' '
> >      + 	4:Subject: [PATCH] subject
> >      + 	8:
> >      + 	9:I want to mention about Signed-off-by: here.
> >      +-	10:
> >      +-	11:Signed-off-by: C O Mitter <committer@example.com>
> >      ++	10:Signed-off-by: C O Mitter <committer@example.com>
> >      + 	EOF
> >      + 	test_cmp expect actual
> >      + '
> 
> So, the updated code mistook the body of the message that is not a
> sign-off, because there is a colon on the line, the line does not
> begin with the colon, and everything before the colon is an alnum or
> a whitespace, so squashed the paragraph break before the real
> trailer block and the last line of the body and made it a body-less
> commit log message?
> 
> This might be a good demonstration of why it is a mistaken design to
> allow whitespaces, which may steer us toward fixing the documentation?
> 
> I dunno.  What do others think?

Yeah, this was the same issue we discussed a few years ago [1]. "git
commit -s" has never supported spaces before the colon (at least, it did
not support it before my patch set in [1] and does not support it now)
so it seems that having no spaces is already the established convention,
and I think that interpret-trailers should follow it. I would agree that
we should fix the documentation to match the current behavior.

[1] https://lore.kernel.org/git/a416ab9b-ff1f-9a71-3e58-60fd4f8a6b8e@google.com/