threads / patch / 63462

patchgit-gui: do not end the commit message with an empty line

Subject: [PATCH] git-gui: do not end the commit message with an empty line

## tl;dr

5 messages between May 14, 2025 and May 15, 2025. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Johannes Sixt· May 14, 2025, 20:50 UTC · lore

The commit message is processed to remove unnecessary empty lines. In particular, it is ensured that the text ends with at most one LF character. This one is always present, because the Tk text widget ensures that is present.

However, we forgot that the processed text is written to the commit message file using 'puts', which also appends a LF character, so that the final commit message ends with two LF. Trim all trailing LF characters, and while we are here, use `string trim`, which lets us remove the leading LF in the same command.

Reported-by: Gareth Fenn <garethfenn@gmail.com>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 lib/commit.tcl | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Show changes to lib/commit.tcl +2 −4

diff --git a/lib/commit.tcl b/lib/commit.tcl
index a570f9cdc6a4..f3c714e600ac 100644
--- a/lib/commit.tcl
+++ b/lib/commit.tcl
@@ -214,12 +214,10 @@ You must stage at least 1 file before you can commit.
 	global comment_string
 	set cmt_rx [strcat {(^|\n)} [regsub -all {\W} $comment_string {\\&}] {[^\n]*}]
 	regsub -all $cmt_rx $msg {\1} msg
-	# Strip leading empty lines
-	regsub {^\n*} $msg {} msg
+	# Strip leading and trailing empty lines
+	set msg [string trim $msg \n]
 	# Compress consecutive empty lines
 	regsub -all {\n{3,}} $msg "\n\n" msg
