{"thread":{"id":"36511","subject":"[RFC/PATCH 1/2] trailer: fix to ignore any line starting with '#'","startedAt":"2014-04-27T20:12:35Z","lastAt":"2014-04-28T22:12:51Z","messageCount":6,"participants":["Christian Couder","Michael Haggerty","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"239821","messageId":"20140427200327.16880.53255.chriscool@tuxfamily.org","threadId":"36511","inReplyTo":null,"subject":"[RFC/PATCH 0/2] fix and examples for git interpret-trailers","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-04-27T20:12:35Z","receivedAt":"2014-04-27T20:12:35Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"As I sent version 11 of the \"Add interpret-trailers builtin\" only a few\ndays ago, I am sending now a short series on top instead of another full\nversion.\n\nChristian Couder (2):\n  trailer: fix to ignore any line starting with '#'\n  trailer: add examples to the documentation\n\n Documentation/git-interpret-trailers.txt | 98 +++++++++++++++++++++++++++++++-\n t/t7513-interpret-trailers.sh            | 26 +++++++++\n trailer.c                                | 29 ++++++----\n 3 files changed, 141 insertions(+), 12 deletions(-)\n\n-- \n1.9.rc0.17.g651113e\n"},{"id":"239820","messageId":"20140427201238.16880.13774.chriscool@tuxfamily.org","threadId":"36511","inReplyTo":"20140427200327.16880.53255.chriscool@tuxfamily.org","subject":"[RFC/PATCH 1/2] trailer: fix to ignore any line starting with '#'","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-04-27T20:12:36Z","receivedAt":"2014-04-27T20:12:36Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"It looks like the commit-msg hook is passed a commit\nmessage that can contain lines starting with a '#'.\nThose comment lines will be removed from the commit\nmessage after the hook is run.\n\nIf we want \"git interpret-trailers\" to be able to\nprocess commit messages correctly in the commit-msg\nhook we need to ignore those lines.\n\nSigned-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n t/t7513-interpret-trailers.sh | 26 ++++++++++++++++++++++++++\n trailer.c                     | 29 ++++++++++++++++++-----------\n 2 files changed, 44 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 9aae721..aa63b1b 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -150,6 +150,32 @@ test_expect_success 'with 2 files arguments' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'with message that has comments' '\n+\tcat basic_message >>message_with_comments &&\n+\tsed -e \"s/ Z\\$/ /\" >>message_with_comments <<-\\EOF &&\n+\t\t# comment\n+\n+\t\t# other comment\n+\t\tCc: Z\n+\t\t# yet another comment\n+\t\tReviewed-by: Johan\n+\t\tReviewed-by: Z\n+\t\t# last comment\n+\n+\tEOF\n+\tcat basic_patch >>message_with_comments &&\n+\tcat basic_message >expected &&\n+\tcat >>expected <<-\\EOF &&\n+\t\t# comment\n+\n+\t\tCc: Peff\n+\t\tReviewed-by: Johan\n+\tEOF\n+\tcat basic_patch >>expected &&\n+\tgit interpret-trailers --trim-empty --trailer \"Cc: Peff\" message_with_comments >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'with commit complex message and trailer args' '\n \tcat complex_message_body >expected &&\n \tsed -e \"s/ Z\\$/ /\" >>expected <<-\\EOF &&\ndiff --git a/trailer.c b/trailer.c\nindex 4d32b42..81b2c5c 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -644,10 +644,9 @@ static int find_patch_start(struct strbuf **lines, int count)\n /*\n  * Return the (0 based) index of the first trailer line or count if\n  * there are no trailers. Trailers are searched only in the lines from\n- * index (count - 1) down to index 0. The has_blank_line parameter\n- * tells if there is a blank line before the trailers.\n+ * index (count - 1) down to index 0.\n  */\n-static int find_trailer_start(struct strbuf **lines, int count, int *has_blank_line)\n+static int find_trailer_start(struct strbuf **lines, int count)\n {\n \tint start, only_spaces = 1;\n \n@@ -656,10 +655,11 @@ static int find_trailer_start(struct strbuf **lines, int count, int *has_blank_l\n \t * for a line with only spaces before lines with one ':'.\n \t */\n \tfor (start = count - 1; start >= 0; start--) {\n+\t\tif (lines[start]->buf[0] == '#')\n+\t\t\tcontinue;\n \t\tif (contains_only_spaces(lines[start]->buf)) {\n \t\t\tif (only_spaces)\n \t\t\t\tcontinue;\n-\t\t\t*has_blank_line = 1;\n \t\t\treturn start + 1;\n \t\t}\n \t\tif (strchr(lines[start]->buf, ':')) {\n@@ -667,13 +667,20 @@ static int find_trailer_start(struct strbuf **lines, int count, int *has_blank_l\n \t\t\t\tonly_spaces = 0;\n \t\t\tcontinue;\n \t\t}\n-\t\t*has_blank_line = start == count - 1 ?\n-\t\t  0 : contains_only_spaces(lines[start + 1]->buf);\n \t\treturn count;\n \t}\n \n-\t*has_blank_line = only_spaces ? count > 0 : 0;\n-\treturn only_spaces ? count : start + 1;\n+\treturn only_spaces ? count : 0;\n+}\n+\n+static int has_blank_line_before(struct strbuf **lines, int start)\n+{\n+\tfor (;start >= 0; start--) {\n+\t\tif (lines[start]->buf[0] == '#')\n+\t\t\tcontinue;\n+\t\treturn contains_only_spaces(lines[start]->buf);\n+\t}\n+\treturn 0;\n }\n \n static void print_lines(struct strbuf **lines, int start, int end)\n@@ -688,19 +695,19 @@ static int process_input_file(struct strbuf **lines,\n \t\t\t      struct trailer_item **in_tok_last)\n {\n \tint count = 0;\n-\tint patch_start, trailer_start, has_blank_line, i;\n+\tint patch_start, trailer_start, i;\n \n \t/* Get the line count */\n \twhile (lines[count])\n \t\tcount++;\n \n \tpatch_start = find_patch_start(lines, count);\n-\ttrailer_start = find_trailer_start(lines, patch_start, &has_blank_line);\n+\ttrailer_start = find_trailer_start(lines, patch_start);\n \n \t/* Print lines before the trailers as is */\n \tprint_lines(lines, 0, trailer_start);\n \n-\tif (!has_blank_line)\n+\tif (!has_blank_line_before(lines, trailer_start - 1))\n \t\tprintf(\"\\n\");\n \n \t/* Parse trailer lines */\n-- \n1.9.rc0.17.g651113e\n"},{"id":"239822","messageId":"20140427201238.16880.7144.chriscool@tuxfamily.org","threadId":"36511","inReplyTo":"20140427200327.16880.53255.chriscool@tuxfamily.org","subject":"[RFC/PATCH 2/2] trailer: add examples to the documentation","fromName":"Christian Couder","fromEmail":"chriscool@tuxfamily.org","sentAt":"2014-04-27T20:12:37Z","receivedAt":"2014-04-27T20:12:37Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n---\n Documentation/git-interpret-trailers.txt | 98 +++++++++++++++++++++++++++++++-\n 1 file changed, 97 insertions(+), 1 deletion(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 450ec54..42c2f71 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -134,9 +134,105 @@ If the command contains the `$ARG` string, this string will be\n replaced with the <value> part of an existing trailer with the same\n <token>, if any, before the command is launched.\n \n+EXAMPLES\n+--------\n+\n+* Configure a 'sign' trailer with a 'Signed-off-by: ' key, and then\n+  add two of these trailers to a message:\n++\n+------------\n+$ git config trailer.sign.key \"Signed-off-by: \"\n+$ cat msg.txt\n+subject\n+\n+message\n+$ cat msg.txt | git interpret-trailers --trailer 'sign: Alice <alice@example.com>' --trailer 'sign: Bob <bob@example.com>'\n+subject\n+\n+message\n+\n+Signed-off-by: Alice <alice@example.com>\n+Signed-off-by: Bob <bob@example.com>\n+------------\n+\n+* Extract the last commit as a patch, and add a 'Cc' and a\n+  'Reviewed-by' trailer to it:\n++\n+------------\n+$ git format-patch -1\n+0001-foo.patch\n+$ git interpret-trailers --trailer 'Cc: Alice <alice@example.com>' --trailer 'Reviewed-by: Bob <bob@example.com>' 0001-foo.patch >0001-bar.patch\n+------------\n+\n+* Configure a 'sign' trailer with a command to automatically add a\n+  'Signed-off-by: ' with the author information only if there is no\n+  'Signed-off-by: ' already, and show how it works:\n++\n+------------\n+$ git config trailer.sign.key \"Signed-off-by: \"\n+$ git config trailer.sign.ifmissing add\n+$ git config trailer.sign.ifexists doNothing\n+$ git config trailer.sign.command 'echo \"$(git config user.name) <$(git config user.email)>\"'\n+$ git interpret-trailers <<EOF\n+> EOF\n+\n+Signed-off-by: Bob <bob@example.com>\n+$ git interpret-trailers <<EOF\n+> Signed-off-by: Alice <alice@example.com>\n+> EOF\n+\n+Signed-off-by: Alice <alice@example.com>\n+------------\n+\n+* Configure a 'fix' trailer with a command to show the subject of a\n+  commit that is fixed, and show how it works:\n++\n+------------\n+$ git config trailer.fix.key \"Fixes #\"\n+$ git config trailer.fix.ifExists \"overwrite\"\n+$ git config trailer.fix.command \"git log -1 --oneline --format=\\\"%h (%s)\\\" --abbrev-commit --abbrev=14 \\$ARG\"\n+$ git interpret-trailers <<EOF\n+> subject\n+> \n+> message\n+> \n+> fix: HEAD~2\n+> EOF\n+subject\n+\n+message\n+\n+Fixes #fe3187489d69c4 (subject of fixed commit)\n+------------\n+\n+* Configure a commit template with some trailers with empty values,\n+  then configure a commit-msg hook that uses git interpret-trailers to\n+  remove trailers with empty values and to add a 'git-version'\n+  trailer:\n++\n+------------\n+$ cat >commit_template.txt <<EOF\n+> ***subject***\n+> \n+> ***message***\n+> \n+> Fixes: \n+> Cc: \n+> Reviewed-by: \n+> Signed-off-by: \n+> EOF\n+$ git config commit.template commit_template.txt\n+$ cat >.git/hooks/commit-msg <<EOF\n+#!/bin/sh\n+git interpret-trailers --trim-empty --trailer \"git-version: \\$(git describe)\" \"\\$1\" > \"\\$1.new\"\n+mv \"\\$1.new\" \"\\$1\"\n+EOF\n+$ chmod +x .git/hooks/commit-msg\n+------------\n+\n SEE ALSO\n --------\n-linkgit:git-commit[1]\n+linkgit:git-commit[1], linkgit:git-format-patch[1], linkgit:git-config[1]\n \n GIT\n ---\n-- \n1.9.rc0.17.g651113e\n"},{"id":"239830","messageId":"535DEE08.9090402@alum.mit.edu","threadId":"36511","inReplyTo":"20140427201238.16880.13774.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH 1/2] trailer: fix to ignore any line starting with '#'","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-04-28T05:58:32Z","receivedAt":"2014-04-28T05:58:32Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 04/27/2014 10:12 PM, Christian Couder wrote:\n> It looks like the commit-msg hook is passed a commit\n> message that can contain lines starting with a '#'.\n> Those comment lines will be removed from the commit\n> message after the hook is run.\n> \n> If we want \"git interpret-trailers\" to be able to\n> process commit messages correctly in the commit-msg\n> hook we need to ignore those lines.\n\nShouldn't this take into account the config setting core.commentchar?\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"239838","messageId":"CAP8UFD0R6g13k8GM6wQXw2qgnjeOb=z_SC6jG6EfXvo8ALdg6g@mail.gmail.com","threadId":"36511","inReplyTo":"535DEE08.9090402@alum.mit.edu","subject":"Re: [RFC/PATCH 1/2] trailer: fix to ignore any line starting with '#'","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2014-04-28T08:16:14Z","receivedAt":"2014-04-28T08:16:14Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Apr 28, 2014 at 7:58 AM, Michael Haggerty <mhagger@alum.mit.edu> wrote:\n> On 04/27/2014 10:12 PM, Christian Couder wrote:\n>> It looks like the commit-msg hook is passed a commit\n>> message that can contain lines starting with a '#'.\n>> Those comment lines will be removed from the commit\n>> message after the hook is run.\n>>\n>> If we want \"git interpret-trailers\" to be able to\n>> process commit messages correctly in the commit-msg\n>> hook we need to ignore those lines.\n>\n> Shouldn't this take into account the config setting core.commentchar?\n\nYes, I will have a look at that.\n\nThanks,\nChristian.\n"},{"id":"240054","messageId":"xmqqbnvlnlwc.fsf@gitster.dls.corp.google.com","threadId":"36511","inReplyTo":"20140427201238.16880.7144.chriscool@tuxfamily.org","subject":"Re: [RFC/PATCH 2/2] trailer: add examples to the documentation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-04-28T22:12:51Z","receivedAt":"2014-04-28T22:12:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder <chriscool@tuxfamily.org> writes:\n\n> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>\n> ---\n>  Documentation/git-interpret-trailers.txt | 98 +++++++++++++++++++++++++++++++-\n>  1 file changed, 97 insertions(+), 1 deletion(-)\n\nVery good to have these examples.  They seem to be clearly written.\n\nHaving said that, this part made me go \"Huh\" for a few reasons.\n\n> +* Configure a 'fix' trailer with a command to show the subject of a\n> +  commit that is fixed, and show how it works:\n> ++\n> +------------\n> +$ git config trailer.fix.key \"Fixes #\"\n> +$ git config trailer.fix.ifExists \"overwrite\"\n> +$ git config trailer.fix.command \"git log -1 --oneline --format=\\\"%h (%s)\\\" --abbrev-commit --abbrev=14 \\$ARG\"\n> +$ git interpret-trailers <<EOF\n> +> subject\n> +> \n> +> message\n> +> \n> +> fix: HEAD~2\n> +> EOF\n> +subject\n> +\n> +message\n> +\n> +Fixes #fe3187489d69c4 (subject of fixed commit)\n> +------------\n\n - It makes it appear that we have a convention to prefix \"#\" when\n   talking about a commit object name, but that is not what we\n   recommend to do.  Do some projects refer to commit object names\n   that way?\n\n - It has been my impression that \"#123456\", when people talk about\n   \"Fixes #123456\", refers to a bug ID in some tracking system, and\n   the scenario the example sets up \"show the subject that is fixed\"\n   would not help those who want to have such a linkage between bug\n   ID and fix.\n\nI however do not think of a good mechanised way to fill in the bug\nID with script [*1*].  I suspect (but I am thinking aloud and am\nopen to better alternatives) it would be better to change this\nexample *not* to show use of trailer.*.command but have it just take\nthe bug ID literally to make it a demonstration that the trailer\ndoes not have to end with \"key: \".  We can of course have your\nexample to turn a commit object name to human readable one-liner as\na separate one to illustrate how to use trailer.*.command, but I do\nnot think it is a good idea to call it \"Fixes\", as that may invite\nconfusions with a bug ID people would expect to see.\n\n\n[Footnote]\n\n*1* A workable project convention may be to document an existing bug\n    in a test-suite and detect a commit that fixes the bug, e.g.\n\n    - Document an expected-to-fail bug like so:\n\n      test_expect_failure 'bug #123456' '\n\t... demonstrate breakage ...\n      '\n\n      when it is noticed to keep track of it.\n\n\n    - A commit that fixes the bug will have a hunk:\n\n      -test_expect_failure 'bug #123456' '\n      +test_expect_success 'bug #123456' '\n\n      and the trailer.fix.command can examine \"git show\" output of\n      the proposed commit to detect that hunk and extract \"bug #\"\n      from there.\n\n    Of course that is not _the_ single best convention, and it would\n    be too big for an example in this documentation.\n"}]}