threads / patch / 61942

patch, 2 partsgit-gui: strip commit messages less aggressively

Subject: [PATCH 2/2] git-gui: strip commit messages less aggressively

## tl;dr

4 messages between Aug 13, 2024 and Aug 15, 2024. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Oswald Buddenhagen· Aug 13, 2024, 09:06 UTC · lore

[PATCH 0/2] Re: [BUG REPORT] git-gui invokes prepare-commit-msg hook incorrectly

Show 12 quoted lines

> >> So it still seems like we have two real options:
> >>
> >> - Start washing the message, allowing the prepare-commit-msg hook to
> >>   provide template-like guidance to the user regardless of if they are
> >>   using git-gui or some other editor, or
> >> - Pass the "message" argument along to the prepare-commit-msg hook so
> >>   that it can at least avoid adding template-like content (but of course
> >>   then lose the value added by that template).
> >
> i'm strongly in favor of the first option.
> it also seems to be the much easier one to implement.
>

so i thought i'd just give it a shot ...

fwiw, it's debatable whether it (stil) makes sense that git-gui reimplements git-commit - maybe it should just call it. then the patch would boil down to adding --cleanup=strip to the command line.

---

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>

Oswald Buddenhagen (2):
  git-gui: strip comments and consecutive empty lines from commit
    messages
  git-gui: strip commit messages less aggressively

 git-gui/lib/commit.tcl | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

-- 
2.46.0.180.gb23db42a00

Oswald Buddenhagen· Aug 13, 2024, 09:06 UTC · re: Oswald Buddenhagen · lore

We would strip all leading and trailing whitespace, which git commit does not. Let's be consistent here.

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 | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Show changes to git-gui/lib/commit.tcl +6 −1

diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl
index f00a634624..208dc2817c 100644
--- a/git-gui/lib/commit.tcl
+++ b/git-gui/lib/commit.tcl
@@ -207,12 +207,17 @@ You must stage at least 1 file before you can commit.
 
 	# -- A message is required.
 	#
-	set msg [string trim [$ui_comm get 1.0 end]]
+	set msg [$ui_comm get 1.0 end]
+	# Strip trailing whitespace
 	regsub -all -line {[ \t\r]+$} $msg {} msg
 	# Strip comment lines
 	regsub -all {(^|\n)#[^\n]*} $msg {\1} msg
+	# Strip leading empty lines
+	regsub {^\n*} $msg {} msg
 	# 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.46.0.180.gb23db42a00

Oswald Buddenhagen· Aug 13, 2024, 09:06 UTC · re: Oswald Buddenhagen · lore

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

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

Show changes to git-gui/lib/commit.tcl +4 −0

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.

-- 
2.46.0.180.gb23db42a00

Johannes Sixt· Aug 15, 2024, 14:36 UTC · re: Oswald Buddenhagen · lore

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

Am 13.08.24 um 11:06 schrieb Oswald Buddenhagen:

Show 35 quoted lines

> 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