diff mbox series

[1/2] git-gui: strip comments and consecutive empty lines from commit messages

Message ID 20240813090631.1133049-2-oswald.buddenhagen@gmx.de (mailing list archive)
State New
Headers show
Series Re: [BUG REPORT] git-gui invokes prepare-commit-msg hook incorrectly | expand

Commit Message

Oswald Buddenhagen Aug. 13, 2024, 9:06 a.m. UTC
This is also known as "washing". This is consistent with the behavior of
interactive git commit, which we should emulate as closely as possible
to avoid usability problems. This way commit message templates and
prepare hooks can be used properly, and comments from conflicted rebases
and merges are cleaned up without having to introduce special handling
for them.

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

---

Cc: Johannes Sixt <j6t@kdbg.org>
Cc: Brian Lyles <brianmlyles@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>
Cc: Eric Sunshine <sunshine@sunshineco.com>
Cc: Sean Allred <allred.sean@gmail.com>
---
 git-gui/lib/commit.tcl | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Johannes Sixt Aug. 15, 2024, 2:36 p.m. UTC | #1
Am 13.08.24 um 11:06 schrieb Oswald Buddenhagen:
> This is also known as "washing". This is consistent with the behavior of
> interactive git commit, which we should emulate as closely as possible
> to avoid usability problems. This way commit message templates and
> prepare hooks can be used properly, and comments from conflicted rebases
> and merges are cleaned up without having to introduce special handling
> for them.
> 
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> 
> ---
> 
> Cc: Johannes Sixt <j6t@kdbg.org>
> Cc: Brian Lyles <brianmlyles@gmail.com>
> Cc: Junio C Hamano <gitster@pobox.com>
> Cc: Eric Sunshine <sunshine@sunshineco.com>
> Cc: Sean Allred <allred.sean@gmail.com>
> ---
>  git-gui/lib/commit.tcl | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl
> index 11379f8ad3..f00a634624 100644
> --- a/git-gui/lib/commit.tcl
> +++ b/git-gui/lib/commit.tcl
> @@ -209,6 +209,10 @@ You must stage at least 1 file before you can commit.
>  	#
>  	set msg [string trim [$ui_comm get 1.0 end]]
>  	regsub -all -line {[ \t\r]+$} $msg {} msg
> +	# Strip comment lines
> +	regsub -all {(^|\n)#[^\n]*} $msg {\1} msg
> +	# Compress consecutive empty lines
> +	regsub -all {\n{3,}} $msg "\n\n" msg
>  	if {$msg eq {}} {
>  		error_popup [mc "Please supply a commit message.
>  

I'm still not convinced that it is appropriate to silently edit a
message entered by the user.

Nevertheless, I will pick up this series for these reasons:

Firstly, during an interactive rebase Git GUI has no way to inform the
user that the commit message is a union of messages of squash- and
fixup-commits except by presenting the comments that git-rebase inserted
in the commit message. Stripping these comments before the user can see
them would be a disservice. Consequently, they should be helped in the
way that this patch does.

Secondly, there is already precedent in b9a43869c9f9 ("git-gui: remove
lines starting with the comment character", 2021-02-03), which invoked
git-stripspace. This commit was reverted later by c0698df0579a for
technical reasons (incompatibilities with macOS). So it seems that a
majority thinks this is the right way to go forward.

Further work would be welcome, though. The comment character can be
configured and should not be hard-coded. The commit cited above shows
how it can be implemented.

Three more instances of `$ui_comm get` exist, which trim whitespace in
various ways. Each should be inspected whether it still does the right
thing.

All of these can be follow-up patchs, of course.

-- Hannes
diff mbox series

Patch

diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl
index 11379f8ad3..f00a634624 100644
--- a/git-gui/lib/commit.tcl
+++ b/git-gui/lib/commit.tcl
@@ -209,6 +209,10 @@  You must stage at least 1 file before you can commit.
 	#
 	set msg [string trim [$ui_comm get 1.0 end]]
 	regsub -all -line {[ \t\r]+$} $msg {} msg
+	# Strip comment lines
+	regsub -all {(^|\n)#[^\n]*} $msg {\1} msg
+	# Compress consecutive empty lines
+	regsub -all {\n{3,}} $msg "\n\n" msg
 	if {$msg eq {}} {
 		error_popup [mc "Please supply a commit message.