git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: combined diff does not detect binary files and ignores -diff attribute

From
Junio C Hamano <gitster@pobox.com>
Date
May 24, 2011, 04:46 UTC
Message-ID
<7v39k4aeos.fsf@alter.siamese.dyndns.org>
In-Reply-To
<20110523234131.GB10488@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 7 quoted lines
>> However, custom diff drivers (still) don't work. :-)
>
> Yeah, I didn't add any support for that. I'm not sure what it should do;
> custom diff drivers don't know how to handle combined diff, do they?
>
> If you write me a test case that explains what _should_ happen, I'll see
> what I can do. :)

I do not think it is sensible to expect anybody to come up with a sane semantics for combined diff to work with GIT_EXTERNAL_DIFF (and external diff driver that can be specified via the attributes mechanism) in any meaningful way.

The whole point of the external diff mechanism is that an external command can take _two_ files and represent the change between them in a way that is more suited for the need of the user than the patch form. The output from such an external command does not have any obligation to even follow the convention used by the patch output, namely:

  @@ from here to there things have changed @@
   this is common
  -this was the removed content
  +this is the new content

as the _whole_ point of the external diff mechanism is to give something that is _different_ from the patch form, in the hope that it is in a more appropriate form for whoever consumes the output.

On the other hand, combined diff is all about combining multiple patches show them side-by-side in a combined fashion. Without the above four kinds of cues, there is no way to even _align_ the change outputs from two parents, let alone _combining_ them.

Anybody interested can check "compare-cooking.perl" in the todo branch, which is used as an external diff driver to view the differences between "What's cooking" postings via these:

    [diff "whatscooking"]
            xfuncname = "^\\[(.*)\\]$"
            command = ./compare-cooking.perl
in the .git/config file, together with
    whats-cooking.txt diff=whatscooking
in the .gitattributes file. Running
    $ git log -p --ext-diff todo -- whats-cooking.txt
would give a sample output.

It is conceivable that we _could_ newly define a "combined external diff driver" that would take 3 or more files, and compute and show the combined result by itself, but that will certainly not go through the codepath you touched with the textconv patch. Calling out to such a new type of external diff driver would have to happen at the level where we have 1+N blob object names for a N-parent commit, namely, at the beginning of show_patch_diff(), bypassing the entire contents of that function and instead letting the new n-way external diff driver do everything.

I however highly doubt that such an interface would make sense. For example, what would be the desirable format to compare three versions of "What's cooking" postings, and how would the updated compare-cooking.perl script would look like?

Previous: Jeff KingNext: Michael J Gruber
Message 28 of 37 in “combined diff does not detect binary files and ignores -diff attribute”
  1. Jay SoffianMay 22, 2011
  2. Michael J GruberMay 23, 2011
  3. Jay SoffianMay 23, 2011
  4. Junio C HamanoMay 23, 2011
  5. Jeff KingMay 23, 2011
  6. Jeff KingMay 23, 2011
  7. 1/5 combine-diff: split header printing into its own functionJeff King, May 23, 2011
  8. 2/5 combine-diff: calculate mode_differs earlierJeff King, May 23, 2011
  9. 3/5 combine-diff: handle binary files as binaryJeff King, May 23, 2011
  10. Junio C HamanoMay 23, 2011
  11. Jeff KingMay 23, 2011
  12. Junio C HamanoMay 30, 2011
  13. Jeff KingMay 30, 2011
  14. Jeff KingMay 30, 2011
  15. Junio C HamanoMay 30, 2011
  16. Junio C HamanoMay 31, 2011
  17. 4/5 refactor get_textconv to not require diff_filespecJeff King, May 23, 2011
  18. 5/5 combine-diff: respect textconv attributesJeff King, May 23, 2011
  19. Junio C HamanoMay 23, 2011
  20. Jeff KingMay 23, 2011
  21. Junio C HamanoMay 24, 2011
  22. Jeff KingMay 24, 2011
  23. Jay SoffianMay 23, 2011
  24. Jay SoffianMay 23, 2011
  25. Jeff KingMay 23, 2011
  26. Jay SoffianMay 24, 2011
  27. Jeff KingMay 23, 2011
  28. Junio C HamanoMay 24, 2011
  29. Michael J GruberMay 24, 2011
  30. Junio C HamanoMay 24, 2011
  31. Michael J GruberMay 24, 2011
  32. Junio C HamanoMay 24, 2011
  33. Jay SoffianMay 24, 2011
  34. Jeff KingMay 24, 2011
  35. Michael J GruberMay 25, 2011
  36. Jeff KingMay 25, 2011
  37. Jay SoffianMay 24, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.