From: Junio C Hamano <gitster@pobox.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Kyle McMartin <kyle@mcmartin.ca>,
Git Mailing List <git@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: Linux 2.6.24-rc6
Date: Fri, 21 Dec 2007 08:56:53 -0800 [thread overview]
Message-ID: <7vmys49eay.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <alpine.LFD.0.9999.0712202054350.21557@woody.linux-foundation.org> (Linus Torvalds's message of "Thu, 20 Dec 2007 20:58:42 -0800 (PST)")
Linus Torvalds <torvalds@linux-foundation.org> writes:
> Actually, the code to finding one '\n' is still needed to avoid the
> (pathological) case of getting a "\No newline", so scrap that one which
> was too aggressive, and use this (simpler) one instead.
>
> Not that it matters in real life, since nobody uses -U0, and "git blame"
> won't care. But let's get it right anyway ;)
Actually "blame won't care" is a bit too strong. It's only we
(you) made it not to care. It is a different story if the
change to make it not to care was sensible.
The diff text "git blame" will see is affected with the tail
trimming optimization exactly the same way as the optimization
triggered this thread. With the common tails trimmed, the hunks
match differently from the way they match without trimming (the
gcc changelog testcase has differences between the unoptimized
blame and the tail-trimming one --- your original to put this
logic only in blame and my rewrite to move it in xdiff-interface
produce the same result that is different from the unoptimized
version, although both are 4x faster than the original).
When there are multiple possible matches, any match among them
is a correct match, and a match with a line in a blob is a match
to the blob no matter what line the match is anyway. The usual
workflow of checking blame to find the commit that introduced
the change and then going to "git show" to view the bigger
picture won't get affected. But the blamed line numbers will be
different from the unoptimized blame, and it may not match the
expectation of human readers. It won't match "git show" output
of the blamed commit.
> This whole function has had more bugs than it has lines.
I have to agree. It started as a brilliant idea but then it was
enhanced (in an attempt to support context) and executed not so
brilliantly.
prev parent reply other threads:[~2007-12-21 16:57 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <alpine.LFD.0.9999.0712201731010.21557@woody.linux-foundation.org>
[not found] ` <20071221024805.GB8535@fattire.cabal.ca>
[not found] ` <20071221030152.GC8535@fattire.cabal.ca>
2007-12-21 3:49 ` Linux 2.6.24-rc6 Linus Torvalds
2007-12-21 3:50 ` Kyle McMartin
2007-12-21 4:22 ` Linus Torvalds
2007-12-21 4:40 ` Linus Torvalds
2007-12-21 4:50 ` Kyle McMartin
2007-12-21 4:58 ` Linus Torvalds
2007-12-21 5:21 ` Linus Torvalds
2007-12-21 17:45 ` Linus Torvalds
2007-12-21 16:56 ` Junio C Hamano [this message]
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=7vmys49eay.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=kyle@mcmartin.ca \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.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).