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
Michael J Gruber <git@drmicha.warpmail.net>
Date
May 24, 2011, 16:38 UTC
Message-ID
<4DDBDF0C.2040708@drmicha.warpmail.net>
In-Reply-To
<7vsjs48616.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 24.05.2011 17:36:
Show 36 quoted lines
> Michael J Gruber <git@drmicha.warpmail.net> writes:
> 
>> Jeff, thanks a bunch for taking this up again! That's a great
>> improvement. (I'm not sure I can devote enough time to reviewing, but
>> I'll see.)
> 
> Thanks. I've given a cursory look at the rest of the series (including the
> addendum to be squashed in) and they seemed Ok. Will give it another round
> of eyeballing before merging to "next".
> 
>>> 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?
>>
>> Yeah, currently --cc with external makes no sense, but there are several
>> external tools which could present a 3-way diff in a useful way (or even
>> n-way with n>3), e.g. vimdiff, kdiff3, meld.
> 
> With an external command that can make 3-way diffs of arbitrary three
> directories, you can trivially do something like:
> 
> 	#!/bin/sh
> 	mkdir tmp-head tmp-index &&
>         ( cd tmp-head &&
>           GIT_DIR=../.git GIT_INDEX_FILE=$GIT_DIR/.index-head &&
>           git checkout -f HEAD
> 	) &&
>         ( cd tmp-index &&
>           GIT_DIR=../.git &&
>           git checkout -f .
> 	) &&
>         $ext_diff3_cmd tmp-head/ tmp-index/ . &
>         wait
>         rm -fr tmp-index tmp-head
> 
and the fancy version of that is "difftool" in a sense, yes.
> But that seems totally offtopic and has nothing to do with the "combined
> diff" discussion, no?

Well, it seemed that Jay wanted an external tool for merge diffs. "-m" does a series of simple 2way diffs, but "--cc" is special in that it looks at all diffs at once, so a corresponding external tool would have to do the same.

Show 11 quoted lines
> If you want to plug in an external command that can make n-way diff of n
> files when some paths are still shown using the usual --cc codepath, then
> you would need an interface totally different from the diff.<driver>.cmd
> interface for two-way diff to the external diff.  I pointed at where to
> plug such a thing, but I do not think it would be of much use unless you
> are handing the whole n-trees to the external command (which essentially
> is what the above outline does). How would the user read the output that
> comes out mixed from different codepaths, some from our own --cc while
> others come from the external command, possibly opening separate windows
> and even worse grabbing control and getting the caller stuck until the
> user closes that window?

I don't quite follow, I think I/we/? am/are mixing external diff drivers and difftools. I agree that is a side track/off-topic.

Show 5 quoted lines
> 
> 	Side note: about getting stuck, will we see an update to the
> 	diffstat count series by the end of this cycle? I do not mind
> 	carrying it over to the next cycle at all, but I'd rather see
> 	something already started gets finished.
Yes, on my list. End of month you said, right?
Show 12 quoted lines
>> When the --cc/textconv issue came up I looked into this, and maybe
>> difftool is a place where one could plug this in first in the sense of
>> refactoring that even more and providing a diff3tool or such to view a
>> merge commit (or compare any 3 versions), or/and provide "git diff3 A B
>> C" which creates a fake merge (A+B -> C).
> 
> You do not need "git diff3 A B C" for a fake merge.
> 
> 	$ git diff 61d7503d 2d22086 5bf29b9
> 
> already is a way to show you how the commit 61d7503d was created by
> merging the other two (the merge result comes first and then its parents).

Yes. (I came across this when I investigated a bit back then but failed to mention it.)

Show 21 quoted lines
> You could put the index into the mix by doing something like:
> 
> 	$ git diff next master $(git write-tree)
> 
> Trying to show combined diff to merge the index and the working tree into
> the current HEAD (which may be an example that does not make much sense)
> would look like this:
> 
> 	$ git diff HEAD $(git write-tree) $(
> 		git read-tree --index-output=.tmp-index HEAD &&
> 		GIT_INDEX_FILE=.tmp-index git add -A :/ &&
>                 GIT_INDEX_FILE=.tmp-index git write-tree
> 	)
> 
> But for the "working tree" set, which paths should be included? The same
> set as what is in the index? Or would we use the set that is the union of
> other tree-like things that are being compared, including the ones that
> are not in the index? Or everything in the working tree, as we are
> interested in what the user _could_ add?  That is one of the reasons why I
> do not think it makes much sense trying to throw the working tree into the
> picture, as it would have to open a large can of worms.

I've simply been drooling too much over vim-fugitive and was wondering which aspects would fit into our ui. I was thinking of a 3-way-diff version of git status, so to say. Next would be a 3-way-merge interface which does all your add/reset/checkout -p from 3 vim bufs in diff mode, which even vim-fugitive doesn't do. We can all dream, can't we?

Michael
Previous: Junio C HamanoNext: Junio C Hamano
Message 31 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.