Subject: Re: [PATCH v2 0/4] Auto-generate mergetool lists

From: Junio C Hamano <[email-masked]>
Date: Tue, 29 Jan 2013 08:15:15 -0800

John Keeping <[email-masked]> writes:

> On Tue, Jan 29, 2013 at 12:56:58PM +0100, Joachim Schmitz wrote:
>> John Keeping wrote:
>> > Currently I'm extracting the command word using:
>> >
>> >    cmd=$(eval -- "set -- $(git config mergetool.$tool.cmd); echo
>> > \"$1\"")
>> 
>> Shouldnt this work?
>> cmd=$((git config "mergetool.$tool.cmd" || git config "difftool.$tool.cmd") 
>> | awk '{print $1}')
>
> That doesn't handle paths with spaces in, whereas the eval in a subshell
> does:
>
>     $ cmd='"my command" $BASE $LOCAL $REMOTE'
>     $ echo "$cmd" | awk '{print $1}'
>     "my
>     $ ( eval -- "set -- $cmd; echo \"\$1\"" )
>     my command

I'd rather not to see you do any of the above.

With any backend that is non-trivial, it would not be unusual for
the *tool.cmd to look like:

     [mergetool]
     	mytool = sh -c '
        	... some massaging to prepare the command line
                ... to run the real tool backend comes here, and
     		... then ...
                my_real_tool $arg1 $arg2 ...
	'

and you will end up detecting the presence of the shell, which is
not very useful.

I think it is perfectly fine to say "you configured it, so it must
exist; it may fail when we try to run it but it is your problem".
It is simpler to explain and requires one less eval.