diff attribute ignored by show and log -p

diff attribute ignored by show and log -p

[Jay Soffian]

% cat .git/info/attributes
*.xib diff=xibdiff
% cat $(git config diff.xibdiff.command)
#!/bin/sh
trap "rm -f \"$2.tmp\" \"$5.tmp\"" 0 1 2 3 15
ibtool --all "$2" > "$2".tmp
ibtool --all "$5" > "$5".tmp
colordiff -u "$2.tmp" "$5.tmp"

Works great for things like:

% git diff <commit1> <commit2> -- /path/to/*.xib

But is apparently ignored by "git log -p" and "git show" which just
use internal diff. Is this behavior intentional?

j.

Re: diff attribute ignored by show and log -p

[Jay Soffian]

On Wed, Dec 16, 2009 at 11:46 PM, Jay Soffian <jaysoffian@gmail.com> wrote:
> % cat .git/info/attributes
> *.xib diff=xibdiff
> % cat $(git config diff.xibdiff.command)
> #!/bin/sh
> trap "rm -f \"$2.tmp\" \"$5.tmp\"" 0 1 2 3 15
> ibtool --all "$2" > "$2".tmp
> ibtool --all "$5" > "$5".tmp
> colordiff -u "$2.tmp" "$5.tmp"
>
> Works great for things like:
>
> % git diff <commit1> <commit2> -- /path/to/*.xib
>
> But is apparently ignored by "git log -p" and "git show" which just
> use internal diff. Is this behavior intentional?

Ah, --ext-diff, and the reasoning behind requiring it for log/show is
explained in 72909be.

j.

Re: diff attribute ignored by show and log -p

[Nanako Shiraishi]

Quoting Jay Soffian <jaysoffian@gmail.com>

>> But is apparently ignored by "git log -p" and "git show" which just
>> use internal diff. Is this behavior intentional?
>
> Ah, --ext-diff, and the reasoning behind requiring it for log/show is
> explained in 72909be.

The need to give --ext-diff is mentioned in 72909be (Add diff-option
--ext-diff, 2007-06-30) but its log message doesn't 'explain' why external
diff isn't used by default and you need to pass that extra option.

Probably --ext-diff should be the default?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: diff attribute ignored by show and log -p

[Jay Soffian]

On Thu, Dec 17, 2009 at 8:17 AM, Nanako Shiraishi <nanako3@lavabit.com> wrote:
> Quoting Jay Soffian <jaysoffian@gmail.com>
>
>>> But is apparently ignored by "git log -p" and "git show" which just
>>> use internal diff. Is this behavior intentional?
>>
>> Ah, --ext-diff, and the reasoning behind requiring it for log/show is
>> explained in 72909be.
>
> The need to give --ext-diff is mentioned in 72909be (Add diff-option
> --ext-diff, 2007-06-30) but its log message doesn't 'explain' why external
> diff isn't used by default and you need to pass that extra option.

"To prevent funky games with external diff engines, git-log and
friends prevent external diff engines from being called."

Seemed reason enough to me.

> Probably --ext-diff should be the default?

Or available via a config option anyway.

j.

Re: diff attribute ignored by show and log -p

[Jeff King]

On Thu, Dec 17, 2009 at 11:44:16AM -0500, Jay Soffian wrote:

> > The need to give --ext-diff is mentioned in 72909be (Add diff-option
> > --ext-diff, 2007-06-30) but its log message doesn't 'explain' why external
> > diff isn't used by default and you need to pass that extra option.
> 
> "To prevent funky games with external diff engines, git-log and
> friends prevent external diff engines from being called."
> 
> Seemed reason enough to me.

I don't remember discussing it at that time, but much later when
touching the external diff code and adding textconv, Junio and I came to
the conclusion that textconv was reasonable to do as part of "git log
-p", as the result is just an internal conversion that still results in
a text diff. Whereas an external diff might be terribly confusing, as it
could be spawning graphical viewers or producing output which does not
match well with the log output.

> > Probably --ext-diff should be the default?
> 
> Or available via a config option anyway.

If you were going to do such a config option, it might make sense to
make it an attribute of the diff driver rather than a global "use
external diff". Then each diff driver could say "Yes, I am reasonable to
be run as part of log -p".

-Peff