Re: [PATCH] gitk: Use git-difftool for external diffs

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 20, 2009, 20:51 UTC
Message-ID: <7veinsx6lj.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20091120185522.GC56351@gmail.com>

David Aguilar <davvid@gmail.com> writes:

Show 9 quoted lines

>> > Also, I don't think we should remove the ability for the user to
>> > choose which external diff tool to use.
>> 
>> This is a larger concern.  Does "difftool" allow use of an arbitrary tool
>> like how gitk does (I have a suspicion that it is not as flexible)?
>
> difftool supports arbitrary tools through configuration.
> For arbitrary tools we set difftool.$name.cmd and
> diff.tool = $name.

So it is not as flexible as just giving a command line template for one-shot invocation, but that is Ok.

Don't get me wrong. I think the longer term direction should be to reduce code from gitk without making lifes harder to existing users, and just like built-in "diff --cc" stripped the combined-diff implemented in Tcl from gitk, use of "difftool" would be a good way to unify the "diffing" experience for a user who uses both command line and gitk at the same time.

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.

I think the last one would be the way to go in the longer term.