From: Junio C Hamano <gitster@pobox.com>
To: "Mark Levedahl" <mlevedahl@gmail.com>
Cc: "Git Mailing List" <git@vger.kernel.org>,
"Karl Hasselström" <kha@treskal.com>,
"Paul Mackerras" <paulus@samba.org>
Subject: Re: Commit 140b378d07229e breaks gitk highlighting
Date: Wed, 10 Sep 2008 12:22:35 -0700 [thread overview]
Message-ID: <7vtzcnx0uc.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <30e4a070809090531i6c6dd2c8r189e70927b5444c9@mail.gmail.com> (Mark Levedahl's message of "Tue, 9 Sep 2008 08:31:19 -0400")
"Mark Levedahl" <mlevedahl@gmail.com> writes:
> 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.
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)
next prev parent reply other threads:[~2008-09-10 19:23 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-09-09 12:31 Commit 140b378d07229e breaks gitk highlighting Mark Levedahl
2008-09-10 16:01 ` Karl Hasselström
2008-09-10 19:22 ` Junio C Hamano [this message]
2008-09-10 20:03 ` Mark Levedahl
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vtzcnx0uc.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=kha@treskal.com \
--cc=mlevedahl@gmail.com \
--cc=paulus@samba.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).