Re: [PATCH] Also use unpack_trees() in do_diff_cache()

[Linus Torvalds <torvalds@linux-foundation.org>]

On Sun, 20 Jan 2008, Johannes Schindelin wrote:
> 
> As in run_diff_index(), we call unpack_trees() with the oneway_diff()
> function in do_diff_cache() now.  This makes the function diff_cache()
> obsolete.

Sadly, this doesn't work yet.

diiff_cache() does magic things with different stages. 

The new unpack_trees() model *should* make it possible to do an even 
better job at this (we should really approximate the diff_files() 
behaviour), but oneway_diff() currently doesn't do it right.

Try

	git diff-index [-p] [--cached] -m HEAD

in a tree with unmerged entries (or just "git diff HEAD", which does the 
same thing).

The following patch gets us closer, but really only for the "--cached" 
case (for the non-cached case, it should use the working tree entry ratehr 
than generate a unmerged entry), and I also suspect there's a case it gets 
wrong for the case of the unmerged path not existign in the tree at all 
(the "if (tree)" entry basically ends up being a stand-in for the "is this 
the first index entry for this path we see" case).

The really nice thing would be to do what "diff_files()" does for this 
all, something that the old model couldn't do at all (that's why it 
currently prints out that "* Unmerged path xyzzy" thing - the *correct* 
thing to do would be to generate the same combined diff that diff_files() 
does).

So some more work is needed - either to get back the old (arguably 
quite broken) behaviour, or to go the whole way and do it right.

Anyway, my point with this all is that even without any of this set of 
fixes, git doesn't do this optimally. Try this with the current stable git 
master:

	git diff --cached HEAD

with a unmerged file, and compare it to the much more useful "git diff" 
output. But right now, this patch series makes this case behave even 
worse, I think.

		Linus

---
 diff-lib.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 23d0fa6..3ac4d1f 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -685,6 +685,12 @@ static int oneway_diff(struct cache_entry **src,
 	if (tree == o->df_conflict_entry)
 		tree = NULL;
 
+	if (ce_stage(idx)) {
+		if (tree)
+			diff_unmerge(&revs->diffopt, idx->name, idx->ce_mode, idx->sha1);
+		return 1;
+	}
+
 	/*
 	 * Something added to the tree?
 	 */