git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Mark Levedahl <mlevedahl@gmail.com>
Cc: Johannes Sixt <j6t@kdbg.org>, git@vger.kernel.org, me@yadavpratyush.com
Subject: Re: [PATCH] git-gui - simplify _open_stdout_stderr
Date: Tue, 1 Apr 2025 18:04:46 +0200 (CEST)	[thread overview]
Message-ID: <0140dfd8-f65d-34a0-2e66-44e3f696b100@gmx.de> (raw)
In-Reply-To: <20230922164412.130504-1-mlevedahl@gmail.com>

Hi Mark,

this patch is still relevant, right? I am Cc:ing Johannes Sixt for
awareness.

Ciao,
Johannes

On Fri, 22 Sep 2023, Mark Levedahl wrote:

> From: "Mark Levedahl Date: Tue, 19 Sep 2023" <mlevedahl@gmail.com>
>
> Since b792230 ("git-gui: Show a progress meter for checking out files",
> 2007-07-08), git-gui includes a workaround for Tcl that does not support
> using 2>@1 to redirect stderr to stdout. Tcl added such support in
> 8.4.7, released in 2004, while the later 8.4.14 still predated git-gui.
> But, Cygwin was stuck on an 8.4.1 Tcl variant until 2011, hence the need
> for this workaround. Commit 7145c65 recently removed much other specific
> code for that obsolete Cygwin Tcl/Tk, but missed this piece.
>
> Also, Tcl since 8.5 explicitly supports 2>@1 across all platforms, and
> git-gui requires Tcl >= 8.5, further evidence the workaround is
> obsolete.  (I did test that 2>@1 works as-expected on current Linux,
> Cygwin, and Git For Windows Tcl packages).
>
> Remove the workaround and exploit concat's documented capability to
> handle both scalar and list arguments, leaving a much simpler function.
> This eliminates any question that cmd might be executed twice.
>
> Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
> ---
>  git-gui.sh | 21 ++-------------------
>  1 file changed, 2 insertions(+), 19 deletions(-)
>
> diff --git a/git-gui.sh b/git-gui.sh
> index 8bc8892..a5d008d 100755
> --- a/git-gui.sh
> +++ b/git-gui.sh
> @@ -583,25 +583,8 @@ proc git {args} {
>
>  proc _open_stdout_stderr {cmd} {
>  	_trace_exec $cmd
> -	if {[catch {
> -			set fd [open [concat [list | ] $cmd] r]
> -		} err]} {
> -		if {   [lindex $cmd end] eq {2>@1}
> -		    && $err eq {can not find channel named "1"}
> -			} {
> -			# Older versions of Tcl 8.4 don't have this 2>@1 IO
> -			# redirect operator.  Fallback to |& cat for those.
> -			# The command was not actually started, so its safe
> -			# to try to start it a second time.
> -			#
> -			set fd [open [concat \
> -				[list | ] \
> -				[lrange $cmd 0 end-1] \
> -				[list |& cat] \
> -				] r]
> -		} else {
> -			error $err
> -		}
> +	if {[catch {set fd [open [concat | $cmd] r]} err]} {
> +		error $err
>  	}
>  	fconfigure $fd -eofchar {}
>  	return $fd
> --
> 2.41.0.99.19
>
>

  reply	other threads:[~2025-04-01 16:04 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-09-22 16:44 [PATCH] git-gui - simplify _open_stdout_stderr Mark Levedahl
2025-04-01 16:04 ` Johannes Schindelin [this message]
2025-04-01 16:48   ` Mark Levedahl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0140dfd8-f65d-34a0-2e66-44e3f696b100@gmx.de \
    --to=johannes.schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=me@yadavpratyush.com \
    --cc=mlevedahl@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).