git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: Brandon Casey <drafnel@gmail.com>
Cc: gitster@pobox.com, pclouds@gmail.com, git@vger.kernel.org,
	Brandon Casey <bcasey@nvidia.com>
Subject: Re: [PATCH v2 01/10] sequencer.c: remove broken support for rfc2822 continuation in footer
Date: Tue, 22 Jan 2013 02:12:08 -0800	[thread overview]
Message-ID: <20130122101208.GC8908@elie.Belkin> (raw)
In-Reply-To: <CA+sFfMfERgGbf58LjOfAhhO_-YLu+yo+L-vYMuAuOUaP90yvgA@mail.gmail.com>

Brandon Casey wrote:
> On Mon, Jan 21, 2013 at 11:54 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:

>>> -             if ((buf[k] == ' ' || buf[k] == '\t') && !first)
>>> -                     continue;
>>
>> This is always the first line examined, so this "continue" never
>> triggers.
>
> This is just totally broken and always has been.  The index variable
> should be 'i' not 'k'.

Yes, now I see.

This test trips when the *next* line starts with ' ' or '\t'.

	commit, cherry-pick -s: remove broken support for multiline rfc2822 fields

	Starting with c1e01b0c (commit: More generous accepting of RFC-2822
	footer lines, 2009-10-28), "git commit -s" carefully parses the last
	paragraph of each commit message to check if it consists only of
	RFC2822-style headers, in which case the signoff will be added as a
	new line in the same list:

		Reported-by: Reporter <reporter@example.com>
		Signed-off-by: Author <author@example.com>
		Acked-by: Lieutenant <lt@example.com>

	It even included support for accepting indented continuation lines for
	multiline fields.  Unfortunately the multiline field support is broken
	because it checks whether buf[k] (the first character of the *next*
	line) instead of buf[i] is a whitespace character.  So for example, it
	confuses the following for a well-formed RFC2822 footer:

		Reported-by: Reporter <reporter@example.com>
		LINE NOISE
		 continuation
		 continuation

	A typical well-formed footer with multiline fields is not accepted:

		Reported-by: Re Porter
		 <reporter@example.com>
		Signed-off-by: Author <author@example.com>

	That this has remained broken for so long is a good sign that nobody
	actually needed continuation lines.  Rip out the broken continuation
	support.

  parent reply	other threads:[~2013-01-22 10:12 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-21  8:40 [PATCH v2 00/10] unify appending of sob Brandon Casey
2013-01-21  8:40 ` [PATCH v2 01/10] sequencer.c: remove broken support for rfc2822 continuation in footer Brandon Casey
2013-01-22  7:54   ` Jonathan Nieder
2013-01-22  9:35     ` Brandon Casey
2013-01-22  9:47       ` Jonathan Nieder
2013-01-22  9:49         ` Jonathan Nieder
2013-01-22 11:08           ` Brandon Casey
2013-01-22 10:12       ` Jonathan Nieder [this message]
2013-01-22 10:21         ` Jonathan Nieder
2013-01-21  8:40 ` [PATCH v2 02/10] t/test-lib-functions.sh: allow to specify the tag name to test_commit Brandon Casey
2013-01-22  8:02   ` Jonathan Nieder
2013-01-22  9:43     ` Brandon Casey
2013-01-21  8:40 ` [PATCH v2 03/10] t/t3511: add some tests of 'cherry-pick -s' functionality Brandon Casey
2013-01-22  8:17   ` Jonathan Nieder
2013-01-27 23:33     ` Brandon Casey
2013-01-27 23:40       ` Jonathan Nieder
2013-01-21  8:40 ` [PATCH v2 04/10] sequencer.c: recognize "(cherry picked from ..." as part of s-o-b footer Brandon Casey
2013-01-22  8:27   ` Jonathan Nieder
2013-01-28  1:24   ` Jonathan Nieder
2013-01-21  8:40 ` [PATCH v2 05/10] sequencer.c: always separate "(cherry picked from" from commit body Brandon Casey
2013-01-22  8:32   ` Jonathan Nieder
2013-01-21  8:40 ` [PATCH v2 06/10] sequencer.c: teach append_signoff how to detect duplicate s-o-b Brandon Casey
2013-01-22  8:38   ` Jonathan Nieder
2013-01-28  0:36     ` Brandon Casey
2013-01-28  2:06       ` Junio C Hamano
2013-02-09 23:06         ` Junio C Hamano
2013-02-09 23:49           ` Brandon Casey
2013-01-21  8:40 ` [PATCH v2 07/10] sequencer.c: teach append_signoff to avoid adding a duplicate newline Brandon Casey
2013-01-21  8:40 ` [PATCH v2 08/10] t4014: more tests about appending s-o-b lines Brandon Casey
2013-01-21  8:40 ` [PATCH v2 09/10] format-patch: update append_signoff prototype Brandon Casey
2013-01-21  8:40 ` [PATCH v2 10/10] Unify appending signoff in format-patch, commit and sequencer Brandon Casey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130122101208.GC8908@elie.Belkin \
    --to=jrnieder@gmail.com \
    --cc=bcasey@nvidia.com \
    --cc=drafnel@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).