{"thread":{"id":"55233","subject":"[PATCH] git-gui: fix commit message comment line removal with older Tcl versions","startedAt":"2021-02-28T23:12:58Z","lastAt":"2021-03-01T06:49:46Z","messageCount":2,"participants":["Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"418001","messageId":"20210228231110.24076-1-sunshine@sunshineco.com","threadId":"55233","inReplyTo":"https://lore.kernel.org/git/CAPig+cT-sfgMDi9-6AEKF85NtOiXeqddJjk-pYuhDtTVAE-UEw@mail.gmail.com/","subject":"[PATCH] git-gui: fix commit message comment line removal with older Tcl versions","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-02-28T23:11:10Z","receivedAt":"2021-02-28T23:12:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"git-gui was recently enhanced to remove comment lines from the commit\nmessage similar to the way git-commit does so (see `core.commentchar`\nand `git-stripspace`). Unfortunately, that change employs features which\nare unavailable in older versions of Tcl, such as 8.5.9 which is shipped\nwith macOS (10.13), with the result that the commit operation errors\nout.\n\nThere are two problems. First, to add a new informational message to the\nmain window, it invokes string method `cat` which does not exist in\nolder Tcl. Fix this by using `append` instead.\n\nSecond, when passing the commit message through git-stripspace, it\ncloses the \"write\" side of the bidirectional pipe after sending the\ncommit message to git-stripspace in order to avoid deadlock before\nreading back the result, however the ability to close only one end of a\npipe is not present in older Tcl. Fix this by employing a temporary file\nto received the output of git-stripspace.\n\nSigned-off-by: Eric Sunshine <sunshine@sunshineco.com>\n---\n\nI'm not a Tcl programmer, so I may have overlooked better or more\nidiomatic ways to fix these problems. There might be some reliable and\nportable way to write and read a bidirectional pipe in older Tcl\nwithout deadlocking and without using non-blocking I/O and an\nevent-loop, but I didn't find it, so I went with the simpler approach\nof using a temporary file.\n\n git-gui.sh     |  3 +--\n lib/commit.tcl | 12 +++++++-----\n 2 files changed, 8 insertions(+), 7 deletions(-)\n\ndiff --git a/git-gui.sh b/git-gui.sh\nindex 236bc4e61d..c04b37b9ee 100755\n--- a/git-gui.sh\n+++ b/git-gui.sh\n@@ -3439,8 +3439,7 @@ proc trace_commit_type {varname args} {\n \t}\n \n \tset comment_char [get_config core.commentchar]\n-\tset txt [string cat $txt \\\n-\t\t\t\t [mc \" (Lines starting with '$comment_char' will be ignored)\"]]\n+\tappend txt [mc \" (Lines starting with '$comment_char' will be ignored)\"]\n \t$ui_coml conf -text $txt\n }\n trace add variable commit_type write trace_commit_type\ndiff --git a/lib/commit.tcl b/lib/commit.tcl\nindex 23d67d4651..3a51e80b8a 100644\n--- a/lib/commit.tcl\n+++ b/lib/commit.tcl\n@@ -142,16 +142,18 @@ proc setup_commit_encoding {msg_wt {quiet 0}} {\n }\n \n proc strip_msg {msg} {\n-\tset cmd [concat [list | ] [_git_cmd stripspace] --strip-comments]\n+\tset strip_p [gitdir GITGUI_EDITMSG_STRIP]\n+\tset cmd [concat [list | ] [_git_cmd stripspace] --strip-comments [list >$strip_p]]\n \t_trace_exec $cmd\n-\tset fd [open $cmd r+]\n+\tset fd [open $cmd w]\n \tfconfigure $fd -translation binary -encoding utf-8\n-\n \tputs -nonewline $fd $msg\n-\tclose $fd w\n-\tset result [read $fd]\n \tclose $fd\n \n+\tset fd [open $strip_p r]\n+\tset result [read $fd]\n+\tclose $fd\n+\tfile delete $strip_p\n \treturn $result\n }\n \n-- \n2.31.0.rc0.1.g37593106bf\n"},{"id":"418002","messageId":"CAPig+cRQN4PjfxEOZ8ZBA_uttsRPS8DPDgToM_JFvichDDh_HQ@mail.gmail.com","threadId":"55233","inReplyTo":"20210228231110.24076-1-sunshine@sunshineco.com","subject":"Re: [PATCH] git-gui: fix commit message comment line removal with older Tcl versions","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-03-01T06:48:49Z","receivedAt":"2021-03-01T06:49:46Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Feb 28, 2021 at 6:12 PM Eric Sunshine <sunshine@sunshineco.com> wrote:\n> git-gui was recently enhanced to remove comment lines from the commit\n> message similar to the way git-commit does so (see `core.commentchar`\n> and `git-stripspace`). Unfortunately, that change employs features which\n> are unavailable in older versions of Tcl, such as 8.5.9 which is shipped\n> with macOS (10.13), with the result that the commit operation errors\n> out.\n>\n> There are two problems. First, to add a new informational message to the\n> main window, it invokes string method `cat` which does not exist in\n> older Tcl. Fix this by using `append` instead.\n>\n> Second, when passing the commit message through git-stripspace, it\n> closes the \"write\" side of the bidirectional pipe after sending the\n> commit message to git-stripspace in order to avoid deadlock before\n> reading back the result, however the ability to close only one end of a\n> pipe is not present in older Tcl. Fix this by employing a temporary file\n> to received the output of git-stripspace.\n\ns/received/receive/\n\n(I noticed this immediately after sending the patch, of course.)\n\n> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>\n> ---\n> diff --git a/lib/commit.tcl b/lib/commit.tcl\n> @@ -142,16 +142,18 @@ proc setup_commit_encoding {msg_wt {quiet 0}} {\n>  proc strip_msg {msg} {\n> -       set cmd [concat [list | ] [_git_cmd stripspace] --strip-comments]\n> +       set strip_p [gitdir GITGUI_EDITMSG_STRIP]\n> +       set cmd [concat [list | ] [_git_cmd stripspace] --strip-comments [list >$strip_p]]\n>         _trace_exec $cmd\n> -       set fd [open $cmd r+]\n> +       set fd [open $cmd w]\n>         fconfigure $fd -translation binary -encoding utf-8\n> -\n>         puts -nonewline $fd $msg\n> -       close $fd w\n> -       set result [read $fd]\n>         close $fd\n>\n> +       set fd [open $strip_p r]\n\nI had meant to insert:\n\n    fconfigure $fd -translation binary -encoding utf-8\n\nhere but forgot. Would you like me to resend the patch or can you\ntweak it locally?\n\n> +       set result [read $fd]\n> +       close $fd\n> +       file delete $strip_p\n>         return $result\n>  }\n"}]}