{"thread":{"id":"40223","subject":"[PATCH v3] trailer: support multiline title","startedAt":"2015-08-30T19:14:40Z","lastAt":"2015-08-31T18:26:02Z","messageCount":7,"participants":["Christian Couder","Matthieu Moy","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"268945","messageId":"1440962080-24804-1-git-send-email-chriscool@tuxfamily.org","threadId":"40223","inReplyTo":null,"subject":"[PATCH v3] trailer: support multiline title","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-08-30T19:14:40Z","receivedAt":"2015-08-30T19:14:40Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"We currently ignore the first line passed to `git interpret-trailers`,\nwhen looking for the beginning of the trailers.\n\nUnfortunately this does not work well when a commit is created with a\nline break in the title, using for example the following command:\n\ngit commit -m 'place of\ncode: change we made'\n\nThat's why instead of ignoring only the first line, it is better to\nignore the first paragraph.\n---\n t/t7513-interpret-trailers.sh | 14 ++++++++++++++\n trailer.c | 15 +++++++++++----\n 2 files changed, 25 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 9577b4e..56efe88 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -112,6 +112,20 @@ test_expect_success 'with only a title in the message' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'with multiline title in the message' '\n+\tcat >expected <<-\\EOF &&\n+\t\tplace of\n+\t\tcode: change\n+\n+\t\tReviewed-by: Peff\n+\t\tAcked-by: Johan\n+\tEOF\n+\tprintf \"%s\\n%s\\n\" \"place of\" \"code: change\" |\n+\tgit interpret-trailers --trailer \"Reviewed-by: Peff\" \\\n+\t\t--trailer \"Acked-by: Johan\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'with config setup' '\n \tgit config trailer.ack.key \"Acked-by: \" &&\n \tcat >expected <<-\\EOF &&\ndiff --git a/trailer.c b/trailer.c\nindex b808868..6f3416f 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -735,15 +735,22 @@ static int find_patch_start(struct strbuf **lines, int count)\n */\n static int find_trailer_start(struct strbuf **lines, int count)\n {\n-\tint start, only_spaces = 1;\n+\tint start, end_of_title, only_spaces = 1;\n+\n+\t/* The first paragraph is the title and cannot be trailers */\n+\tfor (start = 0; start < count; start++) {\n+\t\tif (lines[start]->buf[0] == comment_line_char)\n+\t\t\tcontinue;\n+\t\tif (contains_only_spaces(lines[start]->buf))\n+\t\t\tbreak;\n+\t}\n+\tend_of_title = start;\n \n \t/*\n \t * Get the start of the trailers by looking starting from the end\n \t * for a line with only spaces before lines with one separator.\n-\t * The first line must not be analyzed as the others as it\n-\t * should be either the message title or a blank line.\n \t */\n-\tfor (start = count - 1; start >= 1; start--) {\n+\tfor (start = count - 1; start >= end_of_title; start--) {\n \t\tif (lines[start]->buf[0] == comment_line_char)\n \t\t\tcontinue;\n \t\tif (contains_only_spaces(lines[start]->buf)) {\n-- \n2.5.0.402.gcabd5e3.dirty\n"},{"id":"268968","messageId":"vpqtwrfg8dt.fsf@anie.imag.fr","threadId":"40223","inReplyTo":"1440962080-24804-1-git-send-email-chriscool@tuxfamily.org","subject":"Re: [PATCH v3] trailer: support multiline title","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-08-31T08:38:38Z","receivedAt":"2015-08-31T08:38:38Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Christian Couder writes:\n\n> That's why instead of ignoring only the first line, it is better to\n> ignore the first paragraph.\n> ---\n\nLacks sign-off again.\n\nThis replaces PATCH 2/2 in your previous series, but requires PATCH 1/2,\nright? If so that would be simpler to resend both patches IMHO.\n\nThe patch works for me, thanks.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"268971","messageId":"CAP8UFD3KH=5Y+Y3MKu7U-FkSTeu1qFkRYapne8LyNWVT_qF9iQ@mail.gmail.com","threadId":"40223","inReplyTo":"vpqtwrfg8dt.fsf@anie.imag.fr","subject":"Re: [PATCH v3] trailer: support multiline title","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-08-31T09:14:29Z","receivedAt":"2015-08-31T09:14:29Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Aug 31, 2015 at 10:38 AM, Matthieu Moy\n wrote:\n> Christian Couder writes:\n>\n>> That's why instead of ignoring only the first line, it is better to\n>> ignore the first paragraph.\n>> ---\n>\n> Lacks sign-off again.\n\nOoops, sorry.\n\n> This replaces PATCH 2/2 in your previous series, but requires PATCH 1/2,\n> right? If so that would be simpler to resend both patches IMHO.\n\nThis first patch is already in master. That's why I only sent the\nsecond one. But yeah I could have explained that after the three\ndashes.\n\n> The patch works for me, thanks.\n\nThanks for reviewing and testing,\nChristian.\n"},{"id":"268972","messageId":"vpqio7ves19.fsf@anie.imag.fr","threadId":"40223","inReplyTo":"CAP8UFD3KH=5Y+Y3MKu7U-FkSTeu1qFkRYapne8LyNWVT_qF9iQ@mail.gmail.com","subject":"Re: [PATCH v3] trailer: support multiline title","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-08-31T09:17:06Z","receivedAt":"2015-08-31T09:17:06Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Christian Couder writes:\n\n> This first patch is already in master. That's why I only sent the\n> second one. But yeah I could have explained that after the three\n> dashes.\n\nOK, my bad, I should have \"git fetch\"ed.\n\nThanks,\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"269013","messageId":"xmqq8u8rwclf.fsf@gitster.mtv.corp.google.com","threadId":"40223","inReplyTo":"1440962080-24804-1-git-send-email-chriscool@tuxfamily.org","subject":"Re: [PATCH v3] trailer: support multiline title","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-31T18:13:16Z","receivedAt":"2015-08-31T18:13:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> We currently ignore the first line passed to `git interpret-trailers`,\n> when looking for the beginning of the trailers.\n>\n> Unfortunately this does not work well when a commit is created with a\n> line break in the title, using for example the following command:\n>\n> git commit -m 'place of\n> code: change we made'\n>\n> That's why instead of ignoring only the first line, it is better to\n> ignore the first paragraph.\n> ---\n> t/t7513-interpret-trailers.sh | 14 ++++++++++++++\n> trailer.c | 15 +++++++++++----\n> 2 files changed, 25 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\n> index 9577b4e..56efe88 100755\n> --- a/t/t7513-interpret-trailers.sh\n> +++ b/t/t7513-interpret-trailers.sh\n> @@ -112,6 +112,20 @@ test_expect_success 'with only a title in the message' '\n> \ttest_cmp expected actual\n> '\n> \n> +test_expect_success 'with multiline title in the message' '\n> +\tcat >expected <<-\\EOF &&\n> +\t\tplace of\n> +\t\tcode: change\n> +\n> +\t\tReviewed-by: Peff\n> +\t\tAcked-by: Johan\n> +\tEOF\n> +\tprintf \"%s\\n%s\\n\" \"place of\" \"code: change\" |\n\nJust FYI, I think \"%s\\n\" is sufficient to produce multi-line output,\ne.g.\n\n\tprintf \"%s xyzzy\\n\" a b c d: e:\n\n> +\tgit interpret-trailers --trailer \"Reviewed-by: Peff\" \\\n> +\t\t--trailer \"Acked-by: Johan\" >actual &&\n> +\ttest_cmp expected actual\n> +'\n> +\n> test_expect_success 'with config setup' '\n> \tgit config trailer.ack.key \"Acked-by: \" &&\n> \tcat >expected <<-\\EOF &&\n> diff --git a/trailer.c b/trailer.c\n> index b808868..6f3416f 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -735,15 +735,22 @@ static int find_patch_start(struct strbuf **lines, int count)\n> */\n> static int find_trailer_start(struct strbuf **lines, int count)\n> {\n> -\tint start, only_spaces = 1;\n> +\tint start, end_of_title, only_spaces = 1;\n> +\n> +\t/* The first paragraph is the title and cannot be trailers */\n> +\tfor (start = 0; start < count; start++) {\n> +\t\tif (lines[start]->buf[0] == comment_line_char)\n> +\t\t\tcontinue;\n> +\t\tif (contains_only_spaces(lines[start]->buf))\n> +\t\t\tbreak;\n> +\t}\n> +\tend_of_title = start;\n> \n> \t/*\n> \t * Get the start of the trailers by looking starting from the end\n> \t * for a line with only spaces before lines with one separator.\n> -\t * The first line must not be analyzed as the others as it\n> -\t * should be either the message title or a blank line.\n> \t */\n> -\tfor (start = count - 1; start >= 1; start--) {\n> +\tfor (start = count - 1; start >= end_of_title; start--) {\n> \t\tif (lines[start]->buf[0] == comment_line_char)\n> \t\t\tcontinue;\n> \t\tif (contains_only_spaces(lines[start]->buf)) {\n"},{"id":"269014","messageId":"xmqq4mjfwcl0.fsf@gitster.mtv.corp.google.com","threadId":"40223","inReplyTo":"CAP8UFD3KH=5Y+Y3MKu7U-FkSTeu1qFkRYapne8LyNWVT_qF9iQ@mail.gmail.com","subject":"Re: [PATCH v3] trailer: support multiline title","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-31T18:13:31Z","receivedAt":"2015-08-31T18:13:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> On Mon, Aug 31, 2015 at 10:38 AM, Matthieu Moy\n> wrote:\n>> Christian Couder writes:\n>>\n>>> That's why instead of ignoring only the first line, it is better to\n>>> ignore the first paragraph.\n>>> ---\n>>\n>> Lacks sign-off again.\n>\n> Ooops, sorry.\n\nLet me forge one just for this time.\n\n>\n>> This replaces PATCH 2/2 in your previous series, but requires PATCH 1/2,\n>> right? If so that would be simpler to resend both patches IMHO.\n>\n> This first patch is already in master. That's why I only sent the\n> second one. But yeah I could have explained that after the three\n> dashes.\n>\n>> The patch works for me, thanks.\n>\n> Thanks for reviewing and testing,\n> Christian.\n"},{"id":"269015","messageId":"CAP8UFD0Y9HTzkGkrKnW3NABdGGVhzfcNPRaL6Ay9oNKMn+rLUA@mail.gmail.com","threadId":"40223","inReplyTo":"xmqq4mjfwcl0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v3] trailer: support multiline title","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-08-31T18:26:02Z","receivedAt":"2015-08-31T18:26:02Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Aug 31, 2015 at 8:13 PM, Junio C Hamano wrote:\n> Christian Couder writes:\n>\n>> On Mon, Aug 31, 2015 at 10:38 AM, Matthieu Moy\n>> wrote:\n>>> Christian Couder writes:\n>>>\n>>>> That's why instead of ignoring only the first line, it is better to\n>>>> ignore the first paragraph.\n>>>> ---\n>>>\n>>> Lacks sign-off again.\n>>\n>> Ooops, sorry.\n>\n> Let me forge one just for this time.\n\nGreat, thanks!\n"}]}