From: David Aguilar <davvid@gmail.com>
Date: Sat, 21 Nov 2009 21:47:17 GMT
Subject: Re: [PATCH] gitk: Use git-difftool for external diffs
Message-ID: <A2458D5A-DBAD-4F74-B166-92BEEB162418@gmail.com>
In-Reply-To: <7veinsx6lj.fsf@alter.siamese.dyndns.org>

On Nov 20, 2009, at 12:51 PM, Junio C Hamano <gitster@pobox.com> wrote:

> I do not read Tcl very well but I am guessing that in gitk you specify
>>>
>>
> what tool to run (e.g. "frobanodiff -z"), gitk feeds you two temporary
> files on the filesystem to compare (e.g. "frobanodiff -z $tmp1  
> $tmp2"),
> and your command line is responsible for giving satisfying diff  
> experience
> to the end user.
>
> I see three possible approaches:
>
> * Teach "git-difftool" a mode to compare two arbitrary files on the
>   filesytem, and set that as "External Diff" command that takes the
>   filenames as extra two parameters, just like any other "External  
> Diff"
>   programs given to gitk does.  This is the least palatable, as it  
> won't
>   solve the read-only repository issue at all (it only allows you the
>   logic to choose the configured difftool backend program).
>
> * Instead of disabling the traditional "External Diff" and taking it  
> over
>   like your patch did, add a new codepath for "Difftool" that feeds  
> the
>   commit IDs and paths the way git-difftool expects.  The user can use
>   both, and the issue of read-only repository is solved when  
> "Difftool"
>   is used (but not "External Diff").
>
> * Take over "External Diff" codepath exactly like your patch did, but
>   teach "git-difftool" a new command line option to name an  
> unconfigured
>   external program that takes two filenames.  When "External Diff"
>   program is *not* configured in gitk, the command line to invoke
>   difftool would be exactly as in your patch, i.e. "difftool --no- 
> prompt
>   $from $to -- $path".  Otherwise, when gitk is configured to use an
>   external program, e.g. "frobanodiff -z", for "External Diff", you  
> pass
>   that command line to "git-difftool" via that new option, e.g.
>
>     difftool --no-prompt --extcmd="frobanodiff -z" $from $to -- $path
>
>   Then difftool is responsible for preparing the two necessary  
> temporary
>   files out of the given information ($from/$to/$path) and feeding  
> them
>   to "frobanodiff -z" command line.
>
>   Maybe such --extcmd support already exists in difftool, in which  
> case
>   my earlier suspicion that difftool is not as flexible would be  
> false.

Sounds good.  Adding --extcmd should be nice and straightforward.   
Markus mentioned the need for a diff.guitool variable that would be  
tested for in the gitk case so it sounds like having a --gui option to  
let difftool know to do that should then cover all the bases.

It's looking like we'll have ourselves a small difftool patch series  
soon.  Thanks all,

-- 
         David