git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
To: Johannes Sixt <j6t@kdbg.org>
Cc: Gareth Fenn <garethfenn@gmail.com>,
	Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] git-gui: do not end the commit message with an empty line
Date: Thu, 15 May 2025 11:20:39 +0200	[thread overview]
Message-ID: <aCWx56e02RqAUZgw@ugly> (raw)
In-Reply-To: <ed1ca9fa-15f0-4601-be31-8a578c7fb788@kdbg.org>

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
>

  parent reply	other threads:[~2025-05-15  9:20 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-14 20:50 [PATCH] git-gui: do not end the commit message with an empty line Johannes Sixt
2025-05-14 21:14 ` Junio C Hamano
2025-05-15  5:36   ` Johannes Sixt
2025-05-15  9:20 ` Oswald Buddenhagen [this message]
2025-05-15 18:49   ` [PATCH v2] " Johannes Sixt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aCWx56e02RqAUZgw@ugly \
    --to=oswald.buddenhagen@gmx.de \
    --cc=garethfenn@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).