Re: [PATCH] documentation: git-mergetool updated to warn against builtin tools invocations

From: Sylvain Rabot <sylvain@abstraction.fr>
Date: Jun 3, 2010, 10:55 UTC
Message-ID: <AANLkTikPHdRWsHUI-YwEkECtIbj425hKxzy8_JrA4_ny@mail.gmail.com>
In-Reply-To: <7vbpbt9f9j.fsf@alter.siamese.dyndns.org>

On Thu, Jun 3, 2010 at 01:38, Junio C Hamano <gitster@pobox.com> wrote:

Show 13 quoted lines

> Sylvain Rabot <srabot@steek.com> writes:
>
>> +Be aware that 'git mergetool' has some predefined invocation commands builtin
>> +for known diff tools like meld, diffuse, p4merge ... etc. It means that if
>> +you set the merge.tool configuration to one of these tools, the
>> +mergetool.<tool>.* configurations will not be taken care of. If you
>> +really want to customize the invocation of one of these tools,
>> +set `merge.tool` to "custom" or whatever you want and `mergetool.custom.cmd`
>> +to "/usr/bin/<tool> $LOCAL $MERGED $REMOTE"
>
> Two half-points and three points (that makes them four in total ;-):
>
>  o If I read the above without "It means that", it still makes sense;

Ok

>
>  o "If you really" can go without "really";

Ok

>
>  * I had to read "will not be taken care of" twice; "are ignored" is
>   probably easier to understand;

Ok

Show 7 quoted lines

>
>  * The description and the example makes it sound as if the command line
>   has to have these three tokens in the given order, but the whole point
>   of this mechanism is that you can launch whatever external command with
>   a custom command line, so "and `mergetool.custom.cmd` to a command line
>   to invoke the command.  $LOCAL $MERGED and $REMOTE on this command line
>   are substituted by ...." may be more reader-friendly.

Ok

>
>  * As a documentation update it is perfectly fine to describe this glitch,
>   but I wonder if we might want to lift this restriction (read: consider
>   this as a bug and fix it).

I agree, I wil take a look, I'm not a sh expert but I can try. If I succeed I will send you a patch, otherwise I will send you a corrected version of this patch.

Show 12 quoted lines

>
>> ++
>> +-------------
>> +[merge]
>> +    tool = diffuse-custom
>> +[mergetool "diffuse-custom"]
>> +    cmd = diffuse $LOCAL $MERGED $REMOTE
>> +-------------
>> ++
>
> Thanks.
>

-- 
Sylvain