{"thread":{"id":"63517","subject":"[PATCH] notes: remove trailing whitespace from editor template","startedAt":"2025-05-24T21:35:13Z","lastAt":"2025-06-10T22:56:01Z","messageCount":17,"participants":["kristofferhaugsbakk@fastmail.com","Kristoffer Haugsbakk","Phillip Wood","Junio C Hamano","Eric Sunshine","D. Ben Knoble"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"518848","messageId":"c59ae2c0c7c8420ec1c5bedb87f28c7f5b573a60.1748122397.git.code@khaugsbakk.name","threadId":"63517","inReplyTo":null,"subject":"[PATCH] notes: remove trailing whitespace from editor template","fromName":"","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-05-24T21:35:02Z","receivedAt":"2025-05-24T21:35:13Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"From: Kristoffer Haugsbakk <code@khaugsbakk.name>\n\nThe editor template for editing a note consists of the commented block:\n\n    git show --stat --no-notes <object>\n\nThe indented commit message will introduce trailing whitespace for\nparagraph breaks (blank lines).  Some editors will highlight those lines\nas an error immediately when you open the editor.\n\nLet’s strip all unnecessary whitespace from the template to avoid that\nvery small problem.\n\nSigned-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>\n---\n builtin/notes.c  | 2 ++\n t/t3301-notes.sh | 6 ++++++\n 2 files changed, 8 insertions(+)\n\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex a3f433ca4c0..ca4782eca19 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -180,6 +180,8 @@ static void write_commented_object(int fd, const struct object_id *object)\n \tif (strbuf_read(&buf, show.out, 0) < 0)\n \t\tdie_errno(_(\"could not read 'show' output\"));\n \tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str);\n+\t/* strip trailing whitespace introduced by blank lines */\n+\tstrbuf_stripspace(&cbuf, NULL);\n \twrite_or_die(fd, cbuf.buf, cbuf.len);\n \n \tstrbuf_release(&cbuf);\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex d6c50460d08..70a21be54fc 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -1629,4 +1629,10 @@ test_expect_success 'git notes append aborts when editor fails with -e' '\n \ttest_must_fail git notes show\n '\n \n+test_expect_success 'git notes add has no trailing whitespace in the editor template' '\n+\ttest_commit --signoff 23rd &&\n+\tGIT_EDITOR=\"cat >actual\" git notes add &&\n+\ttest_grep ! \" $\" actual\n+'\n+\n test_done\n\nbase-commit: 8613c2bb6cd16ef530dc5dd74d3b818a1ccbf1c0\n-- \n2.49.0.780.g892193c3f50\n\n"},{"id":"518893","messageId":"a3c252cf-dec3-4b2c-828e-f25e48853585@app.fastmail.com","threadId":"63517","inReplyTo":"c59ae2c0c7c8420ec1c5bedb87f28c7f5b573a60.1748122397.git.code@khaugsbakk.name","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2025-05-25T20:46:35Z","receivedAt":"2025-05-25T20:47:02Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Sat, May 24, 2025, at 23:35, kristofferhaugsbakk@fastmail.com wrote:\n> +\ttest_commit --signoff 23rd &&\n> +\tGIT_EDITOR=\"cat >actual\" git notes add &&\n> +\ttest_grep ! \" $\" actual\n> +'\n> +\n>  test_done\n\nOr alternatively I could assert on the whole template:\n\n```\ntest_expect_success 'git notes add editor template' '\n\ttest_commit --signoff 23rd &&\n\tcat <<-EOF >expect &&\n\n\t#\n\t# Write/edit the notes for the following object:\n\t#\n\t$(git show --stat --no-notes 23rd |\n\t\t    git stripspace |\n\t\t    git stripspace --comment-lines)\n\tEOF\n\tGIT_EDITOR=\"cat >actual\" git notes add &&\n\ttest_cmp expect actual\n'\n```\n\n-- \nKristoffer\n\n"},{"id":"518935","messageId":"66e92d69-8372-47cf-a350-95365f72ca1c@gmail.com","threadId":"63517","inReplyTo":"c59ae2c0c7c8420ec1c5bedb87f28c7f5b573a60.1748122397.git.code@khaugsbakk.name","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-05-26T14:01:04Z","receivedAt":"2025-05-26T14:01:07Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Kristoffer\n\nOn 24/05/2025 22:35, kristofferhaugsbakk@fastmail.com wrote:\n> \n> diff --git a/builtin/notes.c b/builtin/notes.c\n> index a3f433ca4c0..ca4782eca19 100644\n> --- a/builtin/notes.c\n> +++ b/builtin/notes.c\n> @@ -180,6 +180,8 @@ static void write_commented_object(int fd, const struct object_id *object)\n>   \tif (strbuf_read(&buf, show.out, 0) < 0)\n>   \t\tdie_errno(_(\"could not read 'show' output\"));\n>   \tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str);\n> +\t/* strip trailing whitespace introduced by blank lines */\n> +\tstrbuf_stripspace(&cbuf, NULL);\n\nIt doesn't make any difference at the moment but I'd be happier if we \nstripped the trailing space from the commit message before commenting it \nout. That way we know we are only stripping space from the indented \nlines produced by \"git show\". If in the future this function were to \nstart appending the commented log message to a buffer passed in by the \ncaller rather than a file passed by the caller we wont mess up the rest \nof the buffer content.\n\n>   \twrite_or_die(fd, cbuf.buf, cbuf.len);\n > [...]> +test_expect_success 'git notes add has no trailing whitespace \nin the editor template' '\n> +\ttest_commit --signoff 23rd &&\n> +\tGIT_EDITOR=\"cat >actual\" git notes add &&\n> +\ttest_grep ! \" $\" actual\n\nShould that be \" \\$\"? What you've got seems to work with dash but I'm \nnot sure if it is POSIX compliant or not.\n\nBest Wishes\n\nPhillip\n"},{"id":"518952","messageId":"10280d7d-af36-468d-82b8-e0e780c38ef1@app.fastmail.com","threadId":"63517","inReplyTo":"66e92d69-8372-47cf-a350-95365f72ca1c@gmail.com","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2025-05-26T19:44:48Z","receivedAt":"2025-05-26T19:45:11Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"Hi there Phillip\n\nOn Mon, May 26, 2025, at 16:01, Phillip Wood wrote:\n>> diff --git a/builtin/notes.c b/builtin/notes.c\n>> index a3f433ca4c0..ca4782eca19 100644\n>> --- a/builtin/notes.c\n>> +++ b/builtin/notes.c\n>> @@ -180,6 +180,8 @@ static void write_commented_object(int fd, const struct object_id *object)\n>>   \tif (strbuf_read(&buf, show.out, 0) < 0)\n>>   \t\tdie_errno(_(\"could not read 'show' output\"));\n>>   \tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str);\n>> +\t/* strip trailing whitespace introduced by blank lines */\n>> +\tstrbuf_stripspace(&cbuf, NULL);\n>\n> It doesn't make any difference at the moment but I'd be happier if we\n> stripped the trailing space from the commit message before commenting it\n> out. That way we know we are only stripping space from the indented\n> lines produced by \"git show\". If in the future this function were to\n> start appending the commented log message to a buffer passed in by the\n> caller rather than a file passed by the caller we wont mess up the rest\n> of the buffer content.\n\nDo you mean doing the operation on the output buffer instead?:\n\n\tif (strbuf_read(&buf, show.out, 0) < 0)\n\t\tdie_errno(_(\"could not read 'show' output\"));\n\t/* strip trailing whitespace introduced by blank lines */\n\tstrbuf_stripspace(&buf, NULL);\n\tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str);\n\twrite_or_die(fd, cbuf.buf, cbuf.len);\n\nI think that’s cleaner.  But I don’t see how it makes the code more\nfuture-proof.\n\n>\n>>   \twrite_or_die(fd, cbuf.buf, cbuf.len);\n>  > [...]> +test_expect_success 'git notes add has no trailing whitespace\n> in the editor template' '\n>> +\ttest_commit --signoff 23rd &&\n>> +\tGIT_EDITOR=\"cat >actual\" git notes add &&\n>> +\ttest_grep ! \" $\" actual\n>\n> Should that be \" \\$\"? What you've got seems to work with dash but I'm\n> not sure if it is POSIX compliant or not.\n\n`$` is the anchor metacharacter in this context (end of string)\naccording to Posix.\n\nhttps://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_03\n\n    $\n        The <dollar-sign> shall be special when used as an anchor.\n"},{"id":"518963","messageId":"ab009472-d15d-4894-aa83-0ab8b0d2dfbd@gmail.com","threadId":"63517","inReplyTo":"10280d7d-af36-468d-82b8-e0e780c38ef1@app.fastmail.com","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-05-27T08:24:26Z","receivedAt":"2025-05-27T08:24:31Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Kristoffer\n\nOn 26/05/2025 20:44, Kristoffer Haugsbakk wrote:\n> On Mon, May 26, 2025, at 16:01, Phillip Wood wrote:\n>>> diff --git a/builtin/notes.c b/builtin/notes.c\n>>> index a3f433ca4c0..ca4782eca19 100644\n>>> --- a/builtin/notes.c\n>>> +++ b/builtin/notes.c\n>>> @@ -180,6 +180,8 @@ static void write_commented_object(int fd, const struct object_id *object)\n>>>    \tif (strbuf_read(&buf, show.out, 0) < 0)\n>>>    \t\tdie_errno(_(\"could not read 'show' output\"));\n>>>    \tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str);\n>>> +\t/* strip trailing whitespace introduced by blank lines */\n>>> +\tstrbuf_stripspace(&cbuf, NULL);\n>>\n>> It doesn't make any difference at the moment but I'd be happier if we\n>> stripped the trailing space from the commit message before commenting it\n>> out. That way we know we are only stripping space from the indented\n>> lines produced by \"git show\". If in the future this function were to\n>> start appending the commented log message to a buffer passed in by the\n>> caller rather than a file passed by the caller we wont mess up the rest\n>> of the buffer content.\n> \n> Do you mean doing the operation on the output buffer instead?:\n> \n> \tif (strbuf_read(&buf, show.out, 0) < 0)\n> \t\tdie_errno(_(\"could not read 'show' output\"));\n> \t/* strip trailing whitespace introduced by blank lines */\n> \tstrbuf_stripspace(&buf, NULL);\n> \tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str);\n> \twrite_or_die(fd, cbuf.buf, cbuf.len);\n> \n> I think that’s cleaner.  But I don’t see how it makes the code more\n> future-proof.\n\nBecause it is now stripping buf and not cbuf. If in the future we decide \nto build the message in a buffer rather than writing it piecemeal to \ndisk we would change signature of this function to take an strbuf \ninstead of a file descriptor and use the buffer provided by the caller \ninstead of cbuf. If we were to strip cbuf then a naive conversion would \nend up stripping the buffer passed by caller, not just the output from \n\"git show\". Various git notes subcommands have a --stripspace option and \ncalling strbuf_stripspace() on the caller provided buffer would break that.\n\n>>>    \twrite_or_die(fd, cbuf.buf, cbuf.len);\n>>   > [...]> +test_expect_success 'git notes add has no trailing whitespace\n>> in the editor template' '\n>>> +\ttest_commit --signoff 23rd &&\n>>> +\tGIT_EDITOR=\"cat >actual\" git notes add &&\n>>> +\ttest_grep ! \" $\" actual\n>>\n>> Should that be \" \\$\"? What you've got seems to work with dash but I'm\n>> not sure if it is POSIX compliant or not.\n> \n> `$` is the anchor metacharacter in this context (end of string)\n> according to Posix.\n\nRight but what does the shell do to that '$'? It is not escaped and \ninside a double quoted string.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"519001","messageId":"820b1255-c2e0-4383-aebc-46e49112fe2d@app.fastmail.com","threadId":"63517","inReplyTo":"ab009472-d15d-4894-aa83-0ab8b0d2dfbd@gmail.com","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-05-27T16:11:50Z","receivedAt":"2025-05-27T16:12:13Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Tue, May 27, 2025, at 10:24, Phillip Wood wrote:\n>> Do you mean doing the operation on the output buffer instead?:\n>>\n>> \tif (strbuf_read(&buf, show.out, 0) < 0)\n>> \t\tdie_errno(_(\"could not read 'show' output\"));\n>> \t/* strip trailing whitespace introduced by blank lines */\n>> \tstrbuf_stripspace(&buf, NULL);\n>> \tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str);\n>> \twrite_or_die(fd, cbuf.buf, cbuf.len);\n>>\n>> I think that’s cleaner.  But I don’t see how it makes the code more\n>> future-proof.\n>\n> Because it is now stripping buf and not cbuf. If in the future we decide\n> to build the message in a buffer rather than writing it piecemeal to\n> disk we would change signature of this function to take an strbuf\n> instead of a file descriptor and use the buffer provided by the caller\n> instead of cbuf. If we were to strip cbuf then a naive conversion would\n> end up stripping the buffer passed by caller, not just the output from\n> \"git show\". Various git notes subcommands have a --stripspace option and\n> calling strbuf_stripspace() on the caller provided buffer would break that.\n\nSo stripping `buf` is what I should do?\n\n>>> Should that be \" \\$\"? What you've got seems to work with dash but I'm\n>>> not sure if it is POSIX compliant or not.\n>>\n>> `$` is the anchor metacharacter in this context (end of string)\n>> according to Posix.\n>\n> Right but what does the shell do to that '$'? It is not escaped and\n> inside a double quoted string.\n\nOh right, it’s about the shell.  I’ll fix it.\n\nThanks for spotting.\n"},{"id":"519010","messageId":"xmqq5xhmvuol.fsf@gitster.g","threadId":"63517","inReplyTo":"66e92d69-8372-47cf-a350-95365f72ca1c@gmail.com","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-27T17:18:34Z","receivedAt":"2025-05-27T17:18:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> Hi Kristoffer\n>\n> On 24/05/2025 22:35, kristofferhaugsbakk@fastmail.com wrote:\n>> diff --git a/builtin/notes.c b/builtin/notes.c\n>> index a3f433ca4c0..ca4782eca19 100644\n>> --- a/builtin/notes.c\n>> +++ b/builtin/notes.c\n>> @@ -180,6 +180,8 @@ static void write_commented_object(int fd, const struct object_id *object)\n>>   \tif (strbuf_read(&buf, show.out, 0) < 0)\n>>   \t\tdie_errno(_(\"could not read 'show' output\"));\n>>   \tstrbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str);\n>> +\t/* strip trailing whitespace introduced by blank lines */\n>> +\tstrbuf_stripspace(&cbuf, NULL);\n>\n> It doesn't make any difference at the moment but I'd be happier if we\n> stripped the trailing space from the commit message before commenting\n> it out.\n\nYes.  I had the same thought.  If Kristof does not like the fact\nthat one automated source of information consistently indents its\noutput lines, even an empty one, and if users may have legitimate\nreason to place in the final output a trailing whitespace in the\ncomment, it is better for the patch not to close the door to the\nothers.\n\nIn this case I am not all that sympathetic to the idea of the patch.\nThe consistently indented lines makes it more clear from which line\nto which line came from a commit log message; running stripspace\nwould break them into paragraph pieces.  These editors that complain\nprobaly can be fixed?\n\nAlternatively, if it bothers users of certain editing environments\ntoo much, perhaps the indent code in the output phase of \"git show\"\nshould lose the indents for empty lines uniformly, shoudln't it?  It\nprobably should be a fairly isolated change, like the way how the\nexpand_tabs_in_log bit is handled in pretty.c; give another bit and\nteach pp_handle_indent to return when that bit is set and the\npayload it was asked to show with indentation is empty, or something\nlike that.\n\n> Should that be \" \\$\"? What you've got seems to work with dash but I'm\n> not sure if it is POSIX compliant or not.\n\n\"2.6 Word Expansions\" ends with this sentence:\n\n    If a '$' that is neither within single-quotes nor escaped by a\n    <backslash> is immediately followed by a <space>, <tab>, or a\n    <newline>, or is not followed by any character, the '$' shall be\n    treated as a literal character.\n\nTaken together with \"2.2.3 Double-Quotes\", I'd read it as blessing a\nlone '$' at the end of double-quoted string as a literal dollar sign.\n\nThanks.\n"},{"id":"519012","messageId":"CAPig+cScZ_0g7K-pQrxSseFX+fqjA6KAFf3nSi5cxbbCHhR5zQ@mail.gmail.com","threadId":"63517","inReplyTo":"xmqq5xhmvuol.fsf@gitster.g","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-05-27T17:28:34Z","receivedAt":"2025-05-27T17:28:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, May 27, 2025 at 1:19 PM Junio C Hamano <gitster@pobox.com> wrote:\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n> > Should that be \" \\$\"? What you've got seems to work with dash but I'm\n> > not sure if it is POSIX compliant or not.\n>\n> \"2.6 Word Expansions\" ends with this sentence:\n>\n>     If a '$' that is neither within single-quotes nor escaped by a\n>     <backslash> is immediately followed by a <space>, <tab>, or a\n>     <newline>, or is not followed by any character, the '$' shall be\n>     treated as a literal character.\n>\n> Taken together with \"2.2.3 Double-Quotes\", I'd read it as blessing a\n> lone '$' at the end of double-quoted string as a literal dollar sign.\n\nThanks for finding and posting this. I had read the POSIX shell\nlanguage specification pretty closely when writing the lexer and\nparser for chainlint.pl and remembered the above statement, and had\nwanted to cite it after reading Phillip's initial reply but didn't\nmanage to find it again in the limited time I had available for\nsearching through the documentation.\n"},{"id":"519028","messageId":"59e4082a-6cbe-4a3f-b884-1d0cc7294548@app.fastmail.com","threadId":"63517","inReplyTo":"xmqq5xhmvuol.fsf@gitster.g","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-05-27T21:21:10Z","receivedAt":"2025-05-27T21:21:34Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Tue, May 27, 2025, at 19:18, Junio C Hamano wrote:\n> Alternatively, if it bothers users of certain editing environments\n> too much, perhaps the indent code in the output phase of \"git show\"\n> should lose the indents for empty lines uniformly, shoudln't it?  It\n> probably should be a fairly isolated change, like the way how the\n> expand_tabs_in_log bit is handled in pretty.c; give another bit and\n> teach pp_handle_indent to return when that bit is set and the\n> payload it was asked to show with indentation is empty, or something\n> like that.\n\nThat sounds scary. I’ll make do without.\n\n-- \nKristoffer Haugsbakk\n"},{"id":"519030","messageId":"xmqqv7plu20y.fsf@gitster.g","threadId":"63517","inReplyTo":"xmqq5xhmvuol.fsf@gitster.g","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-27T22:22:53Z","receivedAt":"2025-05-27T22:22:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> It probably should be a fairly isolated change, like the way how\n> the expand_tabs_in_log bit is handled in pretty.c; give another\n> bit and teach pp_handle_indent to return when that bit is set and\n> the payload it was asked to show with indentation is empty, or\n> something like that.\n\nAn unconditional change based on the above idea may look like the\nattached patch.\n\nNo tests, no docs, no configuration, and I will not be working on\nthis in immediate future.  On top of what we can see here, probably\nthe following would need to happen before we can call it completed.\n\n - \"git notes\" should take a command line option to control which\n   one of \"--indent-empty-lines\" or \"--no-indent-empty-lines\" is\n   passed.  Optionally, we can introduce a new configuration\n   variable notes.something to control the same.\n\n - \"git log\" and \"git show\", but not \"git rev-list\" plumbing, may\n   want to start paying attention to a new configuration variable\n   log.indentEmptyLines (which defaults to \"yes\" if not specified).\n\n   I am not sure if we want to do this item; as there is no explicit\n   end-user request to add such a feature, I am inclined to say no,\n   at least not as a part of the \"while editing notes with 'git\n   notes add -e' the shortlog ends up with lines that are commented\n   out with trailing whitespaces, which upsets some editors\" topic.\n   But it should be trivial to do.\n\n - documentation.\n\n - tests.\n\nI've only compiled and then manually tested\n\n    $ GIT_EXEC_PATH=$(pwd) ./git notes add -e\n\nto see how the commented out \"git show -s\" output embedded in the\neditor buffer looked like.\n\n builtin/log.c   | 1 +\n builtin/notes.c | 3 ++-\n log-tree.c      | 1 +\n pretty.c        | 8 +++++---\n pretty.h        | 1 +\n revision.c      | 4 ++++\n revision.h      | 2 ++\n 7 files changed, 16 insertions(+), 4 deletions(-)\n\ndiff --git c/builtin/log.c w/builtin/log.c\nindex b450cd3bde..dc2d7d3a07 100644\n--- c/builtin/log.c\n+++ w/builtin/log.c\n@@ -2148,6 +2148,7 @@ int cmd_format_patch(int argc,\n \trev.commit_format = CMIT_FMT_EMAIL;\n \trev.encode_email_headers = cfg.log.default_encode_email_headers;\n \trev.expand_tabs_in_log_default = 0;\n+\trev.indent_empty_lines = 1;\n \trev.verbose_header = 1;\n \trev.diff = 1;\n \trev.max_parents = 1;\ndiff --git c/builtin/notes.c w/builtin/notes.c\nindex a3f433ca4c..b559b39cf0 100644\n--- c/builtin/notes.c\n+++ w/builtin/notes.c\n@@ -167,7 +167,8 @@ static void write_commented_object(int fd, const struct object_id *object)\n \tstruct strbuf cbuf = STRBUF_INIT;\n \n \t/* Invoke \"git show --stat --no-notes $object\" */\n-\tstrvec_pushl(&show.args, \"show\", \"--stat\", \"--no-notes\",\n+\tstrvec_pushl(&show.args, \"show\", \"--stat\",\n+\t\t     \"--no-indent-empty-lines\", \"--no-notes\",\n \t\t     oid_to_hex(object), NULL);\n \tshow.no_stdin = 1;\n \tshow.out = -1;\ndiff --git c/log-tree.c w/log-tree.c\nindex 1d05dc1c70..e8ea2481ea 100644\n--- c/log-tree.c\n+++ w/log-tree.c\n@@ -879,6 +879,7 @@ void show_log(struct rev_info *opt)\n \tctx.mailmap = opt->mailmap;\n \tctx.color = opt->diffopt.use_color;\n \tctx.expand_tabs_in_log = opt->expand_tabs_in_log;\n+\tctx.indent_empty_lines = opt->indent_empty_lines;\n \tctx.output_encoding = get_log_output_encoding();\n \tctx.rev = opt;\n \tif (opt->from_ident.mail_begin && opt->from_ident.name_begin)\ndiff --git c/pretty.c w/pretty.c\nindex 0bc8ad8a9a..4974af4d02 100644\n--- c/pretty.c\n+++ w/pretty.c\n@@ -2247,13 +2247,14 @@ void pp_remainder(struct pretty_print_context *pp,\n \n \tfor (;;) {\n \t\tconst char *line = *msg_p;\n+\t\tint is_blank = 0;\n \t\tint linelen = get_one_line(line);\n \t\t*msg_p += linelen;\n \n \t\tif (!linelen)\n \t\t\tbreak;\n-\n-\t\tif (is_blank_line(line, &linelen)) {\n+\t\tis_blank = is_blank_line(line, &linelen);\n+\t\tif (is_blank) {\n \t\t\tif (first)\n \t\t\t\tcontinue;\n \t\t\tif (pp->fmt == CMIT_FMT_SHORT)\n@@ -2262,7 +2263,8 @@ void pp_remainder(struct pretty_print_context *pp,\n \t\tfirst = 0;\n \n \t\tstrbuf_grow(sb, linelen + indent + 20);\n-\t\tif (indent)\n+\n+\t\tif (indent && (!is_blank || pp->indent_empty_lines))\n \t\t\tpp_handle_indent(pp, sb, indent, line, linelen);\n \t\telse if (pp->expand_tabs_in_log)\n \t\t\tstrbuf_add_tabexpand(sb, opt, pp->color,\ndiff --git c/pretty.h w/pretty.h\nindex df267afe4a..21e584e185 100644\n--- c/pretty.h\n+++ w/pretty.h\n@@ -40,6 +40,7 @@ struct pretty_print_context {\n \tstruct date_mode date_mode;\n \tunsigned date_mode_explicit:1;\n \tint expand_tabs_in_log;\n+\tint indent_empty_lines;\n \tint need_8bit_cte;\n \tchar *notes_message;\n \tstruct reflog_walk_info *reflog_info;\ndiff --git c/revision.c w/revision.c\nindex 2c36a9c179..ae6b98e701 100644\n--- c/revision.c\n+++ w/revision.c\n@@ -2564,6 +2564,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg\n \t\trevs->verbose_header = 1;\n \t\trevs->pretty_given = 1;\n \t\tget_commit_format(optarg, revs);\n+\t} else if (!strcmp(arg, \"--no-indent-empty-lines\")) {\n+\t\trevs->indent_empty_lines = 0;\n+\t} else if (!strcmp(arg, \"--indent-empty-lines\")) {\n+\t\trevs->indent_empty_lines = 1;\n \t} else if (!strcmp(arg, \"--expand-tabs\")) {\n \t\trevs->expand_tabs_in_log = 8;\n \t} else if (!strcmp(arg, \"--no-expand-tabs\")) {\ndiff --git c/revision.h w/revision.h\nindex 6d369cdad6..49b123387f 100644\n--- c/revision.h\n+++ w/revision.h\n@@ -278,6 +278,7 @@ struct rev_info {\n \tstruct date_mode date_mode;\n \tint\t\texpand_tabs_in_log; /* unset if negative */\n \tint\t\texpand_tabs_in_log_default;\n+\tint\t\tindent_empty_lines;\n \n \tunsigned int\tabbrev;\n \tenum cmit_fmt\tcommit_format;\n@@ -412,6 +413,7 @@ struct rev_info {\n \t.expand_tabs_in_log = -1, \\\n \t.commit_format = CMIT_FMT_DEFAULT, \\\n \t.expand_tabs_in_log_default = 8, \\\n+\t.indent_empty_lines = 1, \\\n }\n \n /**\n"},{"id":"519462","messageId":"21a5cbc3-3d0b-410b-80d0-94e6444f8080@gmail.com","threadId":"63517","inReplyTo":"xmqq5xhmvuol.fsf@gitster.g","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2025-06-02T13:52:37Z","receivedAt":"2025-06-02T13:52:40Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 27/05/2025 18:18, Junio C Hamano wrote:\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n> \n>> Should that be \" \\$\"? What you've got seems to work with dash but I'm\n>> not sure if it is POSIX compliant or not.\n> \n> \"2.6 Word Expansions\" ends with this sentence:\n> \n>      If a '$' that is neither within single-quotes nor escaped by a\n>      <backslash> is immediately followed by a <space>, <tab>, or a\n>      <newline>, or is not followed by any character, the '$' shall be\n>      treated as a literal character.\n> \n> Taken together with \"2.2.3 Double-Quotes\", I'd read it as blessing a\n> lone '$' at the end of double-quoted string as a literal dollar sign.\n\nThanks for finding that - TIL. I agree with your interpretation that the \noriginal is in fact safe.\n\nThanks\n\nPhillip\n\n"},{"id":"519590","messageId":"CALnO6CDgcQCuhxcJLH-XwxB85mxqokxsf04CU4yseTy-=XUWLQ@mail.gmail.com","threadId":"63517","inReplyTo":"xmqq5xhmvuol.fsf@gitster.g","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2025-06-03T20:37:31Z","receivedAt":"2025-06-03T20:37:44Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Tue, May 27, 2025 at 1:31 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n>\n> > Hi Kristoffer\n> >\n> > On 24/05/2025 22:35, kristofferhaugsbakk@fastmail.com wrote:\n> >> diff --git a/builtin/notes.c b/builtin/notes.c\n> >> index a3f433ca4c0..ca4782eca19 100644\n> >> --- a/builtin/notes.c\n> >> +++ b/builtin/notes.c\n> >> @@ -180,6 +180,8 @@ static void write_commented_object(int fd, const struct object_id *object)\n> >>      if (strbuf_read(&buf, show.out, 0) < 0)\n> >>              die_errno(_(\"could not read 'show' output\"));\n> >>      strbuf_add_commented_lines(&cbuf, buf.buf, buf.len, comment_line_str);\n> >> +    /* strip trailing whitespace introduced by blank lines */\n> >> +    strbuf_stripspace(&cbuf, NULL);\n> >\n> > It doesn't make any difference at the moment but I'd be happier if we\n> > stripped the trailing space from the commit message before commenting\n> > it out.\n>\n> Yes.  I had the same thought.  If Kristof does not like the fact\n> that one automated source of information consistently indents its\n> output lines, even an empty one, and if users may have legitimate\n> reason to place in the final output a trailing whitespace in the\n> comment, it is better for the patch not to close the door to the\n> others.\n>\n> In this case I am not all that sympathetic to the idea of the patch.\n> The consistently indented lines makes it more clear from which line\n> to which line came from a commit log message; running stripspace\n> would break them into paragraph pieces.  These editors that complain\n> probaly can be fixed?\n\nMy editor doesn't complain, but it does highlight trailing whitespace\nat my behest, and it tends to be an eyesore (on purpose: that way I\nclean it up). Perhaps Kistoffer is coming from a similar place?\n\n>\n> Alternatively, if it bothers users of certain editing environments\n> too much, perhaps the indent code in the output phase of \"git show\"\n> should lose the indents for empty lines uniformly, shoudln't it?  It\n> probably should be a fairly isolated change, like the way how the\n> expand_tabs_in_log bit is handled in pretty.c; give another bit and\n> teach pp_handle_indent to return when that bit is set and the\n> payload it was asked to show with indentation is empty, or something\n> like that.\n\nI think this suggestion would also help folks who \"git commit -v,\"\nwhich IIRC is also indented in the template.\n\n>\n> > Should that be \" \\$\"? What you've got seems to work with dash but I'm\n> > not sure if it is POSIX compliant or not.\n>\n> \"2.6 Word Expansions\" ends with this sentence:\n>\n>     If a '$' that is neither within single-quotes nor escaped by a\n>     <backslash> is immediately followed by a <space>, <tab>, or a\n>     <newline>, or is not followed by any character, the '$' shall be\n>     treated as a literal character.\n>\n> Taken together with \"2.2.3 Double-Quotes\", I'd read it as blessing a\n> lone '$' at the end of double-quoted string as a literal dollar sign.\n>\n> Thanks.\n>\n\n\n-- \nD. Ben Knoble\n"},{"id":"519593","messageId":"CAPig+cRTeZosWC=b=9MOjKaUwPodp7P=X2pwuXVgiRorx_+jmg@mail.gmail.com","threadId":"63517","inReplyTo":"CALnO6CDgcQCuhxcJLH-XwxB85mxqokxsf04CU4yseTy-=XUWLQ@mail.gmail.com","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-06-03T20:46:12Z","receivedAt":"2025-06-03T20:46:24Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Jun 3, 2025 at 4:37 PM D. Ben Knoble <ben.knoble@gmail.com> wrote:\n> On Tue, May 27, 2025 at 1:31 PM Junio C Hamano <gitster@pobox.com> wrote:\n> > In this case I am not all that sympathetic to the idea of the patch.\n> > The consistently indented lines makes it more clear from which line\n> > to which line came from a commit log message; running stripspace\n> > would break them into paragraph pieces.  These editors that complain\n> > probaly can be fixed?\n>\n> My editor doesn't complain, but it does highlight trailing whitespace\n> at my behest, and it tends to be an eyesore (on purpose: that way I\n> clean it up). Perhaps Kistoffer is coming from a similar place?\n>\n> > Alternatively, if it bothers users of certain editing environments\n> > too much, perhaps the indent code in the output phase of \"git show\"\n> > should lose the indents for empty lines uniformly, shoudln't it?  It\n> > probably should be a fairly isolated change, like the way how the\n> > expand_tabs_in_log bit is handled in pretty.c; give another bit and\n> > teach pp_handle_indent to return when that bit is set and the\n> > payload it was asked to show with indentation is empty, or something\n> > like that.\n>\n> I think this suggestion would also help folks who \"git commit -v,\"\n> which IIRC is also indented in the template.\n\nFor what it's worth, there was a previous attempt at something along\nthese lines after which a discussion ensued.\n\nhttps://lore.kernel.org/git/20210830072118.91921-4-sunshine@sunshineco.com/T/\n"},{"id":"520018","messageId":"955b6f38-0a98-419b-abac-899beaab277d@app.fastmail.com","threadId":"63517","inReplyTo":"CAPig+cRTeZosWC=b=9MOjKaUwPodp7P=X2pwuXVgiRorx_+jmg@mail.gmail.com","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Kristoffer Haugsbakk","fromEmail":"code@khaugsbakk.name","sentAt":"2025-06-09T21:10:11Z","receivedAt":"2025-06-09T21:10:33Z","isPatch":true,"sender":{"key":"code@khaugsbakk.name","avatar":"https://avatars.githubusercontent.com/u/2229597?v=4"},"body":"On Tue, Jun 3, 2025, at 22:46, Eric Sunshine wrote:\n>> I think this suggestion would also help folks who \"git commit -v,\"\n>> which IIRC is also indented in the template.\n>\n> For what it's worth, there was a previous attempt at something along\n> these lines after which a discussion ensued.\n>\n> https://lore.kernel.org/git/20210830072118.91921-4-sunshine@sunshineco.com/T/\n\nThat’s very relevant, thanks.  It’s not something that I would have\nnoticed since I am too afraid of editing any file with patch contents in\nthem.  In part that’s why I find it so convenient that you can send\nnotes along with patches.\n\n-- \nKristoffer\n"},{"id":"520019","messageId":"5ca4e740-d243-490c-ad85-13b330165365@app.fastmail.com","threadId":"63517","inReplyTo":"CALnO6CDgcQCuhxcJLH-XwxB85mxqokxsf04CU4yseTy-=XUWLQ@mail.gmail.com","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Kristoffer Haugsbakk","fromEmail":"kristofferhaugsbakk@fastmail.com","sentAt":"2025-06-09T21:12:42Z","receivedAt":"2025-06-09T21:13:05Z","isPatch":true,"sender":{"key":"kristofferhaugsbakk@fastmail.com","avatar":null},"body":"On Tue, Jun 3, 2025, at 22:37, D. Ben Knoble wrote:\n>> In this case I am not all that sympathetic to the idea of the patch.\n>> The consistently indented lines makes it more clear from which line\n>> to which line came from a commit log message; running stripspace\n>> would break them into paragraph pieces.  These editors that complain\n>> probaly can be fixed?\n>\n> My editor doesn't complain, but it does highlight trailing whitespace\n> at my behest, and it tends to be an eyesore (on purpose: that way I\n> clean it up). Perhaps Kistoffer is coming from a similar place?\n\nYes exactly.  Ain’t nothing more to it than that.  :)\n>\n>>\n>> Alternatively, if it bothers users of certain editing environments\n>> too much, perhaps the indent code in the output phase of \"git show\"\n>> should lose the indents for empty lines uniformly, shoudln't it?  It\n>> probably should be a fairly isolated change, like the way how the\n>> expand_tabs_in_log bit is handled in pretty.c; give another bit and\n>> teach pp_handle_indent to return when that bit is set and the\n>> payload it was asked to show with indentation is empty, or something\n>> like that.\n>\n> I think this suggestion would also help folks who \"git commit -v,\"\n> which IIRC is also indented in the template.\n\nIn my testing though it doesn’t introduce trailing whitespace.\n\n-- \nKristoffer Haugsbakk\n"},{"id":"520055","messageId":"CALnO6CB+ZBChPg0CLXO+i3AUEwiiF7z5euXa4N3Yh4FoEX4A4w@mail.gmail.com","threadId":"63517","inReplyTo":"5ca4e740-d243-490c-ad85-13b330165365@app.fastmail.com","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2025-06-10T21:11:39Z","receivedAt":"2025-06-10T21:11:53Z","isPatch":true,"sender":{"key":"ben.knoble@gmail.com","avatar":"https://avatars.githubusercontent.com/u/22802209?v=4"},"body":"On Mon, Jun 9, 2025 at 5:13 PM Kristoffer Haugsbakk\n<kristofferhaugsbakk@fastmail.com> wrote:\n>\n> On Tue, Jun 3, 2025, at 22:37, D. Ben Knoble wrote:\n> >> In this case I am not all that sympathetic to the idea of the patch.\n> >> The consistently indented lines makes it more clear from which line\n> >> to which line came from a commit log message; running stripspace\n> >> would break them into paragraph pieces.  These editors that complain\n> >> probaly can be fixed?\n> >\n> > My editor doesn't complain, but it does highlight trailing whitespace\n> > at my behest, and it tends to be an eyesore (on purpose: that way I\n> > clean it up). Perhaps Kistoffer is coming from a similar place?\n>\n> Yes exactly.  Ain’t nothing more to it than that.  :)\n> >\n> >>\n> >> Alternatively, if it bothers users of certain editing environments\n> >> too much, perhaps the indent code in the output phase of \"git show\"\n> >> should lose the indents for empty lines uniformly, shoudln't it?  It\n> >> probably should be a fairly isolated change, like the way how the\n> >> expand_tabs_in_log bit is handled in pretty.c; give another bit and\n> >> teach pp_handle_indent to return when that bit is set and the\n> >> payload it was asked to show with indentation is empty, or something\n> >> like that.\n> >\n> > I think this suggestion would also help folks who \"git commit -v,\"\n> > which IIRC is also indented in the template.\n>\n> In my testing though it doesn’t introduce trailing whitespace.\n\nReally? When committing a recent patch (editor template attached) with\n-v (for testing this), I saw trailing whitespace on the blank line\nunder the added feed.\n\n>\n> --\n> Kristoffer Haugsbakk\n\n\n\n-- \nD. Ben Knoble\n\n\nnews: add new feed\n; Veuillez saisir le message de validation pour vos modifications. Les lignes\n; commençant par ';' seront ignorées, et un message vide abandonne la validation.\n;\n; Sur la branche master\n; Votre branche est à jour avec 'origin/master'.\n;\n; Modifications qui seront validées :\n;\tmodifié :         links/newsboat/urls\n;\n; Fichiers non suivis:\n;\tlinks/bin/git-stack\n;\n; ------------------------ >8 ------------------------\n; Ne touchez pas à la ligne ci-dessus.\n; Tout ce qui suit sera éliminé.\ndiff --git c/links/newsboat/urls i/links/newsboat/urls\nindex 522d955d..d192de8b 100644\n--- c/links/newsboat/urls\n+++ i/links/newsboat/urls\n@@ -21,6 +21,7 @@ https://ln.ht/_/feed/~ddevault \"tech\"\n https://maia.crimew.gay/feed.xml \"tech\"\n https://www.felienne.com/feed \"tech\"\n https://ronjeffries.com/feed.xml \"tech\"\n+https://eev.ee/feeds/rss.xml \"tech\"\n \n http://journal.stuffwithstuff.com/rss.xml \"~Bob Nystrom\" \"tech\"\n https://blog.sanctum.geek.nz/feed/ \"tech\"\n"},{"id":"520062","messageId":"xmqqtt4nkxz4.fsf@gitster.g","threadId":"63517","inReplyTo":"CALnO6CB+ZBChPg0CLXO+i3AUEwiiF7z5euXa4N3Yh4FoEX4A4w@mail.gmail.com","subject":"Re: [PATCH] notes: remove trailing whitespace from editor template","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-06-10T22:55:59Z","receivedAt":"2025-06-10T22:56:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"D. Ben Knoble\" <ben.knoble@gmail.com> writes:\n\n> Really? When committing a recent patch (editor template attached) with\n> -v (for testing this), I saw trailing whitespace on the blank line\n> under the added feed.\n\nTrue.  Unless you use a special configuration, which might break\nother people's \"patch\" implementations (I think \"git apply\" has\nlearned to grok this ages ago).\n\n$ git -c diff.suppressBlankEmpty=yes commit -v\n\n\n"}]}