{"thread":{"id":"45592","subject":"[PATCH] Fix 'git am' in-body header continuations","startedAt":"2017-04-03T00:50:01Z","lastAt":"2017-04-04T06:49:06Z","messageCount":3,"participants":["Linus Torvalds","Jonathan Tan","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"316078","messageId":"alpine.LFD.2.20.1704021746180.22832@i7.lan","threadId":"45592","inReplyTo":null,"subject":"[PATCH] Fix 'git am' in-body header continuations","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2017-04-03T00:49:54Z","receivedAt":"2017-04-03T00:50:01Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\nFrom: Linus Torvalds \nDate: Sat, 1 Apr 2017 12:14:39 -0700\nSubject: [PATCH] Fix 'git am' in-body header continuations\n\nAn empty line should stop any pending in-body headers, and start the\nactual body parsing.\n\nThis also modifies the original test for the in-body headers to actually\nhave a real commit body that starts with spaces, and changes the test to\ncheck that the long line matches _exactly_, and doesn't get extra data\nfrom the body.\n\nFixes:6b4b013f1884 (\"mailinfo: handle in-body header continuations\")\nCc: Jonathan Tan \nCc: Jeff King \nSigned-off-by: Linus Torvalds \n---\n\nOn Sun, 2 Apr 2017, Junio C Hamano wrote:\n> \n> And that is exactly your patch does. The change \"feels\" correct to\n> me.\n\nOk, resent with the test-case for the original behavior changed to be \nstricter (so it fails without this fix), and with Signed-off lines etc.\n\nI didn't really test the test-case very much, but it seemed to fail \nwithout this patch (because the \"Body test\" thing from the body becomes \npart of the long first line), and passes with it.\n\nBut somebody who is more used to the test-suite should double-check my \nstupid test edit.\n\n mailinfo.c | 7 ++++++-\n t/t4150-am.sh | 6 ++++--\n 2 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/mailinfo.c b/mailinfo.c\nindex a489d9d0f..68037758f 100644\n--- a/mailinfo.c\n+++ b/mailinfo.c\n@@ -757,8 +757,13 @@ static int handle_commit_msg(struct mailinfo *mi, struct strbuf *line)\n \tassert(!mi->filter_stage);\n \n \tif (mi->header_stage) {\n-\t\tif (!line->len || (line->len == 1 && line->buf[0] == '\\n'))\n+\t\tif (!line->len || (line->len == 1 && line->buf[0] == '\\n')) {\n+\t\t\tif (mi->inbody_header_accum.len) {\n+\t\t\t\tflush_inbody_header_accum(mi);\n+\t\t\t\tmi->header_stage = 0;\n+\t\t\t}\n \t\t\treturn 0;\n+\t\t}\n \t}\n \n \tif (mi->use_inbody_headers && mi->header_stage) {\ndiff --git a/t/t4150-am.sh b/t/t4150-am.sh\nindex 89a5bacac..44807e218 100755\n--- a/t/t4150-am.sh\n+++ b/t/t4150-am.sh\n@@ -983,7 +983,9 @@ test_expect_success 'am works with multi-line in-body headers' '\n \trm -fr .git/rebase-apply &&\n \tgit checkout -f first &&\n \techo one >> file &&\n-\tgit commit -am \"$LONG\" --author=\"$LONG \" &&\n+\tgit commit -am \"$LONG\n+\n+ Body test\" --author=\"$LONG \" &&\n \tgit format-patch --stdout -1 >patch &&\n \t# bump from, date, and subject down to in-body header\n \tperl -lpe \"\n@@ -997,7 +999,7 @@ test_expect_success 'am works with multi-line in-body headers' '\n \tgit am msg &&\n \t# Ensure that the author and full message are present\n \tgit cat-file commit HEAD | grep \"^author.*long@example.com\" &&\n-\tgit cat-file commit HEAD | grep \"^$LONG\"\n+\tgit cat-file commit HEAD | grep \"^$LONG$\"\n '\n \n test_done\n-- \n2.12.2.578.g5c4e54f4e\n\n"},{"id":"316101","messageId":"f5763dab-b829-f216-a377-8a71fc4f0c1e@google.com","threadId":"45592","inReplyTo":"alpine.LFD.2.20.1704021746180.22832@i7.lan","subject":"Re: [PATCH] Fix 'git am' in-body header continuations","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-04-03T18:00:09Z","receivedAt":"2017-04-03T18:00:17Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"This looks good to me.\n\nOn 04/02/2017 05:49 PM, Linus Torvalds wrote:\n>\n> From: Linus Torvalds \n> Date: Sat, 1 Apr 2017 12:14:39 -0700\n> Subject: [PATCH] Fix 'git am' in-body header continuations\n>\n> An empty line should stop any pending in-body headers, and start the\n> actual body parsing.\n>\n> This also modifies the original test for the in-body headers to actually\n> have a real commit body that starts with spaces, and changes the test to\n> check that the long line matches _exactly_, and doesn't get extra data\n> from the body.\n>\n> Fixes:6b4b013f1884 (\"mailinfo: handle in-body header continuations\")\n> Cc: Jonathan Tan \n> Cc: Jeff King \n> Signed-off-by: Linus Torvalds \n> ---\n> diff --git a/t/t4150-am.sh b/t/t4150-am.sh\n> index 89a5bacac..44807e218 100755\n> --- a/t/t4150-am.sh\n> +++ b/t/t4150-am.sh\n> @@ -983,7 +983,9 @@ test_expect_success 'am works with multi-line in-body headers' '\n> \trm -fr .git/rebase-apply &&\n> \tgit checkout -f first &&\n> \techo one >> file &&\n> -\tgit commit -am \"$LONG\" --author=\"$LONG \" &&\n> +\tgit commit -am \"$LONG\n> +\n> + Body test\" --author=\"$LONG \" &&\n\nInstead of \"Body test\", I would write something more descriptive like \n\"Not a continuation line because of blank line above\", but I'm fine with \neither.\n\n> \tgit format-patch --stdout -1 >patch &&\n> \t# bump from, date, and subject down to in-body header\n> \tperl -lpe \"\n> @@ -997,7 +999,7 @@ test_expect_success 'am works with multi-line in-body headers' '\n> \tgit am msg &&\n> \t# Ensure that the author and full message are present\n> \tgit cat-file commit HEAD | grep \"^author.*long@example.com\" &&\n> -\tgit cat-file commit HEAD | grep \"^$LONG\"\n> +\tgit cat-file commit HEAD | grep \"^$LONG$\"\n> '\n>\n> test_done\n>\n"},{"id":"316135","messageId":"20170404064844.argmnrdb7tuk2wkj@sigill.intra.peff.net","threadId":"45592","inReplyTo":"f5763dab-b829-f216-a377-8a71fc4f0c1e@google.com","subject":"Re: [PATCH] Fix 'git am' in-body header continuations","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-04-04T06:48:45Z","receivedAt":"2017-04-04T06:49:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Apr 03, 2017 at 11:00:09AM -0700, Jonathan Tan wrote:\n\n> > diff --git a/t/t4150-am.sh b/t/t4150-am.sh\n> > index 89a5bacac..44807e218 100755\n> > --- a/t/t4150-am.sh\n> > +++ b/t/t4150-am.sh\n> > @@ -983,7 +983,9 @@ test_expect_success 'am works with multi-line in-body headers' '\n> > \trm -fr .git/rebase-apply &&\n> > \tgit checkout -f first &&\n> > \techo one >> file &&\n> > -\tgit commit -am \"$LONG\" --author=\"$LONG \" &&\n> > +\tgit commit -am \"$LONG\n> > +\n> > + Body test\" --author=\"$LONG \" &&\n> \n> Instead of \"Body test\", I would write something more descriptive like \"Not a\n> continuation line because of blank line above\", but I'm fine with either.\n\nYeah. I also wonder if we can make the indentation more obvious. I\nthought at first that the patch was whitespace mangled. :-/\n\nMaybe:\n\n SP=\" \" &&\n cat >msg <<-EOF &&\n $LONG\n\n $SP This line is indented but not a header continuation.\n EOF\n git commit -F msg ...\n\nor something.\n\nIt might also be easier to understand what's going on if this gets its\nown test. This is really just testing mailinfo. I wonder if it would\nmake more sense in t5100, where we would not have to deal with all the\ncommit/format-patch cruft.\n\n-Peff\n"}]}