threads / patch / 55233

patchgit-gui: fix commit message comment line removal with older Tcl versions

Subject: [PATCH] git-gui: fix commit message comment line removal with older Tcl versions

## tl;dr

2 messages between Feb 28, 2021 and Mar 1, 2021. Diffs are folded; open one to read it.

replies: 1people: 1as markdown or json

Eric Sunshine· Feb 28, 2021, 23:11 UTC · lore

git-gui was recently enhanced to remove comment lines from the commit message similar to the way git-commit does so (see `core.commentchar` and `git-stripspace`). Unfortunately, that change employs features which are unavailable in older versions of Tcl, such as 8.5.9 which is shipped with macOS (10.13), with the result that the commit operation errors out.

There are two problems. First, to add a new informational message to the main window, it invokes string method `cat` which does not exist in older Tcl. Fix this by using `append` instead.

Second, when passing the commit message through git-stripspace, it closes the "write" side of the bidirectional pipe after sending the commit message to git-stripspace in order to avoid deadlock before reading back the result, however the ability to close only one end of a pipe is not present in older Tcl. Fix this by employing a temporary file to received the output of git-stripspace.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---

I'm not a Tcl programmer, so I may have overlooked better or more idiomatic ways to fix these problems. There might be some reliable and portable way to write and read a bidirectional pipe in older Tcl without deadlocking and without using non-blocking I/O and an event-loop, but I didn't find it, so I went with the simpler approach of using a temporary file.

 git-gui.sh     |  3 +--
 lib/commit.tcl | 12 +++++++-----
 2 files changed, 8 insertions(+), 7 deletions(-)
Show changes to 2 files +8 −7

git-gui.sh, lib/commit.tcl

diff --git a/git-gui.sh b/git-gui.sh
index 236bc4e61d..c04b37b9ee 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -3439,8 +3439,7 @@ proc trace_commit_type {varname args} {
 	}
 
 	set comment_char [get_config core.commentchar]
-	set txt [string cat $txt \
-				 [mc " (Lines starting with '$comment_char' will be ignored)"]]
+	append txt [mc " (Lines starting with '$comment_char' will be ignored)"]
 	$ui_coml conf -text $txt
 }
 trace add variable commit_type write trace_commit_type
diff --git a/lib/commit.tcl b/lib/commit.tcl
index 23d67d4651..3a51e80b8a 100644
--- a/lib/commit.tcl
+++ b/lib/commit.tcl
@@ -142,16 +142,18 @@ proc setup_commit_encoding {msg_wt {quiet 0}} {
 }
 
 proc strip_msg {msg} {
-	set cmd [concat [list | ] [_git_cmd stripspace] --strip-comments]
+	set strip_p [gitdir GITGUI_EDITMSG_STRIP]
+	set cmd [concat [list | ] [_git_cmd stripspace] --strip-comments [list >$strip_p]]
 	_trace_exec $cmd
-	set fd [open $cmd r+]
+	set fd [open $cmd w]
 	fconfigure $fd -translation binary -encoding utf-8
-
 	puts -nonewline $fd $msg
-	close $fd w
-	set result [read $fd]
 	close $fd
 
+	set fd [open $strip_p r]
+	set result [read $fd]
+	close $fd
+	file delete $strip_p
 	return $result
 }
 
-- 
2.31.0.rc0.1.g37593106bf
Eric Sunshine· Mar 1, 2021, 06:48 UTC · re: Eric Sunshine · lore

Re: [PATCH] git-gui: fix commit message comment line removal with older Tcl versions

On Sun, Feb 28, 2021 at 6:12 PM Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 17 quoted lines
> git-gui was recently enhanced to remove comment lines from the commit
> message similar to the way git-commit does so (see `core.commentchar`
> and `git-stripspace`). Unfortunately, that change employs features which
> are unavailable in older versions of Tcl, such as 8.5.9 which is shipped
> with macOS (10.13), with the result that the commit operation errors
> out.
>
> There are two problems. First, to add a new informational message to the
> main window, it invokes string method `cat` which does not exist in
> older Tcl. Fix this by using `append` instead.
>
> Second, when passing the commit message through git-stripspace, it
> closes the "write" side of the bidirectional pipe after sending the
> commit message to git-stripspace in order to avoid deadlock before
> reading back the result, however the ability to close only one end of a
> pipe is not present in older Tcl. Fix this by employing a temporary file
> to received the output of git-stripspace.
s/received/receive/
(I noticed this immediately after sending the patch, of course.)
Show 19 quoted lines
> Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
> ---
> diff --git a/lib/commit.tcl b/lib/commit.tcl
> @@ -142,16 +142,18 @@ proc setup_commit_encoding {msg_wt {quiet 0}} {
>  proc strip_msg {msg} {
> -       set cmd [concat [list | ] [_git_cmd stripspace] --strip-comments]
> +       set strip_p [gitdir GITGUI_EDITMSG_STRIP]
> +       set cmd [concat [list | ] [_git_cmd stripspace] --strip-comments [list >$strip_p]]
>         _trace_exec $cmd
> -       set fd [open $cmd r+]
> +       set fd [open $cmd w]
>         fconfigure $fd -translation binary -encoding utf-8
> -
>         puts -nonewline $fd $msg
> -       close $fd w
> -       set result [read $fd]
>         close $fd
>
> +       set fd [open $strip_p r]
I had meant to insert:
    fconfigure $fd -translation binary -encoding utf-8

here but forgot. Would you like me to resend the patch or can you tweak it locally?

Show 5 quoted lines
> +       set result [read $fd]
> +       close $fd
> +       file delete $strip_p
>         return $result
>  }

← back to recent threads