# [PATCH 2/2] git-gui: strip commit messages less aggressively 4 messages from 2024-08-13 to 2024-08-15. Participants: Oswald Buddenhagen, Johannes Sixt. Thread: https://gitlist.dev/t/61942 ## Oswald Buddenhagen, 2024-08-13 09:06 Subject: [PATCH 2/2] git-gui: strip commit messages less aggressively Message-ID: <20240813090631.1133049-3-oswald.buddenhagen@gmx.de> URL: https://gitlist.dev/e/20240813090631.1133049-3-oswald.buddenhagen%40gmx.de In-Reply-To: <20240813090631.1133049-1-oswald.buddenhagen@gmx.de> ``` We would strip all leading and trailing whitespace, which git commit does not. Let's be consistent here. Signed-off-by: Oswald Buddenhagen --- Cc: Johannes Sixt Cc: Brian Lyles Cc: Junio C Hamano Cc: Eric Sunshine Cc: Sean Allred --- git-gui/lib/commit.tcl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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, 2024-08-13 09:06 Subject: [PATCH 0/2] Re: [BUG REPORT] git-gui invokes prepare-commit-msg hook incorrectly Message-ID: <20240813090631.1133049-1-oswald.buddenhagen@gmx.de> URL: https://gitlist.dev/e/20240813090631.1133049-1-oswald.buddenhagen%40gmx.de ``` > >> 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 Cc: Brian Lyles Cc: Junio C Hamano Cc: Eric Sunshine Cc: Sean Allred 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, 2024-08-13 09:06 Subject: [PATCH 1/2] git-gui: strip comments and consecutive empty lines from commit messages Message-ID: <20240813090631.1133049-2-oswald.buddenhagen@gmx.de> URL: https://gitlist.dev/e/20240813090631.1133049-2-oswald.buddenhagen%40gmx.de In-Reply-To: <20240813090631.1133049-1-oswald.buddenhagen@gmx.de> ``` 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 --- Cc: Johannes Sixt Cc: Brian Lyles Cc: Junio C Hamano Cc: Eric Sunshine Cc: Sean Allred --- 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. -- 2.46.0.180.gb23db42a00 ``` ## Johannes Sixt, 2024-08-15 14:36 Subject: Re: [PATCH 1/2] git-gui: strip comments and consecutive empty lines from commit messages Message-ID: <152246bb-0f38-491d-80b9-ed029010f29f@kdbg.org> URL: https://gitlist.dev/e/152246bb-0f38-491d-80b9-ed029010f29f%40kdbg.org In-Reply-To: <20240813090631.1133049-2-oswald.buddenhagen@gmx.de> ``` 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 > > --- > > Cc: Johannes Sixt > Cc: Brian Lyles > Cc: Junio C Hamano > Cc: Eric Sunshine > Cc: Sean Allred > --- > 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 ```