git diff-tree ignores --textconv

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

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

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

So here:

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

On 2018-09-23 05:43 PM, Jeff King wrote:

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

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

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.

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

On 2018-09-23 06:23 PM, Jeff King wrote:

Fantastic. Thank you so much, Jeff.