Re: [PATCH 0/4] diff text conversion filter

From: Johannes Sixt <j.sixt@viscovery.net>
Date: Oct 7, 2008, 16:15 UTC
Message-ID: <48EB8B1C.9060201@viscovery.net>
In-Reply-To: <20081007154646.GB26531@coredump.intra.peff.net>

Jeff King schrieb:

Show 42 quoted lines

> On Tue, Oct 07, 2008 at 08:15:45AM +0200, Matthieu Moy wrote:
> 
>> One proposal: have a diff.<driver>.activate with several values:
>>
>> * "always": activate the diff driver in any porcelain
>> * "diff": activate it only for "git diff", as currently
>> * "singlefile": Johannes's heuristic proposal
>>
>> That way, one could say easily "activate exiftags filter all the time,
>> but MS-Word only when I request a diff for a single file", and this
>> leaves room for other values if the need be. Well, there's no room for
>> "use MS-Word native diff tool in git-gui but antiword/catdoc +
>> textconv in 'git log -p'" here, but do we want it?
>>
>> Or is all that just overkill?
> 
> Actually, as soon as I read the first part of your mail I thought of the
> "ms-word vs antiword" situation. The example you gave seems plausible.
> And it seems to me that it is really a superset of the problem we are
> discussing. That is, gitattributes is really just saying "this is a
> 'foo' type file". And if we have a mechanism flexible enough to say "in
> this situation, this is how you handle 'foo' type files", then that
> would work as the basis for implementing these heuristics.
> 
> For example, maybe you could set up some mapping like:
> 
>   git diff --diffdriver=foo,bar
> 
> where a file with gitattribute diff=x would look for the driver config
> for x.foo, then x.bar, and then finally fall back to just x. Then you
> could easily have (ignoring the fact that I'm not sure about the config
> syntax for having _3_ section parts):
> 
>   git config diff.doc.graphical.command ms-word-diff-script
>   git config diff.doc.textconv antiword
> 
> and then git-gui would be configured to diff with:
> 
>   git diff --diffdriver=graphical
> 
> whereas a regular "git diff" would always fall back to
> "diff.doc.textconv". Make sense?

I think you are complicating things. We already have

   diff.doc.command ms-word-diff-script

and with your 4-patch-series-under-discussion we would have

   diff.doc.texconv antiword

and that should be sufficient. I'm proposing this heuristics:

 * If only textconv is given, all porcelains pick it.
 * If only command is given, all porcelains pick it.
 * If both are given, then
   - git log picks textconv.
   - git show and git diff:
     . if exactly one pathspec was given, pick command;
     . otherwise pick textconv

Plumbing never picks any of them, just like today, nor should git format-patch. The are other porcelains that could be sorted into this list, like git blame and (the summary line of) git commit.

BTW, please don't take git-gui as an example that would lauch MS-Word on each diff. (Neither would gitk.) Both rely on plumbing, and that's good. gitk has a menu entry "External diff", where the diff.doc.command could be hooked into.

-- Hannes