{"thread":{"id":"61942","subject":"[PATCH 2/2] git-gui: strip commit messages less aggressively","startedAt":"2024-08-13T09:06:37Z","lastAt":"2024-08-15T14:37:15Z","messageCount":4,"participants":["Oswald Buddenhagen","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"500702","messageId":"20240813090631.1133049-3-oswald.buddenhagen@gmx.de","threadId":"61942","inReplyTo":"20240813090631.1133049-1-oswald.buddenhagen@gmx.de","subject":"[PATCH 2/2] git-gui: strip commit messages less aggressively","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2024-08-13T09:06:31Z","receivedAt":"2024-08-13T09:06:37Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"We would strip all leading and trailing whitespace, which git commit\ndoes not. Let's be consistent here.\n\nSigned-off-by: Oswald Buddenhagen \n\n---\n\nCc: Johannes Sixt \nCc: Brian Lyles \nCc: Junio C Hamano \nCc: Eric Sunshine \nCc: Sean Allred \n---\n git-gui/lib/commit.tcl | 7 ++++++-\n 1 file changed, 6 insertions(+), 1 deletion(-)\n\ndiff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl\nindex f00a634624..208dc2817c 100644\n--- a/git-gui/lib/commit.tcl\n+++ b/git-gui/lib/commit.tcl\n@@ -207,12 +207,17 @@ You must stage at least 1 file before you can commit.\n \n \t# -- A message is required.\n \t#\n-\tset msg [string trim [$ui_comm get 1.0 end]]\n+\tset msg [$ui_comm get 1.0 end]\n+\t# Strip trailing whitespace\n \tregsub -all -line {[ \\t\\r]+$} $msg {} msg\n \t# Strip comment lines\n \tregsub -all {(^|\\n)#[^\\n]*} $msg {\\1} msg\n+\t# Strip leading empty lines\n+\tregsub {^\\n*} $msg {} msg\n \t# Compress consecutive empty lines\n \tregsub -all {\\n{3,}} $msg \"\\n\\n\" msg\n+\t# Strip trailing empty line\n+\tregsub {\\n\\n$} $msg \"\\n\" msg\n \tif {$msg eq {}} {\n \t\terror_popup [mc \"Please supply a commit message.\n \n-- \n2.46.0.180.gb23db42a00\n\n"},{"id":"500703","messageId":"20240813090631.1133049-1-oswald.buddenhagen@gmx.de","threadId":"61942","inReplyTo":null,"subject":"[PATCH 0/2] Re: [BUG REPORT] git-gui invokes prepare-commit-msg hook incorrectly","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2024-08-13T09:06:29Z","receivedAt":"2024-08-13T09:06:37Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"> >> So it still seems like we have two real options:\n> >>\n> >> - Start washing the message, allowing the prepare-commit-msg hook to\n> >> provide template-like guidance to the user regardless of if they are\n> >> using git-gui or some other editor, or\n> >> - Pass the \"message\" argument along to the prepare-commit-msg hook so\n> >> that it can at least avoid adding template-like content (but of course\n> >> then lose the value added by that template).\n> >\n> i'm strongly in favor of the first option.\n> it also seems to be the much easier one to implement.\n>\nso i thought i'd just give it a shot ...\n\nfwiw, it's debatable whether it (stil) makes sense that git-gui reimplements\ngit-commit - maybe it should just call it. then the patch would boil down to\nadding --cleanup=strip to the command line.\n\n---\n\nCc: Johannes Sixt \nCc: Brian Lyles \nCc: Junio C Hamano \nCc: Eric Sunshine \nCc: Sean Allred \n\nOswald Buddenhagen (2):\n git-gui: strip comments and consecutive empty lines from commit\n messages\n git-gui: strip commit messages less aggressively\n\n git-gui/lib/commit.tcl | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\n-- \n2.46.0.180.gb23db42a00\n\n"},{"id":"500704","messageId":"20240813090631.1133049-2-oswald.buddenhagen@gmx.de","threadId":"61942","inReplyTo":"20240813090631.1133049-1-oswald.buddenhagen@gmx.de","subject":"[PATCH 1/2] git-gui: strip comments and consecutive empty lines from commit messages","fromName":"Oswald Buddenhagen","fromEmail":"oswald.buddenhagen@gmx.de","sentAt":"2024-08-13T09:06:30Z","receivedAt":"2024-08-13T09:06:39Z","isPatch":true,"sender":{"key":"oswald.buddenhagen@gmx.de","avatar":"https://avatars.githubusercontent.com/u/812380?v=4"},"body":"This is also known as \"washing\". This is consistent with the behavior of\ninteractive git commit, which we should emulate as closely as possible\nto avoid usability problems. This way commit message templates and\nprepare hooks can be used properly, and comments from conflicted rebases\nand merges are cleaned up without having to introduce special handling\nfor them.\n\nSigned-off-by: Oswald Buddenhagen \n\n---\n\nCc: Johannes Sixt \nCc: Brian Lyles \nCc: Junio C Hamano \nCc: Eric Sunshine \nCc: Sean Allred \n---\n git-gui/lib/commit.tcl | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl\nindex 11379f8ad3..f00a634624 100644\n--- a/git-gui/lib/commit.tcl\n+++ b/git-gui/lib/commit.tcl\n@@ -209,6 +209,10 @@ You must stage at least 1 file before you can commit.\n \t#\n \tset msg [string trim [$ui_comm get 1.0 end]]\n \tregsub -all -line {[ \\t\\r]+$} $msg {} msg\n+\t# Strip comment lines\n+\tregsub -all {(^|\\n)#[^\\n]*} $msg {\\1} msg\n+\t# Compress consecutive empty lines\n+\tregsub -all {\\n{3,}} $msg \"\\n\\n\" msg\n \tif {$msg eq {}} {\n \t\terror_popup [mc \"Please supply a commit message.\n \n-- \n2.46.0.180.gb23db42a00\n\n"},{"id":"501011","messageId":"152246bb-0f38-491d-80b9-ed029010f29f@kdbg.org","threadId":"61942","inReplyTo":"20240813090631.1133049-2-oswald.buddenhagen@gmx.de","subject":"Re: [PATCH 1/2] git-gui: strip comments and consecutive empty lines from commit messages","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2024-08-15T14:36:57Z","receivedAt":"2024-08-15T14:37:15Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 13.08.24 um 11:06 schrieb Oswald Buddenhagen:\n> This is also known as \"washing\". This is consistent with the behavior of\n> interactive git commit, which we should emulate as closely as possible\n> to avoid usability problems. This way commit message templates and\n> prepare hooks can be used properly, and comments from conflicted rebases\n> and merges are cleaned up without having to introduce special handling\n> for them.\n> \n> Signed-off-by: Oswald Buddenhagen \n> \n> ---\n> \n> Cc: Johannes Sixt \n> Cc: Brian Lyles \n> Cc: Junio C Hamano \n> Cc: Eric Sunshine \n> Cc: Sean Allred \n> ---\n> git-gui/lib/commit.tcl | 4 ++++\n> 1 file changed, 4 insertions(+)\n> \n> diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl\n> index 11379f8ad3..f00a634624 100644\n> --- a/git-gui/lib/commit.tcl\n> +++ b/git-gui/lib/commit.tcl\n> @@ -209,6 +209,10 @@ You must stage at least 1 file before you can commit.\n> \t#\n> \tset msg [string trim [$ui_comm get 1.0 end]]\n> \tregsub -all -line {[ \\t\\r]+$} $msg {} msg\n> +\t# Strip comment lines\n> +\tregsub -all {(^|\\n)#[^\\n]*} $msg {\\1} msg\n> +\t# Compress consecutive empty lines\n> +\tregsub -all {\\n{3,}} $msg \"\\n\\n\" msg\n> \tif {$msg eq {}} {\n> \t\terror_popup [mc \"Please supply a commit message.\n> \n\nI'm still not convinced that it is appropriate to silently edit a\nmessage entered by the user.\n\nNevertheless, I will pick up this series for these reasons:\n\nFirstly, during an interactive rebase Git GUI has no way to inform the\nuser that the commit message is a union of messages of squash- and\nfixup-commits except by presenting the comments that git-rebase inserted\nin the commit message. Stripping these comments before the user can see\nthem would be a disservice. Consequently, they should be helped in the\nway that this patch does.\n\nSecondly, there is already precedent in b9a43869c9f9 (\"git-gui: remove\nlines starting with the comment character\", 2021-02-03), which invoked\ngit-stripspace. This commit was reverted later by c0698df0579a for\ntechnical reasons (incompatibilities with macOS). So it seems that a\nmajority thinks this is the right way to go forward.\n\nFurther work would be welcome, though. The comment character can be\nconfigured and should not be hard-coded. The commit cited above shows\nhow it can be implemented.\n\nThree more instances of `$ui_comm get` exist, which trim whitespace in\nvarious ways. Each should be inspected whether it still does the right\nthing.\n\nAll of these can be follow-up patchs, of course.\n\n-- Hannes\n\n"}]}