git diff-tree ignores --textconv

git diff-tree ignores --textconv

[Stas Bekman]

Hi,

I'm using a 3rd party application that internally uses 'git diff-tree'
instead of 'git diff'. I'm trying to add filter and it works with 'git
diff' but it gets ignored with 'git diff-tree' despite having --textconv.

I was able to reproduce the problem with the following much more
simplified setup:


$ git check-attr diff test/test.ipynb
test/test.ipynb: diff: jupyternotebook


$ git config --get diff.jupyternotebook.command
git-nbdiffdriver diff


$ GIT_TRACE=1 git diff test/test.ipynb
[...]
run_command: GIT_DIFF_PATH_COUNTER=1 GIT_DIFF_PATH_TOTAL=1
'git-nbdiffdriver diff'
[...]
<shows nbdiff output as desired>


$ GIT_TRACE=1 git diff-tree -p HEAD --textconv test/test.ipynb
<shows normal diff, ignoring nbdiff>

Git tracing shows nothing relevant as in the command above.

According to https://git-scm.com/docs/git-diff-tree adding --textconv
should have respected the configured diff filter, but it does nothing.

Is this a bug or do I have something misconfigured?

Thank you.

git version 2.17.1

-- 
________________________________________________
Stas Bekman       <'))))><       <'))))><
https://stasosphere.com  https://chestofbooks.com
https://experientialsexlab.com https://stason.org
https://stasosphere.com/experience-life/my-books

Re: git diff-tree ignores --textconv

[Jeff King]

On Sun, Sep 23, 2018 at 03:41:45PM -0700, Stas Bekman wrote:

> $ git config --get diff.jupyternotebook.command
> git-nbdiffdriver diff

That's an "external diff driver", not a textconv driver.

So here:

> $ GIT_TRACE=1 git diff-tree -p HEAD --textconv test/test.ipynb
> <shows normal diff, ignoring nbdiff>

You probably want "--ext-diff", not "--textconv".

There's some discussion in the gitattributes manpage, but the short of
it is that textconv converts binary input to text, which is then fed
through the normal diff mechanism. Whereas an external diff driver is
given both sides and can produce whatever output it wants. Textconv is
less flexible, but generally way easier to write.

-Peff

Re: git diff-tree ignores --textconv

[Stas Bekman]

On 2018-09-23 05:43 PM, Jeff King wrote:
> On Sun, Sep 23, 2018 at 03:41:45PM -0700, Stas Bekman wrote:
> 
>> $ git config --get diff.jupyternotebook.command
>> git-nbdiffdriver diff
> 
> That's an "external diff driver", not a textconv driver.
> 
> So here:
> 
>> $ GIT_TRACE=1 git diff-tree -p HEAD --textconv test/test.ipynb
>> <shows normal diff, ignoring nbdiff>
> 
> You probably want "--ext-diff", not "--textconv".
> 
> There's some discussion in the gitattributes manpage, but the short of
> it is that textconv converts binary input to text, which is then fed
> through the normal diff mechanism. Whereas an external diff driver is
> given both sides and can produce whatever output it wants. Textconv is
> less flexible, but generally way easier to write.

Thank you, Jeff, for explaining my misunderstanding and how to fix it.

Would it be safe to ask the maintainer of the application to include
both --textconv and --ext-diff in that 'git diff-tree' call? I only need
the latter, but someone needed --textconv there as it's in the code.

This is for this package:
https://github.com/rsmmr/git-notifier

It was added here:
https://github.com/rsmmr/git-notifier/search?q=textconv&unscoped_q=textconv

-- 
________________________________________________
Stas Bekman       <'))))><       <'))))><
https://stasosphere.com  https://chestofbooks.com
https://experientialsexlab.com https://stason.org
https://stasosphere.com/experience-life/my-books

Re: git diff-tree ignores --textconv

[Jeff King]

On Sun, Sep 23, 2018 at 05:56:03PM -0700, Stas Bekman wrote:

> > You probably want "--ext-diff", not "--textconv".
> [...]
> Would it be safe to ask the maintainer of the application to include
> both --textconv and --ext-diff in that 'git diff-tree' call? I only need
> the latter, but someone needed --textconv there as it's in the code.

I think so. The main reason that they are not the default for plumbing
commands such as diff-tree is that the output may be quite surprising to
anything trying to parse the output. Using --textconv will always
produce a diff, but one that may not be applied to the original content.
Using --ext-diff may produce output that doesn't even look like a diff,
though in practice they often do.

> This is for this package:
> https://github.com/rsmmr/git-notifier

It looks like the output is meant to be read by humans, so yeah, I think
it would be fine (and preferred) to enable both.

-Peff

Re: git diff-tree ignores --textconv

[Stas Bekman]

On 2018-09-23 06:23 PM, Jeff King wrote:
> On Sun, Sep 23, 2018 at 05:56:03PM -0700, Stas Bekman wrote:
> 
>>> You probably want "--ext-diff", not "--textconv".
>> [...]
>> Would it be safe to ask the maintainer of the application to include
>> both --textconv and --ext-diff in that 'git diff-tree' call? I only need
>> the latter, but someone needed --textconv there as it's in the code.
> 
> I think so. The main reason that they are not the default for plumbing
> commands such as diff-tree is that the output may be quite surprising to
> anything trying to parse the output. Using --textconv will always
> produce a diff, but one that may not be applied to the original content.
> Using --ext-diff may produce output that doesn't even look like a diff,
> though in practice they often do.
> 
>> This is for this package:
>> https://github.com/rsmmr/git-notifier
> 
> It looks like the output is meant to be read by humans, so yeah, I think
> it would be fine (and preferred) to enable both.

Fantastic. Thank you so much, Jeff.

-- 
________________________________________________
Stas Bekman       <'))))><       <'))))><
https://stasosphere.com  https://chestofbooks.com
https://experientialsexlab.com https://stason.org
https://stasosphere.com/experience-life/my-books