Re: [PATCH] git-gui: Use shell to launch textconv filter in "blame"

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Pat Thoyts <patthoyts@users.sourceforge.net> writes:

>> 	if {$commit eq {}} {
>> 		if {$do_textconv ne 0} {
>>-			set fd [open |[list $textconv $path] r]
>>+			# Run textconv with sh -c "..." to allow it to
>>+			# contain command + arguments.
>>+			set fd [open |[list [shellpath] -c "$textconv \"\$0\"" $path] r]
>> 		} else {
>> 			set fd [open $path r]
>> 		}
>
> I don't believe we need to put all this in to launch this via the
> shell. We just have to pass a list where the first element is the
> command-name.
>
> The following works for me using your 'textconv = odf2txt --width=40'
> test and also a 'textconv = od -t x1' that I tried for a hex dump
> output. I couldn't make run-mailcap do anything useful for me.
>
> diff --git a/lib/blame.tcl b/lib/blame.tcl
> index 2137ec9..c06ef04 100644
> --- a/lib/blame.tcl
> +++ b/lib/blame.tcl
> @@ -460,7 +460,7 @@ method _load {jump} {
>         }
>         if {$commit eq {}} {
>                 if {$do_textconv ne 0} {
> -                       set fd [open |[list $textconv $path] r]
> +                       set fd [open |[linsert $textconv end $path] r]
>                 } else {
>                         set fd [open $path r]
>                 }

I'm not very fluent in Tcl, but I don't think this runs the command
through a shell (pstree agrees with me). That will work in most cases,
so that may be acceptable, but if you want to have full compatibility
with what "git blame" does (by using a shell) and allow e.g.

textconv = LANG=C some-command

or

textconv = cd ../; do-whatever

which are already managed by "git blame" and are OK with my version,
it's not going to do it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/