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

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

[Johannes Sixt]

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(-)

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

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

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> 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"?


>
> 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(-)

> 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.

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

[Johannes Sixt]

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

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

[Oswald Buddenhagen]

On Wed, May 14, 2025 at 10:50:05PM +0200, Johannes Sixt wrote:
>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.

>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>

>---
> 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".

>+	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
>

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

[Johannes Sixt]

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, 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(-)

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