{"thread":{"id":"42745","subject":"[PATCH 0/5] Use skip_blank_lines() for more commit messages","startedAt":"2016-06-29T14:14:47Z","lastAt":"2016-07-02T06:35:06Z","messageCount":11,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"290482","messageId":"cover.1467209576.git.johannes.schindelin@gmx.de","threadId":"42745","inReplyTo":null,"subject":"[PATCH 0/5] Use skip_blank_lines() for more commit messages","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-29T14:14:37Z","receivedAt":"2016-06-29T14:14:47Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"These patches are on top of js/find-commit-subject-ignore-leading-blanks\n(whether you want to put them into the same topic branch before merging\nor put them into their own add-on topic branch does not really matter to\nme).\n\nIt occurred to me that there might be more code locations where finding\nthe commit object's body is hand-rolled, and could benefit from using\nthe skip_blank_lines() function to handle not-quite-standard but\nstill-allowed objects from being processed correctly.\n\nI found a couple of instances, and made them consistent with the\npretty-printing machinery.\n\nI made fine-grained commits to allow for dropping contentious patches\nmore easily.\n\nThere are four more instances which I did *not* change:\n\n- fast-export: I designed this command to be as faithful in exporting\n the commits as possible. As such, even non-standard commit objects\n should be kept as-are.\n\n- fmt-merge-msg: when showing signed commit messages, it is better to\n show the commit messages verbatim rather than trimming them.\n\n- when signing a commit, it is better to use the original commit message\n than a munged one.\n\n- notes' messages are left alone. I do not know whether there might be\n an intentional use of leading blank lines, but I wanted to err on the\n safe side.\n\nI also considered treating tag messages the same way, but tags are much\nmore likely to be signed, and I did not want to risk munging the tag\nmessages just before signing them.\n\n\nJohannes Schindelin (5):\n commit -C: skip blank lines at the beginning of the message\n sequencer: use skip_blank_lines() to find the commit subject\n reset --hard: skip blank lines when reporting the commit subject\n commit -S: avoid invalid pointer with empty message\n Skip blank lines when matching ^{/foo}\n\n builtin/commit.c | 2 +-\n builtin/reset.c | 2 +-\n commit.c | 6 +++++-\n sequencer.c | 6 ++----\n sha1_name.c | 3 ++-\n 5 files changed, 11 insertions(+), 8 deletions(-)\n\nPublished-As: https://github.com/dscho/git/releases/tag/more-blank-line-skipping-v1\n-- \n2.9.0.270.g810e421\n\nbase-commit: 38a2ea1900489a76d9840002bf60bd7ff29f07cd\n"},{"id":"290483","messageId":"043eafd7ad4128d3a887c224ec6576d96872d1df.1467209576.git.johannes.schindelin@gmx.de","threadId":"42745","inReplyTo":"cover.1467209576.git.johannes.schindelin@gmx.de","subject":"[PATCH 1/5] commit -C: skip blank lines at the beginning of the message","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-29T14:14:42Z","receivedAt":"2016-06-29T14:14:54Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Consistent with the pretty-printing machinery, we skip leading blank\nlines (if any) of existing commit messages.\n\nWhile Git itself only produces commit objects with a single empty line\nbetween commit header and commit message, it is legal to have more than\none blank line (i.e. lines containing only white space, or no\ncharacters) at the beginning of the commit message, and the\npretty-printing code already handles that.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin/commit.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 3f18942..1f6dbcd 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -715,7 +715,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\tchar *buffer;\n \t\tbuffer = strstr(use_message_buffer, \"\\n\\n\");\n \t\tif (buffer)\n-\t\t\tstrbuf_addstr(&sb, buffer + 2);\n+\t\t\tstrbuf_addstr(&sb, skip_blank_lines(buffer + 2));\n \t\thook_arg1 = \"commit\";\n \t\thook_arg2 = use_message;\n \t} else if (fixup_message) {\n-- \n2.9.0.270.g810e421\n\n\n"},{"id":"290484","messageId":"69bc831064331117141c7153dc72e9e658691155.1467209576.git.johannes.schindelin@gmx.de","threadId":"42745","inReplyTo":"cover.1467209576.git.johannes.schindelin@gmx.de","subject":"[PATCH 2/5] sequencer: use skip_blank_lines() to find the commit subject","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-29T14:14:46Z","receivedAt":"2016-06-29T14:15:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Just like we already taught the find_commit_subject() function (to make\nit consistent with the code in pretty.c), we now simply skip leading\nblank lines of the commit message.\n\nSigned-off-by: Johannes Schindelin \n---\n sequencer.c | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 7d7add6..cdfac82 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -544,10 +544,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n \t\t * information followed by \"\\n\\n\".\n \t\t */\n \t\tp = strstr(msg.message, \"\\n\\n\");\n-\t\tif (p) {\n-\t\t\tp += 2;\n-\t\t\tstrbuf_addstr(&msgbuf, p);\n-\t\t}\n+\t\tif (p)\n+\t\t\tstrbuf_addstr(&msgbuf, skip_blank_lines(p + 2));\n \n \t\tif (opts->record_origin) {\n \t\t\tif (!has_conforming_footer(&msgbuf, NULL, 0))\n-- \n2.9.0.270.g810e421\n\n\n"},{"id":"290485","messageId":"fd3b5aee740e88ceff0fbe77c61a10767c4f8622.1467209576.git.johannes.schindelin@gmx.de","threadId":"42745","inReplyTo":"cover.1467209576.git.johannes.schindelin@gmx.de","subject":"[PATCH 3/5] reset --hard: skip blank lines when reporting the commit subject","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-29T14:14:50Z","receivedAt":"2016-06-29T14:15:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When there are blank lines at the beginning of a commit message, the\npretty printing machinery already skips them when showing a commit\nsubject (or the complete commit message). We shall henceforth do the\nsame when reporting the commit subject after the user called\n\n\tgit reset --hard \n\nSigned-off-by: Johannes Schindelin \n---\n builtin/reset.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex acd6278..5c6206b 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -103,7 +103,7 @@ static void print_new_head_line(struct commit *commit)\n \tif (body) {\n \t\tconst char *eol;\n \t\tsize_t len;\n-\t\tbody += 2;\n+\t\tbody = skip_blank_lines(body + 2);\n \t\teol = strchr(body, '\\n');\n \t\tlen = eol ? eol - body : strlen(body);\n \t\tprintf(\" %.*s\\n\", (int) len, body);\n-- \n2.9.0.270.g810e421\n\n\n"},{"id":"290486","messageId":"51b87a251ac80177ff7321ec0e182cbda6dd93a1.1467209576.git.johannes.schindelin@gmx.de","threadId":"42745","inReplyTo":"cover.1467209576.git.johannes.schindelin@gmx.de","subject":"[PATCH 4/5] commit -S: avoid invalid pointer with empty message","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-29T14:14:54Z","receivedAt":"2016-06-29T14:15:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"While it is not recommended, `git fsck` says that:\n\n\tNot having a body is not a crime [...]\n\n... which means that we cannot assume that the commit buffer contains an\nempty line to separate header from body (essentially saying that there\nis only a header).\n\nSo let's tread carefully here.\n\nSigned-off-by: Johannes Schindelin \n---\n commit.c | 6 +++++-\n 1 file changed, 5 insertions(+), 1 deletion(-)\n\ndiff --git a/commit.c b/commit.c\nindex 24d4715..0bb51a2 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -1090,11 +1090,15 @@ static const int gpg_sig_header_len = sizeof(gpg_sig_header) - 1;\n \n static int do_sign_commit(struct strbuf *buf, const char *keyid)\n {\n+\tconst char *eoh = strstr(buf->buf, \"\\n\\n\");\n \tstruct strbuf sig = STRBUF_INIT;\n \tint inspos, copypos;\n \n \t/* find the end of the header */\n-\tinspos = strstr(buf->buf, \"\\n\\n\") - buf->buf + 1;\n+\tif (!eoh)\n+\t\tinspos = buf->len;\n+\telse\n+\t\tinspos = eoh - buf->buf + 1;\n \n \tif (!keyid || !*keyid)\n \t\tkeyid = get_signing_key();\n-- \n2.9.0.270.g810e421\n\n\n"},{"id":"290487","messageId":"7cebdcd80a93d1604258e9dc9cb286cd6ba97ba8.1467209576.git.johannes.schindelin@gmx.de","threadId":"42745","inReplyTo":"cover.1467209576.git.johannes.schindelin@gmx.de","subject":"[PATCH 5/5] Skip blank lines when matching ^{/foo}","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-29T14:15:00Z","receivedAt":"2016-06-29T14:15:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"When trying to match a pattern in the commit subject, simply skip leading\nblank lines in the commit message. This is consistent with the\npretty-printing machinery: it silently ignores leading blank lines in the\ncommit object's body.\n\nSigned-off-by: Johannes Schindelin \n---\n sha1_name.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex ca7ddd6..da354a5 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -912,7 +912,8 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1,\n \t\t\tcontinue;\n \t\tbuf = get_commit_buffer(commit, NULL);\n \t\tp = strstr(buf, \"\\n\\n\");\n-\t\tmatches = negative ^ (p && !regexec(®ex, p + 2, 0, NULL, 0));\n+\t\tmatches = negative ^ (p && !regexec(®ex,\n+\t\t\tskip_blank_lines(p + 2), 0, NULL, 0));\n \t\tunuse_commit_buffer(commit, buf);\n \n \t\tif (matches) {\n-- \n2.9.0.270.g810e421\n"},{"id":"290515","messageId":"xmqqlh1n51eo.fsf@gitster.mtv.corp.google.com","threadId":"42745","inReplyTo":"cover.1467209576.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 0/5] Use skip_blank_lines() for more commit messages","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-29T17:51:27Z","receivedAt":"2016-06-29T17:52:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> It occurred to me that there might be more code locations where finding\n> the commit object's body is hand-rolled, and could benefit from using\n> the skip_blank_lines() function to handle not-quite-standard but\n> still-allowed objects from being processed correctly.\n\nThanks for following up on this one. We could have left them as\nlow-hanging-fruit for new people, but doing these ourselves while\nwe still remember is a good idea.\n"},{"id":"290546","messageId":"xmqqpoqz1wmh.fsf@gitster.mtv.corp.google.com","threadId":"42745","inReplyTo":"69bc831064331117141c7153dc72e9e658691155.1467209576.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 2/5] sequencer: use skip_blank_lines() to find the commit subject","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-29T22:03:02Z","receivedAt":"2016-06-29T22:04:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Just like we already taught the find_commit_subject() function (to make\n> it consistent with the code in pretty.c), we now simply skip leading\n> blank lines of the commit message.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> sequencer.c | 6 ++----\n> 1 file changed, 2 insertions(+), 4 deletions(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index 7d7add6..cdfac82 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -544,10 +544,8 @@ static int do_pick_commit(struct commit *commit, struct replay_opts *opts)\n> \t\t * information followed by \"\\n\\n\".\n> \t\t */\n> \t\tp = strstr(msg.message, \"\\n\\n\");\n> -\t\tif (p) {\n> -\t\t\tp += 2;\n> -\t\t\tstrbuf_addstr(&msgbuf, p);\n> -\t\t}\n> +\t\tif (p)\n> +\t\t\tstrbuf_addstr(&msgbuf, skip_blank_lines(p + 2));\n> \n> \t\tif (opts->record_origin) {\n> \t\t\tif (!has_conforming_footer(&msgbuf, NULL, 0))\n\nHere, msg.message was read in get_message() by calling\nlogmsg_reencode() that reads from get_commit_buffer(). The code\nstructure is exactly the same as the previous one.\n\nGood.\n"},{"id":"290547","messageId":"xmqqtwgb1ws6.fsf@gitster.mtv.corp.google.com","threadId":"42745","inReplyTo":"043eafd7ad4128d3a887c224ec6576d96872d1df.1467209576.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 1/5] commit -C: skip blank lines at the beginning of the message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-29T21:59:37Z","receivedAt":"2016-06-29T22:05:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Consistent with the pretty-printing machinery, we skip leading blank\n> lines (if any) of existing commit messages.\n>\n> While Git itself only produces commit objects with a single empty line\n> between commit header and commit message, it is legal to have more than\n> one blank line (i.e. lines containing only white space, or no\n> characters) at the beginning of the commit message, and the\n> pretty-printing code already handles that.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n> builtin/commit.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 3f18942..1f6dbcd 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -715,7 +715,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n> \t\tchar *buffer;\n> \t\tbuffer = strstr(use_message_buffer, \"\\n\\n\");\n> \t\tif (buffer)\n> -\t\t\tstrbuf_addstr(&sb, buffer + 2);\n> +\t\t\tstrbuf_addstr(&sb, skip_blank_lines(buffer + 2));\n> \t\thook_arg1 = \"commit\";\n> \t\thook_arg2 = use_message;\n> \t} else if (fixup_message) {\n\nuse_message_buffer is the contents of the commit object read\nelsewhere, and strstr() skips the header part, so this follows\nexactly the same pattern as the one you fixed earlier. Good.\n"},{"id":"290551","messageId":"xmqqh9cb1ub0.fsf@gitster.mtv.corp.google.com","threadId":"42745","inReplyTo":"7cebdcd80a93d1604258e9dc9cb286cd6ba97ba8.1467209576.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH 5/5] Skip blank lines when matching ^{/foo}","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-29T22:53:07Z","receivedAt":"2016-06-29T23:00:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> When trying to match a pattern in the commit subject, simply skip leading\n> blank lines in the commit message. This is consistent with the\n> pretty-printing machinery: it silently ignores leading blank lines in the\n> commit object's body.\n>\n> Signed-off-by: Johannes Schindelin \n> ---\n\nHmm.\n\nI just tried\n\n $ git commit -s -m ' pull: fast-forward \"pull --rebase=true\"'\n\nexpecting that \":/^ pull:\" would find it ;-)\n\nThe point of this patch is to make it not work, of course, which \nmay or may not be a good thing. I'd rather keep this separate from\nthe other fixes.\n\nAlso 4/5 is another different class of true bugfix, I would suspect,\nso let's take it on a separate topic that can be merged down to\nolder maintainance tracks.\n\nThanks.\n\n> sha1_name.c | 3 ++-\n> 1 file changed, 2 insertions(+), 1 deletion(-)\n>\n> diff --git a/sha1_name.c b/sha1_name.c\n> index ca7ddd6..da354a5 100644\n> --- a/sha1_name.c\n> +++ b/sha1_name.c\n> @@ -912,7 +912,8 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1,\n> \t\t\tcontinue;\n> \t\tbuf = get_commit_buffer(commit, NULL);\n> \t\tp = strstr(buf, \"\\n\\n\");\n> -\t\tmatches = negative ^ (p && !regexec(®ex, p + 2, 0, NULL, 0));\n> +\t\tmatches = negative ^ (p && !regexec(®ex,\n> +\t\t\tskip_blank_lines(p + 2), 0, NULL, 0));\n> \t\tunuse_commit_buffer(commit, buf);\n> \n> \t\tif (matches) {\n"},{"id":"290733","messageId":"alpine.DEB.2.20.1607020826530.12947@virtualbox","threadId":"42745","inReplyTo":"xmqqh9cb1ub0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 5/5] Skip blank lines when matching ^{/foo}","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-02T06:34:29Z","receivedAt":"2016-07-02T06:35:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Wed, 29 Jun 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > When trying to match a pattern in the commit subject, simply skip leading\n> > blank lines in the commit message. This is consistent with the\n> > pretty-printing machinery: it silently ignores leading blank lines in the\n> > commit object's body.\n> >\n> > Signed-off-by: Johannes Schindelin \n> > ---\n> \n> Hmm.\n> \n> I just tried\n> \n> $ git commit -s -m ' pull: fast-forward \"pull --rebase=true\"'\n> \n> expecting that \":/^ pull:\" would find it ;-)\n\nThis patch would try to address a different concern, though:\n\n\tgit write-object -w --stdin <<-EOF\n\ttree $(git rev-parse HEAD:)\n\tparent $(git rev-parse HEAD)\n\tauthor B O Gus 1234567890 +0000\n\tcommitter F A Ke 1234567890 +0000\n\n\n\tThere are two blank lines before the subject\n\tEOF\n\nIt is not about leading whitespace in the commit subject, but about blank\nlines before it.\n\nGranted, our current tools do not generate such commit objects, but we\naccept them, and already handle them in some places.\n\n> Also 4/5 is another different class of true bugfix, I would suspect,\n> so let's take it on a separate topic that can be merged down to\n> older maintainance tracks.\n\nSure!\n\nThanks,\nDscho\n"}]}