Commit 140b378d07229e breaks gitk highlighting

Previous thread: Re: git-p4 and keyword expansion by dhruva on Tuesday, September 9, 2008 - 4:14 am. (1 message)

Next thread: [RFC] origin link for cherry-pick and revert by Stephen R. van den Berg on Tuesday, September 9, 2008 - 6:22 am. (135 messages)
From: Mark Levedahl
Date: Tuesday, September 9, 2008 - 5:31 am

gitk since the above commit ("Teach git diff-tree --stdin to diff
trees") fails to reliably highlight commits matching a given path
specification. For an example of the problem,

   $gitk ec3a4ba519c001

   and select commit "touching paths" with "t" in the entry bar

The 4'th commit down (t9124: clean up chdir usage) is NOT highlighted,
though it clearly touches t. Reverting commit 140b378 restores the
correct behavior. This gets into parts of the code I don't understand,
so I decline to try to offer a patch.

Mark
--

From: Karl
Date: Wednesday, September 10, 2008 - 9:01 am

Hmmm, not good. Any idea of what git commands gitk actually runs in
your example? Would be nice to be able to diff the output before and
after the change.

-- 
Karl Hasselström, kha@treskal.com
      www.treskal.com/kalle
--

From: Junio C Hamano
Date: Wednesday, September 10, 2008 - 12:22 pm

Feeding the following to "git diff-tree -r -s --stdin -- t" misbehaves
with that change.  The second line is skipped.

7c4d0219cf9ab6a7738a09ad7fec72d5e9f2ac67
a786091b4a487bc08bbff4864717cf5d8383e983
4a09bc966449ca0a7e9a5bb70f91b47debdd7c4e

This should fix it.

I think we should have a stronger warning against misusing lookup_object()
without knowing what it does.  It is marked "Internal only" in the header,
but it does not explain why.  The interface is there to help the lazy
object lookup (e.g. you parse a commit and learn about a parent it has.
You do not want to read the parent commit at that point, so you create a
placeholder commit object and point at it from the child.  You would want
to make sure that you populate the same placeholder object with the data
you read from the parent commit when it gets the turn of the parent commit
to be parsed).

diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 1138c2d..a9e32c9 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -71,8 +71,7 @@ static int diff_tree_stdin(char *line)
 	line[len-1] = 0;
 	if (get_sha1_hex(line, sha1))
 		return -1;
-	obj = lookup_object(sha1);
-	obj = obj ? obj : parse_object(sha1);
+	obj = parse_object(sha1);
 	if (!obj)
 		return -1;
 	if (obj->type == OBJ_COMMIT)
--

From: Mark Levedahl
Date: Wednesday, September 10, 2008 - 1:03 pm

Yes, that fixes the problems I see.

Thanks,
Mark
--

Previous thread: Re: git-p4 and keyword expansion by dhruva on Tuesday, September 9, 2008 - 4:14 am. (1 message)

Next thread: [RFC] origin link for cherry-pick and revert by Stephen R. van den Berg on Tuesday, September 9, 2008 - 6:22 am. (135 messages)