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

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 29, 2013, 16:15 UTC
Message-ID: <7vsj5krmng.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130129120923.GE7498@serenity.lan>

John Keeping <john@keeping.me.uk> writes:

Show 19 quoted lines

> 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.