{"thread":{"id":"58318","subject":"[PATCH] trailer: allow spaces in tokens","startedAt":"2022-08-18T07:06:21Z","lastAt":"2022-08-30T17:21:50Z","messageCount":19,"participants":["Max Bernstein via GitGitGadget","Junio C Hamano","Jonathan Tan","Maxwell Bernstein","Christian Couder","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"461404","messageId":"pull.1309.git.git.1660806376021.gitgitgadget@gmail.com","threadId":"58318","inReplyTo":null,"subject":"[PATCH] trailer: allow spaces in tokens","fromName":"Max Bernstein via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-18T07:06:15Z","receivedAt":"2022-08-18T07:06:21Z","isPatch":true,"sender":{"key":"name:Max Bernstein","avatar":null},"body":"From: Max Bernstein \n\nThe docs (for example, https://git-scm.com/docs/git-interpret-trailers)\nsay that whitespace should be allowed inside tokens:\n\n> There can also be whitespaces inside the token and the value.\n\nThe code since e4319562bc2834096fade432fd90c985b96476db does not allow\nthat, so re-enable and add a test.\n\nSigned-off-by: Max Bernstein \n---\n trailer: allow spaces in tokens\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1309%2Ftekknolagi%2Fmaint-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1309/tekknolagi/maint-v1\nPull-Request: https://github.com/git/git/pull/1309\n\n t/t7513-interpret-trailers.sh | 40 +++++++++++++++++++++++++++++++++++\n trailer.c | 7 +-----\n 2 files changed, 41 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 97f10905d23..47bf83003ef 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1481,6 +1481,46 @@ test_expect_success 'only-trailers omits non-trailer in middle of block' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'supports spaces inside token' '\n+\tgit config --unset trailer.sign.command &&\n+\tcat >expected <<-\\EOF &&\n+\t\tSigned-off-by: nobody \n+\t\tsome other trailer: a value\n+\t\tSigned-off-by: somebody \n+\tEOF\n+\techo \"wrote to expected\" 1>&2 &&\n+\tgit interpret-trailers --only-trailers >actual <<-\\EOF &&\n+\t\tsubject\n+\n+\t\tit is important that the trailers below are signed-off-by\n+\t\tso that they meet the \"25% trailers Git knows about\" heuristic\n+\n+\t\tSigned-off-by: nobody \n+\t\tsome other trailer: a value\n+\t\tSigned-off-by: somebody \n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'does not support space at beginning of token' '\n+\tcat >expected <<-\\EOF &&\n+\t\tSigned-off-by: nobody not a trailer: thing\n+\t\tSigned-off-by: somebody \n+\tEOF\n+\techo \"wrote to expected\" 1>&2 &&\n+\tgit interpret-trailers --only-trailers --unfold >actual <<-\\EOF &&\n+\t\tsubject\n+\n+\t\tit is important that the trailers below are signed-off-by\n+\t\tso that they meet the \"25% trailers Git knows about\" heuristic\n+\n+\t\tSigned-off-by: nobody \n+\t\t not a trailer: thing\n+\t\tSigned-off-by: somebody \n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'only input' '\n \tgit config trailer.sign.command \"echo config-value\" &&\n \tcat >expected <<-\\EOF &&\ndiff --git a/trailer.c b/trailer.c\nindex d419c20735e..d02a9154512 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -618,17 +618,12 @@ static int token_matches_item(const char *tok, struct arg_item *item, size_t tok\n */\n static ssize_t find_separator(const char *line, const char *separators)\n {\n-\tint whitespace_found = 0;\n \tconst char *c;\n \tfor (c = line; *c; c++) {\n \t\tif (strchr(separators, *c))\n \t\t\treturn c - line;\n-\t\tif (!whitespace_found && (isalnum(*c) || *c == '-'))\n+\t\tif (isalnum(*c) || *c == '-' || (c != line && (*c == ' ' || *c == '\\t')))\n \t\t\tcontinue;\n-\t\tif (c != line && (*c == ' ' || *c == '\\t')) {\n-\t\t\twhitespace_found = 1;\n-\t\t\tcontinue;\n-\t\t}\n \t\tbreak;\n \t}\n \treturn -1;\n\nbase-commit: ad60dddad72dfb8367bd695028b5b8dc6c33661b\n-- \ngitgitgadget\n"},{"id":"461410","messageId":"pull.1309.v2.git.git.1660809243298.gitgitgadget@gmail.com","threadId":"58318","inReplyTo":"pull.1309.git.git.1660806376021.gitgitgadget@gmail.com","subject":"[PATCH v2] trailer: allow spaces in tokens","fromName":"Max Bernstein via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2022-08-18T07:54:03Z","receivedAt":"2022-08-18T07:54:12Z","isPatch":true,"sender":{"key":"name:Max Bernstein","avatar":null},"body":"From: Max Bernstein \n\nThe docs (for example, https://git-scm.com/docs/git-interpret-trailers)\nsay that whitespace should be allowed inside tokens:\n\n> There can also be whitespaces inside the token and the value.\n\nThe code since e4319562bc2834096fade432fd90c985b96476db does not allow\nthat, so re-enable and add a test.\n\nSigned-off-by: Max Bernstein \n---\n trailer: allow spaces in tokens\n \n Changed since v1:\n \n * \"Fixed\" a format-patch test. I'm not sure if the new output is what\n everyone might expect for this particular case.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1309%2Ftekknolagi%2Fmaint-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1309/tekknolagi/maint-v2\nPull-Request: https://github.com/git/git/pull/1309\n\nRange-diff vs v1:\n\n 1: 404a6d1b193 ! 1: 4d490851ac2 trailer: allow spaces in tokens\n @@ Commit message\n \n Signed-off-by: Max Bernstein \n \n + ## t/t4014-format-patch.sh ##\n +@@ t/t4014-format-patch.sh: test_expect_success 'signoff: not really a signoff' '\n + \t4:Subject: [PATCH] subject\n + \t8:\n + \t9:I want to mention about Signed-off-by: here.\n +-\t10:\n +-\t11:Signed-off-by: C O Mitter \n ++\t10:Signed-off-by: C O Mitter \n + \tEOF\n + \ttest_cmp expect actual\n + '\n +\n ## t/t7513-interpret-trailers.sh ##\n @@ t/t7513-interpret-trailers.sh: test_expect_success 'only-trailers omits non-trailer in middle of block' '\n \ttest_cmp expected actual\n\n\n t/t4014-format-patch.sh | 3 +--\n t/t7513-interpret-trailers.sh | 40 +++++++++++++++++++++++++++++++++++\n trailer.c | 7 +-----\n 3 files changed, 42 insertions(+), 8 deletions(-)\n\ndiff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh\nindex fbec8ad2ef7..db95cb6ee66 100755\n--- a/t/t4014-format-patch.sh\n+++ b/t/t4014-format-patch.sh\n@@ -1544,8 +1544,7 @@ test_expect_success 'signoff: not really a signoff' '\n \t4:Subject: [PATCH] subject\n \t8:\n \t9:I want to mention about Signed-off-by: here.\n-\t10:\n-\t11:Signed-off-by: C O Mitter \n+\t10:Signed-off-by: C O Mitter \n \tEOF\n \ttest_cmp expect actual\n '\ndiff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\nindex 97f10905d23..47bf83003ef 100755\n--- a/t/t7513-interpret-trailers.sh\n+++ b/t/t7513-interpret-trailers.sh\n@@ -1481,6 +1481,46 @@ test_expect_success 'only-trailers omits non-trailer in middle of block' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'supports spaces inside token' '\n+\tgit config --unset trailer.sign.command &&\n+\tcat >expected <<-\\EOF &&\n+\t\tSigned-off-by: nobody \n+\t\tsome other trailer: a value\n+\t\tSigned-off-by: somebody \n+\tEOF\n+\techo \"wrote to expected\" 1>&2 &&\n+\tgit interpret-trailers --only-trailers >actual <<-\\EOF &&\n+\t\tsubject\n+\n+\t\tit is important that the trailers below are signed-off-by\n+\t\tso that they meet the \"25% trailers Git knows about\" heuristic\n+\n+\t\tSigned-off-by: nobody \n+\t\tsome other trailer: a value\n+\t\tSigned-off-by: somebody \n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'does not support space at beginning of token' '\n+\tcat >expected <<-\\EOF &&\n+\t\tSigned-off-by: nobody not a trailer: thing\n+\t\tSigned-off-by: somebody \n+\tEOF\n+\techo \"wrote to expected\" 1>&2 &&\n+\tgit interpret-trailers --only-trailers --unfold >actual <<-\\EOF &&\n+\t\tsubject\n+\n+\t\tit is important that the trailers below are signed-off-by\n+\t\tso that they meet the \"25% trailers Git knows about\" heuristic\n+\n+\t\tSigned-off-by: nobody \n+\t\t not a trailer: thing\n+\t\tSigned-off-by: somebody \n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'only input' '\n \tgit config trailer.sign.command \"echo config-value\" &&\n \tcat >expected <<-\\EOF &&\ndiff --git a/trailer.c b/trailer.c\nindex d419c20735e..d02a9154512 100644\n--- a/trailer.c\n+++ b/trailer.c\n@@ -618,17 +618,12 @@ static int token_matches_item(const char *tok, struct arg_item *item, size_t tok\n */\n static ssize_t find_separator(const char *line, const char *separators)\n {\n-\tint whitespace_found = 0;\n \tconst char *c;\n \tfor (c = line; *c; c++) {\n \t\tif (strchr(separators, *c))\n \t\t\treturn c - line;\n-\t\tif (!whitespace_found && (isalnum(*c) || *c == '-'))\n+\t\tif (isalnum(*c) || *c == '-' || (c != line && (*c == ' ' || *c == '\\t')))\n \t\t\tcontinue;\n-\t\tif (c != line && (*c == ' ' || *c == '\\t')) {\n-\t\t\twhitespace_found = 1;\n-\t\t\tcontinue;\n-\t\t}\n \t\tbreak;\n \t}\n \treturn -1;\n\nbase-commit: ad60dddad72dfb8367bd695028b5b8dc6c33661b\n-- \ngitgitgadget\n"},{"id":"461439","messageId":"xmqqo7whqyzs.fsf@gitster.g","threadId":"58318","inReplyTo":"pull.1309.git.git.1660806376021.gitgitgadget@gmail.com","subject":"Re: [PATCH] trailer: allow spaces in tokens","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-18T16:48:23Z","receivedAt":"2022-08-18T16:49:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"[jc: summoning an area expert and the developer whose commit is\nblamed for the breakage by adding their addresses on Cc: list]\n\n\"Max Bernstein via GitGitGadget\" writes:\n\n> From: Max Bernstein \n>\n> The docs (for example, https://git-scm.com/docs/git-interpret-trailers)\n\nRefer to \"git help interpret-trailers\" instead.\n\n> say that whitespace should be allowed inside tokens:\n>\n>> There can also be whitespaces inside the token and the value.\n>\n> The code since e4319562bc2834096fade432fd90c985b96476db does not allow\n\nRefer to an existing commit like so\n\n\te4319562 (trailer: be stricter in parsing separators, 2016-11-02)\n\n> that, so re-enable and add a test.\n\nJonathan, do you remember if the \"be stricter\" was in a response to\nspecific real world issues, or is this \"we no longer allow whitespace\"\nan unintended side effect of the change?\n\nIf the former, an equally valid \"fix\" to what Max reports here would\nbe to add such a real world issue or two as new tests to demonostrate\nwhy allowing whitespaces was a mistake that hurts real-world workflow,\nand fix the documentation.\n\nI actually am suspecting it is the latter, only because we would have\nadded a testcase to show why whitespaces in the trailer label is a\nbad idea in e4319562 if this was triggered by a real-world issue.\n\nI would say that it would be a disaster, if we took any random\nline with colon : in it in the middle of the commit message and\nmistook it as a trailer (like the line above), but since we do not\nallow paragraph breaks in the trailer block, as long as the message\nhas a valid trailer block, it might not be a huge issue. I dunno.\n\n> diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh\n> index 97f10905d23..47bf83003ef 100755\n> --- a/t/t7513-interpret-trailers.sh\n> +++ b/t/t7513-interpret-trailers.sh\n> @@ -1481,6 +1481,46 @@ test_expect_success 'only-trailers omits non-trailer in middle of block' '\n> \ttest_cmp expected actual\n> '\n> \n> +test_expect_success 'supports spaces inside token' '\n> +\tgit config --unset trailer.sign.command &&\n> +\tcat >expected <<-\\EOF &&\n> +\t\tSigned-off-by: nobody \n> +\t\tsome other trailer: a value\n> +\t\tSigned-off-by: somebody \n> +\tEOF\n> +\techo \"wrote to expected\" 1>&2 &&\n> +\tgit interpret-trailers --only-trailers >actual <<-\\EOF &&\n> +\t\tsubject\n> +\n> +\t\tit is important that the trailers below are signed-off-by\n\nIt also is important that if you add colon at the end of the above\nline, it is *not* mistaken as a trailer with whitespace in token,\nwith an empty value.\n\n> +\t\tso that they meet the \"25% trailers Git knows about\" heuristic\n> +\n> +\t\tSigned-off-by: nobody \n> +\t\tsome other trailer: a value\n> +\t\tSigned-off-by: somebody \n> +\tEOF\n> +\ttest_cmp expected actual\n> +'\n> +\n> +test_expect_success 'does not support space at beginning of token' '\n> +\tcat >expected <<-\\EOF &&\n> +\t\tSigned-off-by: nobody not a trailer: thing\n> +\t\tSigned-off-by: somebody \n> +\tEOF\n> +\techo \"wrote to expected\" 1>&2 &&\n> +\tgit interpret-trailers --only-trailers --unfold >actual <<-\\EOF &&\n> +\t\tsubject\n> +\n> +\t\tit is important that the trailers below are signed-off-by\n> +\t\tso that they meet the \"25% trailers Git knows about\" heuristic\n> +\n> +\t\tSigned-off-by: nobody \n> +\t\t not a trailer: thing\n> +\t\tSigned-off-by: somebody \n> +\tEOF\n> +\ttest_cmp expected actual\n> +'\n> +\n> test_expect_success 'only input' '\n> \tgit config trailer.sign.command \"echo config-value\" &&\n> \tcat >expected <<-\\EOF &&\n> diff --git a/trailer.c b/trailer.c\n> index d419c20735e..d02a9154512 100644\n> --- a/trailer.c\n> +++ b/trailer.c\n> @@ -618,17 +618,12 @@ static int token_matches_item(const char *tok, struct arg_item *item, size_t tok\n> */\n> static ssize_t find_separator(const char *line, const char *separators)\n> {\n> -\tint whitespace_found = 0;\n> \tconst char *c;\n> \tfor (c = line; *c; c++) {\n> \t\tif (strchr(separators, *c))\n> \t\t\treturn c - line;\n> -\t\tif (!whitespace_found && (isalnum(*c) || *c == '-'))\n> +\t\tif (isalnum(*c) || *c == '-' || (c != line && (*c == ' ' || *c == '\\t')))\n> \t\t\tcontinue;\n> -\t\tif (c != line && (*c == ' ' || *c == '\\t')) {\n> -\t\t\twhitespace_found = 1;\n> -\t\t\tcontinue;\n> -\t\t}\n> \t\tbreak;\n> \t}\n> \treturn -1;\n>\n> base-commit: ad60dddad72dfb8367bd695028b5b8dc6c33661b\n"},{"id":"461442","messageId":"xmqqk075qyq0.fsf@gitster.g","threadId":"58318","inReplyTo":"pull.1309.v2.git.git.1660809243298.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] trailer: allow spaces in tokens","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-18T16:54:15Z","receivedAt":"2022-08-18T16:54:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Max Bernstein via GitGitGadget\" writes:\n\n> Range-diff vs v1:\n>\n> 1: 404a6d1b193 ! 1: 4d490851ac2 trailer: allow spaces in tokens\n> @@ Commit message\n> \n> Signed-off-by: Max Bernstein \n> \n> + ## t/t4014-format-patch.sh ##\n> +@@ t/t4014-format-patch.sh: test_expect_success 'signoff: not really a signoff' '\n> + \t4:Subject: [PATCH] subject\n> + \t8:\n> + \t9:I want to mention about Signed-off-by: here.\n> +-\t10:\n> +-\t11:Signed-off-by: C O Mitter \n> ++\t10:Signed-off-by: C O Mitter \n> + \tEOF\n> + \ttest_cmp expect actual\n> + '\n\nSo, the updated code mistook the body of the message that is not a\nsign-off, because there is a colon on the line, the line does not\nbegin with the colon, and everything before the colon is an alnum or\na whitespace, so squashed the paragraph break before the real\ntrailer block and the last line of the body and made it a body-less\ncommit log message?\n\nThis might be a good demonstration of why it is a mistaken design to\nallow whitespaces, which may steer us toward fixing the documentation?\n\nI dunno. What do others think?\n"},{"id":"461451","messageId":"20220818175216.3530525-1-jonathantanmy@google.com","threadId":"58318","inReplyTo":"xmqqo7whqyzs.fsf@gitster.g","subject":"Re: [PATCH] trailer: allow spaces in tokens","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2022-08-18T17:52:16Z","receivedAt":"2022-08-18T17:52:23Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Junio C Hamano writes:\n> Jonathan, do you remember if the \"be stricter\" was in a response to\n> specific real world issues, or is this \"we no longer allow whitespace\"\n> an unintended side effect of the change?\n\nRereading the relevant mailing list thread, I think it's a bit of both.\nThe sequence of events was as follows:\n - I made a patch set that changed sequencer.c (which, among other\n things, controls the behavior of \"commit -s\") to use\n interpret-trailers's functionality. This caused a change in the\n behavior of a \"commit -s\" unit test (the \"I want to mention about\n Signed-off-by: here.\" one), because \"commit -s\" previously did not\n support spaces before the separator. [1]\n - You mentioned that perhaps we should tighten it back. [2]\n - I agreed and tightened it, but didn't change the documentation. [3]\n\n[1] https://lore.kernel.org/git/a416ab9b-ff1f-9a71-3e58-60fd4f8a6b8e@google.com/\n[2] https://lore.kernel.org/git/xmqqtwbroykt.fsf@gitster.mtv.corp.google.com/\n[3] https://lore.kernel.org/git/cover.1478028700.git.jonathantanmy@google.com/\n\nSo the \"real world issue\" was a test in our test suite, and perhaps the\nside effect was unintended in that we wanted to preserve the behavior of\n\"commit -s\" but didn't think about other possible uses of\ninterpret-trailers.\n\n> If the former, an equally valid \"fix\" to what Max reports here would\n> be to add such a real world issue or two as new tests to demonostrate\n> why allowing whitespaces was a mistake that hurts real-world workflow,\n> and fix the documentation.\n>\n> I actually am suspecting it is the latter, only because we would have\n> added a testcase to show why whitespaces in the trailer label is a\n> bad idea in e4319562 if this was triggered by a real-world issue.\n\nThe \"I want to mention about Signed-off-by: here.\" test might be a\nsufficient real-world issue.\n\n> I would say that it would be a disaster, if we took any random\n> line with colon : in it in the middle of the commit message and\n> mistook it as a trailer (like the line above), but since we do not\n> allow paragraph breaks in the trailer block, as long as the message\n> has a valid trailer block, it might not be a huge issue. I dunno.\n\nYes, it would be fine if the paragraph was two or more lines long, but\nnot if it's a single line.\n"},{"id":"461452","messageId":"20220818175600.3531267-1-jonathantanmy@google.com","threadId":"58318","inReplyTo":"xmqqk075qyq0.fsf@gitster.g","subject":"Re: [PATCH v2] trailer: allow spaces in tokens","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2022-08-18T17:56:00Z","receivedAt":"2022-08-18T17:56:08Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"Junio C Hamano writes:\n> \"Max Bernstein via GitGitGadget\" writes:\n> \n> > Range-diff vs v1:\n> >\n> > 1: 404a6d1b193 ! 1: 4d490851ac2 trailer: allow spaces in tokens\n> > @@ Commit message\n> > \n> > Signed-off-by: Max Bernstein \n> > \n> > + ## t/t4014-format-patch.sh ##\n> > +@@ t/t4014-format-patch.sh: test_expect_success 'signoff: not really a signoff' '\n> > + \t4:Subject: [PATCH] subject\n> > + \t8:\n> > + \t9:I want to mention about Signed-off-by: here.\n> > +-\t10:\n> > +-\t11:Signed-off-by: C O Mitter \n> > ++\t10:Signed-off-by: C O Mitter \n> > + \tEOF\n> > + \ttest_cmp expect actual\n> > + '\n> \n> So, the updated code mistook the body of the message that is not a\n> sign-off, because there is a colon on the line, the line does not\n> begin with the colon, and everything before the colon is an alnum or\n> a whitespace, so squashed the paragraph break before the real\n> trailer block and the last line of the body and made it a body-less\n> commit log message?\n> \n> This might be a good demonstration of why it is a mistaken design to\n> allow whitespaces, which may steer us toward fixing the documentation?\n> \n> I dunno. What do others think?\n\nYeah, this was the same issue we discussed a few years ago [1]. \"git\ncommit -s\" has never supported spaces before the colon (at least, it did\nnot support it before my patch set in [1] and does not support it now)\nso it seems that having no spaces is already the established convention,\nand I think that interpret-trailers should follow it. I would agree that\nwe should fix the documentation to match the current behavior.\n\n[1] https://lore.kernel.org/git/a416ab9b-ff1f-9a71-3e58-60fd4f8a6b8e@google.com/\n"},{"id":"461453","messageId":"xmqqedxdph79.fsf@gitster.g","threadId":"58318","inReplyTo":"20220818175216.3530525-1-jonathantanmy@google.com","subject":"Re: [PATCH] trailer: allow spaces in tokens","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-18T17:58:02Z","receivedAt":"2022-08-18T17:58:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Tan writes:\n\n> The \"I want to mention about Signed-off-by: here.\" test might be a\n> sufficient real-world issue.\n>\n>> I would say that it would be a disaster, if we took any random\n>> line with colon : in it in the middle of the commit message and\n>> mistook it as a trailer (like the line above), but since we do not\n>> allow paragraph breaks in the trailer block, as long as the message\n>> has a valid trailer block, it might not be a huge issue. I dunno.\n>\n> Yes, it would be fine if the paragraph was two or more lines long, but\n> not if it's a single line.\n\nOK, so it seems that we want to fix the documentation, and we have\nsufficient test coverage (I didn't notice that Max needed to adjust\nthe tests to accomodate the new \"loose\" definition when I wrote the\nmessage you were responding to).\n\nTHanks.\n"},{"id":"461458","messageId":"CM9DZ4Z18PY9.1T2O8U54X66FD@cedar","threadId":"58318","inReplyTo":"xmqqk075qyq0.fsf@gitster.g","subject":"Re: [PATCH v2] trailer: allow spaces in tokens","fromName":"Maxwell Bernstein","fromEmail":"tekk.nolagi@gmail.com","sentAt":"2022-08-18T19:03:19Z","receivedAt":"2022-08-18T19:03:25Z","isPatch":true,"sender":{"key":"tekk.nolagi@gmail.com","avatar":null},"body":"On Thu Aug 18, 2022 at 9:54 AM PDT, Junio C Hamano wrote:\n> So, the updated code mistook the body of the message that is not a\n> sign-off, because there is a colon on the line, the line does not\n> begin with the colon, and everything before the colon is an alnum or\n> a whitespace, so squashed the paragraph break before the real\n> trailer block and the last line of the body and made it a body-less\n> commit log message?\n\nI agree that it is not ideal, and I'm not sure how to fix it. This commit\nprobably shouldn't go in as-is; I modified the test in order to demonstrate\nthis problem.\n\n> This might be a good demonstration of why it is a mistaken design to\n> allow whitespaces, which may steer us toward fixing the documentation?\n>\n> I dunno. What do others think?\n\nI think allowing whitespace is good at least for the Phabricator project, which\ndespite being dead, has a number of existing users and existing commits. It\nunfortunately has a \"Differential revision\" trailer with whitespace.\n"},{"id":"461466","messageId":"CAP8UFD1304rQh+9jMUkTmTvmNBxm6BNDt3=d07+C8iXSbJb2RA@mail.gmail.com","threadId":"58318","inReplyTo":"CM9DZ4Z18PY9.1T2O8U54X66FD@cedar","subject":"Re: [PATCH v2] trailer: allow spaces in tokens","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-08-18T20:46:18Z","receivedAt":"2022-08-18T20:46:33Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Thu, Aug 18, 2022 at 9:03 PM Maxwell Bernstein wrote:\n>\n> On Thu Aug 18, 2022 at 9:54 AM PDT, Junio C Hamano wrote:\n> > So, the updated code mistook the body of the message that is not a\n> > sign-off, because there is a colon on the line, the line does not\n> > begin with the colon, and everything before the colon is an alnum or\n> > a whitespace, so squashed the paragraph break before the real\n> > trailer block and the last line of the body and made it a body-less\n> > commit log message?\n>\n> I agree that it is not ideal, and I'm not sure how to fix it. This commit\n> probably shouldn't go in as-is; I modified the test in order to demonstrate\n> this problem.\n>\n> > This might be a good demonstration of why it is a mistaken design to\n> > allow whitespaces, which may steer us toward fixing the documentation?\n> >\n> > I dunno. What do others think?\n>\n> I think allowing whitespace is good at least for the Phabricator project, which\n> despite being dead, has a number of existing users and existing commits. It\n> unfortunately has a \"Differential revision\" trailer with whitespace.\n\nI think allowing one white space before the separator is a good idea\nfor the following reasons.\n\nFirst, if people use something like #, !, ~ or % as a separator, for\nexample in the case of a trailer like \"Bug #43\", it is very natural to\nhave a white space before the # separator.\n\nNote that GitLab for example uses the ~N format, where N is a number,\nfor issues, !N for merge requests and %N for milestones. I think\nBugzilla and many other bug trackers use a #N format for bug numbers.\n\nAlso in some languages, like French, the typographical rules when\nwriting regular text is to have a space (technically it's supposed to\nbe a \"non breaking space\", see:\nhttps://en.wikipedia.org/wiki/Non-breaking_space, but in practice\npeople use a regular space most of the time) before a colon. So it is\nvery natural for a number of people in the world to automatically add\na white space before a colon.\n\nhttps://en.wikipedia.org/wiki/Colon_(punctuation)#Spacing says: \"In\nprint, a thin space was traditionally placed before a colon and a\nthick space after it. In modern English-language printing, no space is\nplaced before a colon and a single space is placed after it. In\nFrench-language typing and printing, the traditional rules are\npreserved. \"\n\nI think it would be very annoying for users to find out that a number\nof otherwise proper trailers would not be taken into account only\nbecause they have a white space before the colon. At least there\nshould be an option to allow that.\n"},{"id":"461469","messageId":"xmqqa681nsrv.fsf@gitster.g","threadId":"58318","inReplyTo":"CAP8UFD1304rQh+9jMUkTmTvmNBxm6BNDt3=d07+C8iXSbJb2RA@mail.gmail.com","subject":"Re: [PATCH v2] trailer: allow spaces in tokens","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-18T21:31:00Z","receivedAt":"2022-08-18T21:31:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> I think allowing one white space before the separator is a good idea\n> for the following reasons.\n> ...\n> I think it would be very annoying for users to find out that a number\n> of otherwise proper trailers would not be taken into account only\n> because they have a white space before the colon. At least there\n> should be an option to allow that.\n\nIn short, you do not support Max's patch that allows arbitrary\nnumber of white spaces anywhere but at the beginning of line,\nbut see a room for unrelated improvement from the current code,\nnamely, to allow exactly one optional space, immediately before\nthe separator and nowhere else.\n\nIt may be a reasonable thing to do that may not break too many\nthings. I do not know if that is loose enough to satisify Max's\noriginal wish, though.\n\nThanks.\n\n"},{"id":"461505","messageId":"xmqqwnb4n98f.fsf@gitster.g","threadId":"58318","inReplyTo":"xmqqa681nsrv.fsf@gitster.g","subject":"Re: [PATCH v2] trailer: allow spaces in tokens","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-19T04:33:04Z","receivedAt":"2022-08-19T04:33:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> In short, you do not support Max's patch that allows arbitrary\n> number of white spaces anywhere but at the beginning of line,\n> but see a room for unrelated improvement from the current code,\n> namely, to allow exactly one optional space, immediately before\n> the separator and nowhere else.\n\nAh, no, sorry, I misread the situation. It's not a room for\nimprovement. It is very close to what the current code already\ndoes, i.e. to allow optional spaces immediately before the\nseparator. The difference is that the current code allows arbitrary\nnumber of optional spaces, not zero or exactly one.\n\nSo the only thing we need to do is to update the documentation that\nMax misread as allowing spaces at arbitrary place to reflect what\nthe code has been doing, i.e. an optional run of spaces is allowed\nbetween the \"token\" and the \"separator\"?\n\nAnybody wants to do the honors to produce such a patch, then?\n\nThanks.\n\n\n \n"},{"id":"461540","messageId":"CAP8UFD2kMXHxvg1tKVNLhY0Gweq2YrD7+tHmZXKwguYzRQ1Qpg@mail.gmail.com","threadId":"58318","inReplyTo":"xmqqwnb4n98f.fsf@gitster.g","subject":"Re: [PATCH v2] trailer: allow spaces in tokens","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-08-19T10:29:54Z","receivedAt":"2022-08-19T10:30:12Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Fri, Aug 19, 2022 at 6:33 AM Junio C Hamano wrote:\n>\n> Junio C Hamano writes:\n>\n> > In short, you do not support Max's patch that allows arbitrary\n> > number of white spaces anywhere but at the beginning of line,\n\nI haven't closely followed the patches that might have tweaked the\noriginal default behavior regarding whitespaces in tokens, but I think\nin general it's not a good idea to change the default behavior for no\nvery good reason, because it could cause regressions in the way others\nalready use `git interpret-trailers`. It could have been Ok to accept\nthat during a few years after the original implementation was merged\nin 2014, but these days people and tools (like GitLab for example)\nrely on trailers and `git interpret-trailers` more and more.\n\nSo in general I think it's better to have a more cautious approach\nnow, like we usually have for other commands, and only allow new\noptions to tweak the default behavior. If one such option appears to\nbe very widely used, then we might decide later that it should become\nthe default behavior.\n\nI understand that having commands with many options might not be\nconsidered by some of us as good design, but if we don't like that,\nthen we could perhaps just have only one option, maybe called\nsomething like \"trailer.tokenFormat\", or just \"trailer.format\", that\nwould contain a regex that every token, or trailer, should match.\n\nIt's a complex subject because there are different and competing\nviewpoints. On one hand it can be annoying to have some trailers\nignored just because they don't match the default format in a very\nsmall way. (And rewriting trailers could mean rewriting published\nhistory which is not an easy thing to do.) On the other hand if we\ndevelop something like \"trailer.tokenFormat\" that allows everything,\nthen we might consider that we should instead encourage people to do\nthe right thing and to use as much as possible regular trailers with a\nstrict format. So allowing new options to only tweak things in a small\nway might be the right thing to do after all.\n\n> > but see a room for unrelated improvement from the current code,\n> > namely, to allow exactly one optional space, immediately before\n> > the separator and nowhere else.\n>\n> Ah, no, sorry, I misread the situation. It's not a room for\n> improvement. It is very close to what the current code already\n> does, i.e. to allow optional spaces immediately before the\n> separator. The difference is that the current code allows arbitrary\n> number of optional spaces, not zero or exactly one.\n\nYeah, I think it makes sense to not change this default behavior, but\nmaybe, if people don't like it for some reason, allow options to tweak\nit.\n\n> So the only thing we need to do is to update the documentation that\n> Max misread as allowing spaces at arbitrary place to reflect what\n> the code has been doing, i.e. an optional run of spaces is allowed\n> between the \"token\" and the \"separator\"?\n\nYeah, I also think that the documentation should be very clear (and\naccurate of course) about where whitespaces and how many of them are\nallowed by default, and what can possibly be tweaked by options.\n\n> Anybody wants to do the honors to produce such a patch, then?\n\nOk, I will give it a try soon.\n"},{"id":"461755","messageId":"p8srs909-19p5-4o24-r245-roq46pp4s6p5@tzk.qr","threadId":"58318","inReplyTo":"CAP8UFD2kMXHxvg1tKVNLhY0Gweq2YrD7+tHmZXKwguYzRQ1Qpg@mail.gmail.com","subject":"Re: [PATCH v2] trailer: allow spaces in tokens","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-08-22T13:58:21Z","receivedAt":"2022-08-22T13:58:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Christian,\n\nOn Fri, 19 Aug 2022, Christian Couder wrote:\n\n> On Fri, Aug 19, 2022 at 6:33 AM Junio C Hamano wrote:\n> >\n> > Junio C Hamano writes:\n> >\n> > > but see a room for unrelated improvement from the current code,\n> > > namely, to allow exactly one optional space, immediately before the\n> > > separator and nowhere else.\n> >\n> > Ah, no, sorry, I misread the situation. It's not a room for\n> > improvement. It is very close to what the current code already does,\n> > i.e. to allow optional spaces immediately before the separator. The\n> > difference is that the current code allows arbitrary number of\n> > optional spaces, not zero or exactly one.\n>\n> Yeah, I think it makes sense to not change this default behavior, but\n> maybe, if people don't like it for some reason, allow options to tweak\n> it.\n\nThat sounds like a very good compromise to me.\n\nCiao,\nDscho\n"},{"id":"461856","messageId":"20220823140630.159718-1-christian.couder@gmail.com","threadId":"58318","inReplyTo":"CAP8UFD2kMXHxvg1tKVNLhY0Gweq2YrD7+tHmZXKwguYzRQ1Qpg@mail.gmail.com","subject":"[PATCH] Documentation: clarify whitespace rules for trailers","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-08-23T14:06:30Z","receivedAt":"2022-08-23T17:07:14Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Commit e4319562bc (trailer: be stricter in parsing separators, 2016-11-02)\nrestricted whitespaces allowed by `git interpret-trailers` in the \"token\"\npart of the trailers it reads. This commit didn't update the related\ndocumentation in Documentation/git-interpret-trailers.txt though.\n\nAlso commit 60ef86a162 (trailer: support values folded to multiple lines,\n2016-10-21) updated the documentation, but didn't make it clear how many\nwhitespace characters are allowed at the beginning of new lines in folded\nvalues.\n\nLet's fix both of these issues by rewriting the paragraph describing\nwhat whitespaces are allowed by `git interpret-trailers` in the trailers\nit reads.\n---\n Documentation/git-interpret-trailers.txt | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 956a01d184..0e125d795b 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -60,10 +60,12 @@ non-whitespace lines before a line that starts with '---' (followed by a\n space or the end of the line). Such three minus signs start the patch\n part of the message. See also `--no-divider` below.\n \n-When reading trailers, there can be whitespaces after the\n-token, the separator and the value. There can also be whitespaces\n-inside the token and the value. The value may be split over multiple lines with\n-each subsequent line starting with whitespace, like the \"folding\" in RFC 822.\n+When reading trailers, there can be no whitespace inside the token,\n+and only one regular space or tab character between the token and the\n+separator. There can be whitespaces before, inside or after the\n+value. The value may be split over multiple lines with each subsequent\n+line starting with at least one whitespace, like the \"folding\" in RFC\n+822.\n \n Note that 'trailers' do not follow and are not intended to follow many\n rules for RFC 822 headers. For example they do not follow\n-- \n2.37.0\n\n"},{"id":"461904","messageId":"xmqq4jy18q7h.fsf@gitster.g","threadId":"58318","inReplyTo":"20220823140630.159718-1-christian.couder@gmail.com","subject":"Re: [PATCH] Documentation: clarify whitespace rules for trailers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-24T18:13:38Z","receivedAt":"2022-08-24T18:13:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> Commit e4319562bc (trailer: be stricter in parsing separators, 2016-11-02)\n> restricted whitespaces allowed by `git interpret-trailers` in the \"token\"\n> part of the trailers it reads. This commit didn't update the related\n> documentation in Documentation/git-interpret-trailers.txt though.\n\nOK.\n\n> Also commit 60ef86a162 (trailer: support values folded to multiple lines,\n> 2016-10-21) updated the documentation, but didn't make it clear how many\n> whitespace characters are allowed at the beginning of new lines in folded\n> values.\n>\n> Let's fix both of these issues by rewriting the paragraph describing\n> what whitespaces are allowed by `git interpret-trailers` in the trailers\n> it reads.\n> ---\n> Documentation/git-interpret-trailers.txt | 10 ++++++----\n> 1 file changed, 6 insertions(+), 4 deletions(-)\n\nMissing sign-off.\n\n\n> diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\n> index 956a01d184..0e125d795b 100644\n> --- a/Documentation/git-interpret-trailers.txt\n> +++ b/Documentation/git-interpret-trailers.txt\n> @@ -60,10 +60,12 @@ non-whitespace lines before a line that starts with '---' (followed by a\n> space or the end of the line). Such three minus signs start the patch\n> part of the message. See also `--no-divider` below.\n> \n> -When reading trailers, there can be whitespaces after the\n> -token, the separator and the value. There can also be whitespaces\n> -inside the token and the value. The value may be split over multiple lines with\n> -each subsequent line starting with whitespace, like the \"folding\" in RFC 822.\n> +When reading trailers, there can be no whitespace inside the token,\n> +and only one regular space or tab character between the token and the\n> +separator.\n\nThat may have been the intent, but does it match the behaviour?\n\n static ssize_t find_separator(const char *line, const char *separators)\n {\n int whitespace_found = 0;\n const char *c;\n for (c = line; *c; c++) {\n if (strchr(separators, *c))\n return c - line;\n if (!whitespace_found && (isalnum(*c) || *c == '-'))\n continue;\n if (c != line && (*c == ' ' || *c == '\\t')) {\n whitespace_found = 1;\n continue;\n }\n break;\n }\n return -1;\n }\n\nWhen parsing \"X :\", first we encounter 'X', we haven't seen\nwhitespace, 'X' passes isalnum(), and we continue. Then we\nencounter ' ', we haven't seen whitespace but it is neither isalnum\nor dash, so we go on without hitting the first continue. We are not\nat the beginning of the line, we are seeing a space, so we remember\nthe fact that we saw whitespace and continue. Next we see another ' ',\nwe do not hit the first continue, we are not at the beginning of the\nline, and we are looking at ' ', so we again continue. Finally, we see\n':' that is a separator and we return happily.\n\nThe code seems to be allowing zero or more space/tab before the\nseparator.\n\nStepping back and reading the original again, I think the original\nwas almost correct. There can be whitespaces after the token.\nThere can be whitespaces after the separator. There can be\nwhitespaces after the value. The only thing it got wrong was that\nit pretended to allow whitespaces inside the token, while in reality\nwe allow whitespaces inside the value but not inside the token.\n\nSo, a minimum fix would be to s/token and the value/value/; I do not\nmind a more extensive rewriting if it improves clarity and correctness,\nbut \"only one between the token and the separator\" is not quite correct.\nBesides, that phrasing gives a false impression that the whitespace is\nmandatory, but you wanted to express \"zero or one\" optionality, no?\n\n> There can be whitespaces before, inside or after the\n> +value. The value may be split over multiple lines with each subsequent\n> +line starting with at least one whitespace, like the \"folding\" in RFC\n> +822.\n> \n> Note that 'trailers' do not follow and are not intended to follow many\n> rules for RFC 822 headers. For example they do not follow\n"},{"id":"461948","messageId":"CAP8UFD0EfhDo9ZMNSYp8YVHXJs6mYtSBs=hwoFZWWF3xZ0wjpg@mail.gmail.com","threadId":"58318","inReplyTo":"xmqq4jy18q7h.fsf@gitster.g","subject":"Re: [PATCH] Documentation: clarify whitespace rules for trailers","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-08-25T11:59:38Z","receivedAt":"2022-08-25T12:00:04Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Wed, Aug 24, 2022 at 8:13 PM Junio C Hamano wrote:\n>\n> Christian Couder writes:\n\n> > -When reading trailers, there can be whitespaces after the\n> > -token, the separator and the value. There can also be whitespaces\n> > -inside the token and the value. The value may be split over multiple lines with\n> > -each subsequent line starting with whitespace, like the \"folding\" in RFC 822.\n> > +When reading trailers, there can be no whitespace inside the token,\n> > +and only one regular space or tab character between the token and the\n> > +separator.\n>\n> That may have been the intent, but does it match the behaviour?\n>\n> static ssize_t find_separator(const char *line, const char *separators)\n> {\n> int whitespace_found = 0;\n> const char *c;\n> for (c = line; *c; c++) {\n> if (strchr(separators, *c))\n> return c - line;\n> if (!whitespace_found && (isalnum(*c) || *c == '-'))\n> continue;\n> if (c != line && (*c == ' ' || *c == '\\t')) {\n> whitespace_found = 1;\n> continue;\n> }\n> break;\n> }\n> return -1;\n> }\n>\n> When parsing \"X :\", first we encounter 'X', we haven't seen\n> whitespace, 'X' passes isalnum(), and we continue. Then we\n> encounter ' ', we haven't seen whitespace but it is neither isalnum\n> or dash, so we go on without hitting the first continue. We are not\n> at the beginning of the line, we are seeing a space, so we remember\n> the fact that we saw whitespace and continue. Next we see another ' ',\n> we do not hit the first continue, we are not at the beginning of the\n> line, and we are looking at ' ', so we again continue. Finally, we see\n> ':' that is a separator and we return happily.\n>\n> The code seems to be allowing zero or more space/tab before the\n> separator.\n\nYeah, I agree. I misread the code. I will send a new version soon.\n\nThanks.\n"},{"id":"461960","messageId":"xmqqsflk2t3e.fsf@gitster.g","threadId":"58318","inReplyTo":"CAP8UFD0EfhDo9ZMNSYp8YVHXJs6mYtSBs=hwoFZWWF3xZ0wjpg@mail.gmail.com","subject":"Re: [PATCH] Documentation: clarify whitespace rules for trailers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-25T16:20:05Z","receivedAt":"2022-08-25T16:20:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n>> The code seems to be allowing zero or more space/tab before the\n>> separator.\n>\n> Yeah, I agree. I misread the code. I will send a new version soon.\n\nI recall I did suggest to tighten to \"zero or one\" some point in the\ndiscussion, but we decided to correctly document what we do right\nnow, so let's do that first.\n\nThanks.\n"},{"id":"462184","messageId":"20220830105046.324041-1-christian.couder@gmail.com","threadId":"58318","inReplyTo":"CAP8UFD0EfhDo9ZMNSYp8YVHXJs6mYtSBs=hwoFZWWF3xZ0wjpg@mail.gmail.com","subject":"[PATCH v2] Documentation: clarify whitespace rules for trailers","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2022-08-30T10:50:46Z","receivedAt":"2022-08-30T10:52:11Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Commit e4319562bc (trailer: be stricter in parsing separators, 2016-11-02)\nrestricted whitespaces allowed by `git interpret-trailers` in the \"token\"\npart of the trailers it reads. This commit didn't update the related\ndocumentation in Documentation/git-interpret-trailers.txt though.\n\nAlso commit 60ef86a162 (trailer: support values folded to multiple lines,\n2016-10-21) updated the documentation, but didn't make it clear how many\nwhitespace characters are allowed at the beginning of new lines in folded\nvalues.\n\nLet's fix both of these issues by rewriting the paragraph describing\nwhat whitespaces are allowed by `git interpret-trailers` in the trailers\nit reads.\n\nSigned-off-by: Christian Couder \n---\nChanges since v1:\n\n - rebased on top of 6c8e4ee870 (The sixteenth batch, 2022-08-29),\n - added my \"Signed-off-by: ...\",\n - clarified that there can be no whitespace before the token,\n - clarified that \"any number of regular space and tab characters are\n allowed between the token and the separator\".\n\n Documentation/git-interpret-trailers.txt | 10 ++++++----\n 1 file changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\nindex 956a01d184..6d6197cd0a 100644\n--- a/Documentation/git-interpret-trailers.txt\n+++ b/Documentation/git-interpret-trailers.txt\n@@ -60,10 +60,12 @@ non-whitespace lines before a line that starts with '---' (followed by a\n space or the end of the line). Such three minus signs start the patch\n part of the message. See also `--no-divider` below.\n \n-When reading trailers, there can be whitespaces after the\n-token, the separator and the value. There can also be whitespaces\n-inside the token and the value. The value may be split over multiple lines with\n-each subsequent line starting with whitespace, like the \"folding\" in RFC 822.\n+When reading trailers, there can be no whitespace before or inside the\n+token, but any number of regular space and tab characters are allowed\n+between the token and the separator. There can be whitespaces before,\n+inside or after the value. The value may be split over multiple lines\n+with each subsequent line starting with at least one whitespace, like\n+the \"folding\" in RFC 822.\n \n Note that 'trailers' do not follow and are not intended to follow many\n rules for RFC 822 headers. For example they do not follow\n-- \n2.37.2.383.g1c78c54f66\n\n"},{"id":"462209","messageId":"xmqqh71tlkc1.fsf@gitster.g","threadId":"58318","inReplyTo":"20220830105046.324041-1-christian.couder@gmail.com","subject":"Re: [PATCH v2] Documentation: clarify whitespace rules for trailers","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-08-30T17:20:14Z","receivedAt":"2022-08-30T17:21:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Christian Couder writes:\n\n> - rebased on top of 6c8e4ee870 (The sixteenth batch, 2022-08-29),\n> - added my \"Signed-off-by: ...\",\n> - clarified that there can be no whitespace before the token,\n> - clarified that \"any number of regular space and tab characters are\n> allowed between the token and the separator\".\n>\n> Documentation/git-interpret-trailers.txt | 10 ++++++----\n> 1 file changed, 6 insertions(+), 4 deletions(-)\n\nLooks good. Thanks. Will queue.\n\n>\n> diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt\n> index 956a01d184..6d6197cd0a 100644\n> --- a/Documentation/git-interpret-trailers.txt\n> +++ b/Documentation/git-interpret-trailers.txt\n> @@ -60,10 +60,12 @@ non-whitespace lines before a line that starts with '---' (followed by a\n> space or the end of the line). Such three minus signs start the patch\n> part of the message. See also `--no-divider` below.\n> \n> -When reading trailers, there can be whitespaces after the\n> -token, the separator and the value. There can also be whitespaces\n> -inside the token and the value. The value may be split over multiple lines with\n> -each subsequent line starting with whitespace, like the \"folding\" in RFC 822.\n> +When reading trailers, there can be no whitespace before or inside the\n> +token, but any number of regular space and tab characters are allowed\n> +between the token and the separator. There can be whitespaces before,\n> +inside or after the value. The value may be split over multiple lines\n> +with each subsequent line starting with at least one whitespace, like\n> +the \"folding\" in RFC 822.\n> \n> Note that 'trailers' do not follow and are not intended to follow many\n> rules for RFC 822 headers. For example they do not follow\n"}]}