{"thread":{"id":"63462","subject":"[PATCH] git-gui: do not end the commit message with an empty line","startedAt":"2025-05-14T20:50:13Z","lastAt":"2025-05-15T18:49:48Z","messageCount":5,"participants":["Johannes Sixt","Junio C Hamano","Oswald Buddenhagen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"518071","messageId":"ed1ca9fa-15f0-4601-be31-8a578c7fb788@kdbg.org","threadId":"63462","inReplyTo":null,"subject":"[PATCH] git-gui: do not end the commit message with an empty line","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2025-05-14T20:50:05Z","receivedAt":"2025-05-14T20:50:13Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"The commit message is processed to remove unnecessary empty lines.\nIn particular, it is ensured that the text ends with at most one LF\ncharacter. This one is always present, because the Tk text widget\nensures that is present.\n\nHowever, we forgot that the processed text is written to the commit\nmessage file using 'puts', which also appends a LF character, so that\nthe final commit message ends with two LF. Trim all trailing LF\ncharacters, and while we are here, use `string trim`, which lets us\nremove the leading LF in the same command.\n\nReported-by: Gareth Fenn \nSigned-off-by: Johannes Sixt \n---\n lib/commit.tcl | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/lib/commit.tcl b/lib/commit.tcl\nindex a570f9cdc6a4..f3c714e600ac 100644\n--- a/lib/commit.tcl\n+++ b/lib/commit.tcl\n@@ -214,12 +214,10 @@ You must stage at least 1 file before you can commit.\n \tglobal comment_string\n \tset cmt_rx [strcat {(^|\\n)} [regsub -all {\\W} $comment_string {\\\\&}] {[^\\n]*}]\n \tregsub -all $cmt_rx $msg {\\1} msg\n-\t# Strip leading empty lines\n-\tregsub {^\\n*} $msg {} msg\n+\t# Strip leading and trailing empty lines\n+\tset msg [string trim $msg \\n]\n \t# Compress consecutive empty lines\n \tregsub -all {\\n{3,}} $msg \"\\n\\n\" msg\n-\t# Strip trailing empty line\n-\tregsub {\\n\\n$} $msg \"\\n\" msg\n \tif {$msg eq {}} {\n \t\terror_popup [mc \"Please supply a commit message.\n \n-- \n2.49.0.212.gc22db56b11\n\n"},{"id":"518073","messageId":"xmqqv7q27v49.fsf@gitster.g","threadId":"63462","inReplyTo":"ed1ca9fa-15f0-4601-be31-8a578c7fb788@kdbg.org","subject":"Re: [PATCH] git-gui: do not end the commit message with an empty line","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-05-14T21:14:30Z","receivedAt":"2025-05-14T21:14:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> The commit message is processed to remove unnecessary empty lines.\n> In particular, it is ensured that the text ends with at most one LF\n> character. This one is always present, because the Tk text widget\n> ensures that is present.\n>\n> However, we forgot that the processed text is written to the commit\n> message file using 'puts', which also appends a LF character, so that\n> the final commit message ends with two LF. Trim all trailing LF\n> characters, and while we are here, use `string trim`, which lets us\n> remove the leading LF in the same command.\n\n\nThe above reasoning look sensible.\n\nAs git-gui uses plumbing commit-tree to create the\ncommit object, it has to be more careful not to have extra blank\nlines, as it cannot assume stripspace internal to \"git commit\"?\n\n\n>\n> Reported-by: Gareth Fenn \n> Signed-off-by: Johannes Sixt \n> ---\n> lib/commit.tcl | 6 ++----\n> 1 file changed, 2 insertions(+), 4 deletions(-)\n\n> diff --git a/lib/commit.tcl b/lib/commit.tcl\n> index a570f9cdc6a4..f3c714e600ac 100644\n> --- a/lib/commit.tcl\n> +++ b/lib/commit.tcl\n> @@ -214,12 +214,10 @@ You must stage at least 1 file before you can commit.\n> \tglobal comment_string\n> \tset cmt_rx [strcat {(^|\\n)} [regsub -all {\\W} $comment_string {\\\\&}] {[^\\n]*}]\n> \tregsub -all $cmt_rx $msg {\\1} msg\n> -\t# Strip leading empty lines\n> -\tregsub {^\\n*} $msg {} msg\n> +\t# Strip leading and trailing empty lines\n> +\tset msg [string trim $msg \\n]\n> \t# Compress consecutive empty lines\n> \tregsub -all {\\n{3,}} $msg \"\\n\\n\" msg\n> -\t# Strip trailing empty line\n> -\tregsub {\\n\\n$} $msg \"\\n\" msg\n> \tif {$msg eq {}} {\n> \t\terror_popup [mc \"Please supply a commit message.\n"},{"id":"518083","messageId":"049648a9-5580-4214-bd00-c905127939b5@kdbg.org","threadId":"63462","inReplyTo":"xmqqv7q27v49.fsf@gitster.g","subject":"Re: [PATCH] git-gui: do not end the commit message with an empty line","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2025-05-15T05:36:04Z","receivedAt":"2025-05-15T05:36:13Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 14.05.25 um 23:14 schrieb Junio C Hamano:\n> As git-gui uses plumbing commit-tree to create the\n> commit object, it has to be more careful not to have extra blank\n> lines, as it cannot assume stripspace internal to \"git commit\"?\n\nCorrect. We had used git stripspace for about a month, introduced by\nb9a4386, reverted by c0698df. The reasons not to use it were old Tcl\nversions and MacOS. We may revisit this case after we raise version\nrequirements.\n\n-- Hannes\n\n"},{"id":"518088","messageId":"aCWx56e02RqAUZgw@ugly","threadId":"63462","inReplyTo":"ed1ca9fa-15f0-4601-be31-8a578c7fb788@kdbg.org","subject":"Re: [PATCH] git-gui: do not end the commit message with an empty line","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2025-05-15T09:20:39Z","receivedAt":"2025-05-15T09:20:41Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"On Wed, May 14, 2025 at 10:50:05PM +0200, Johannes Sixt wrote:\n>The commit message is processed to remove unnecessary empty lines.\n>In particular, it is ensured that the text ends with at most one LF\n>character. This one is always present, because the Tk text widget\n>ensures that is present.\n>\n>However, we forgot\n\n\"did not consider\" would be more accurate.\n\n>that the processed text is written to the commit\n>message file using 'puts', which also appends a LF character, so that\n>the final commit message ends with two LF.\n\none could suppress that with -nonewline, but the proposed code is \nshorter.\n\n>Trim all trailing LF\n>characters, and while we are here, use `string trim`, which lets us\n>remove the leading LF in the same command.\n>\n>Reported-by: Gareth Fenn \n>Signed-off-by: Johannes Sixt \n\nReviewed-by: Oswald Buddenhagen \n\n>---\n> lib/commit.tcl | 6 ++----\n> 1 file changed, 2 insertions(+), 4 deletions(-)\n>\n>diff --git a/lib/commit.tcl b/lib/commit.tcl\n>index a570f9cdc6a4..f3c714e600ac 100644\n>--- a/lib/commit.tcl\n>+++ b/lib/commit.tcl\n>@@ -214,12 +214,10 @@ You must stage at least 1 file before you can commit.\n> \tglobal comment_string\n> \tset cmt_rx [strcat {(^|\\n)} [regsub -all {\\W} $comment_string {\\\\&}] {[^\\n]*}]\n> \tregsub -all $cmt_rx $msg {\\1} msg\n>-\t# Strip leading empty lines\n>-\tregsub {^\\n*} $msg {} msg\n>+\t# Strip leading and trailing empty lines\n>\npedantically, stripping the final LF doesn't strip a trailing empty \nline, but prevents one from being created - unless the commit message is \ncompletely empty, where that would fail. i guess this case is not all \nthat important, so we can ignore it. however, it may make sense to add \nanother comment like \"puts will re-add a trailing newline\".\n\n>+\tset msg [string trim $msg \\n]\n> \t# Compress consecutive empty lines\n> \tregsub -all {\\n{3,}} $msg \"\\n\\n\" msg\n>-\t# Strip trailing empty line\n>-\tregsub {\\n\\n$} $msg \"\\n\" msg\n> \tif {$msg eq {}} {\n> \t\terror_popup [mc \"Please supply a commit message.\n> \n>-- \n>2.49.0.212.gc22db56b11\n>\n"},{"id":"518160","messageId":"3180a451-6f84-4a60-9d98-a9fd10b0fd81@kdbg.org","threadId":"63462","inReplyTo":"aCWx56e02RqAUZgw@ugly","subject":"[PATCH v2] git-gui: do not end the commit message with an empty line","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2025-05-15T18:49:45Z","receivedAt":"2025-05-15T18:49:48Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"The commit message is processed to remove unnecessary empty lines.\nIn particular, it is ensured that the text ends with at most one LF\ncharacter. This one is always present, because the Tk text widget\nensures that is present.\n\nHowever, did not consider that the processed text is written to the\ncommit message file using `puts`, which also appends a LF character,\nso that the final commit message ends with two LF. Trim all trailing\nLF characters, and while we are here, use `string trim`, which lets\nus remove the leading LF in the same command.\n\nReported-by: Gareth Fenn \nReviewed-by: Oswald Buddenhagen \nSigned-off-by: Johannes Sixt \n---\nAm 15.05.25 um 11:20 schrieb Oswald Buddenhagen:\n> On Wed, May 14, 2025 at 10:50:05PM +0200, Johannes Sixt wrote:\n>> However, we forgot\n> \n> \"did not consider\" would be more accurate.\n\nFair enough.\n\n>> +    # Strip leading and trailing empty lines\n>>\n> [...] however, it may make sense to add\n> another comment like \"puts will re-add a trailing newline\".\n\nI did that.\n\n>> +    set msg [string trim $msg \\n]\n\n lib/commit.tcl | 6 ++----\n 1 file changed, 2 insertions(+), 4 deletions(-)\n\ndiff --git a/lib/commit.tcl b/lib/commit.tcl\nindex a570f9cdc6a4..0c2be6f619cb 100644\n--- a/lib/commit.tcl\n+++ b/lib/commit.tcl\n@@ -214,12 +214,10 @@ You must stage at least 1 file before you can commit.\n \tglobal comment_string\n \tset cmt_rx [strcat {(^|\\n)} [regsub -all {\\W} $comment_string {\\\\&}] {[^\\n]*}]\n \tregsub -all $cmt_rx $msg {\\1} msg\n-\t# Strip leading empty lines\n-\tregsub {^\\n*} $msg {} msg\n+\t# Strip leading and trailing empty lines (puts adds one \\n)\n+\tset msg [string trim $msg \\n]\n \t# Compress consecutive empty lines\n \tregsub -all {\\n{3,}} $msg \"\\n\\n\" msg\n-\t# Strip trailing empty line\n-\tregsub {\\n\\n$} $msg \"\\n\" msg\n \tif {$msg eq {}} {\n \t\terror_popup [mc \"Please supply a commit message.\n \n-- \n2.49.0.212.gc22db56b11\n\n"}]}