-	# Strip trailing empty line
-	regsub {\n\n$} $msg "\n" msg
 	if {$msg eq {}} {
 		error_popup [mc "Please supply a commit message.

-- 
2.49.0.212.gc22db56b11

Junio C Hamano· May 14, 2025, 21:14 UTC · re: Johannes Sixt · lore

Re: [PATCH] git-gui: do not end the commit message with an empty line

Johannes Sixt <j6t@kdbg.org> writes:

Show 10 quoted lines

> The commit message is processed to remove unnecessary empty lines.
> In particular, it is ensured that the text ends with at most one LF
> character. This one is always present, because the Tk text widget
> ensures that is present.
>
> However, we forgot that the processed text is written to the commit
> message file using 'puts', which also appends a LF character, so that
> the final commit message ends with two LF. Trim all trailing LF
> characters, and while we are here, use `string trim`, which lets us
> remove the leading LF in the same command.

The above reasoning look sensible.

As git-gui uses plumbing commit-tree to create the commit object, it has to be more careful not to have extra blank lines, as it cannot assume stripspace internal to "git commit"?

Show 6 quoted lines

>
> Reported-by: Gareth Fenn <garethfenn@gmail.com>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  lib/commit.tcl | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Show 18 quoted lines

> diff --git a/lib/commit.tcl b/lib/commit.tcl
> index a570f9cdc6a4..f3c714e600ac 100644
> --- a/lib/commit.tcl
> +++ b/lib/commit.tcl
> @@ -214,12 +214,10 @@ You must stage at least 1 file before you can commit.
>  	global comment_string
>  	set cmt_rx [strcat {(^|\n)} [regsub -all {\W} $comment_string {\\&}] {[^\n]*}]
>  	regsub -all $cmt_rx $msg {\1} msg
> -	# Strip leading empty lines
> -	regsub {^\n*} $msg {} msg
> +	# Strip leading and trailing empty lines
> +	set msg [string trim $msg \n]
>  	# Compress consecutive empty lines
>  	regsub -all {\n{3,}} $msg "\n\n" msg
> -	# Strip trailing empty line
> -	regsub {\n\n$} $msg "\n" msg
>  	if {$msg eq {}} {
>  		error_popup [mc "Please supply a commit message.

Johannes Sixt· May 15, 2025, 05:36 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-gui: do not end the commit message with an empty line

Am 14.05.25 um 23:14 schrieb Junio C Hamano:

> As git-gui uses plumbing commit-tree to create the
> commit object, it has to be more careful not to have extra blank
> lines, as it cannot assume stripspace internal to "git commit"?

Correct. We had used git stripspace for about a month, introduced by b9a4386, reverted by c0698df. The reasons not to use it were old Tcl versions and MacOS. We may revisit this case after we raise version requirements.

-- Hannes

Oswald Buddenhagen· May 15, 2025, 09:20 UTC · re: Johannes Sixt · lore

Re: [PATCH] git-gui: do not end the commit message with an empty line

On Wed, May 14, 2025 at 10:50:05PM +0200, Johannes Sixt wrote:

Show 6 quoted lines

>The commit message is processed to remove unnecessary empty lines.
>In particular, it is ensured that the text ends with at most one LF
>character. This one is always present, because the Tk text widget
>ensures that is present.
>
>However, we forgot

"did not consider" would be more accurate.

>that the processed text is written to the commit
>message file using 'puts', which also appends a LF character, so that
>the final commit message ends with two LF.

one could suppress that with -nonewline, but the proposed code is shorter.

Show 6 quoted lines

>Trim all trailing LF
>characters, and while we are here, use `string trim`, which lets us
>remove the leading LF in the same command.
>
>Reported-by: Gareth Fenn <garethfenn@gmail.com>
>Signed-off-by: Johannes Sixt <j6t@kdbg.org>

Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>

Show 16 quoted lines

>---
> lib/commit.tcl | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
>diff --git a/lib/commit.tcl b/lib/commit.tcl
>index a570f9cdc6a4..f3c714e600ac 100644
>--- a/lib/commit.tcl
>+++ b/lib/commit.tcl
>@@ -214,12 +214,10 @@ You must stage at least 1 file before you can commit.
> 	global comment_string
> 	set cmt_rx [strcat {(^|\n)} [regsub -all {\W} $comment_string {\\&}] {[^\n]*}]
> 	regsub -all $cmt_rx $msg {\1} msg
>-	# Strip leading empty lines
>-	regsub {^\n*} $msg {} msg
>+	# Strip leading and trailing empty lines
>

pedantically, stripping the final LF doesn't strip a trailing empty line, but prevents one from being created - unless the commit message is completely empty, where that would fail. i guess this case is not all that important, so we can ignore it. however, it may make sense to add another comment like "puts will re-add a trailing newline".

Show 11 quoted lines

>+	set msg [string trim $msg \n]
> 	# Compress consecutive empty lines
> 	regsub -all {\n{3,}} $msg "\n\n" msg
>-	# Strip trailing empty line
>-	regsub {\n\n$} $msg "\n" msg
> 	if {$msg eq {}} {
> 		error_popup [mc "Please supply a commit message.
> 
>-- 
>2.49.0.212.gc22db56b11
>

Johannes Sixt· May 15, 2025, 18:49 UTC · re: Oswald Buddenhagen · lore

[PATCH v2] git-gui: do not end the commit message with an empty line

However, did not consider that the processed text is written to the commit message file using `puts`, which also appends a LF character, so that the final commit message ends with two LF. Trim all trailing LF characters, and while we are here, use `string trim`, which lets us remove the leading LF in the same command.

Reported-by: Gareth Fenn <garethfenn@gmail.com>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
Am 15.05.25 um 11:20 schrieb Oswald Buddenhagen:

> On Wed, May 14, 2025 at 10:50:05PM +0200, Johannes Sixt wrote:
>> However, we forgot
> 
> "did not consider" would be more accurate.

Fair enough.

>> +    # Strip leading and trailing empty lines
>>
> [...] however, it may make sense to add
> another comment like "puts will re-add a trailing newline".

I did that.

>> +    set msg [string trim $msg \n]

 lib/commit.tcl | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Show changes to lib/commit.tcl +2 −4

diff --git a/lib/commit.tcl b/lib/commit.tcl
index a570f9cdc6a4..0c2be6f619cb 100644
--- a/lib/commit.tcl
+++ b/lib/commit.tcl
@@ -214,12 +214,10 @@ You must stage at least 1 file before you can commit.
 	global comment_string
 	set cmt_rx [strcat {(^|\n)} [regsub -all {\W} $comment_string {\\&}] {[^\n]*}]
 	regsub -all $cmt_rx $msg {\1} msg
-	# Strip leading empty lines
-	regsub {^\n*} $msg {} msg
+	# Strip leading and trailing empty lines (puts adds one \n)
+	set msg [string trim $msg \n]
 	# Compress consecutive empty lines
 	regsub -all {\n{3,}} $msg "\n\n" msg
-	# Strip trailing empty line
-	regsub {\n\n$} $msg "\n" msg
 	if {$msg eq {}} {
 		error_popup [mc "Please supply a commit message.

-- 
2.49.0.212.gc22db56b11