Re: [PATCH RFC] Rename detection and whitespace

From: Jeff King <peff@peff.net>
Date: Apr 20, 2011, 06:03 UTC
Message-ID: <20110420060343.GD28597@sigill.intra.peff.net>
In-Reply-To: <BANLkTikiH7bfWFGjFCDL-SnO9HQR-9Uofw@mail.gmail.com>

On Tue, Apr 19, 2011 at 09:13:19PM +0100, Ciaran wrote:

Show 9 quoted lines

> For me I tracked the behaviour down to the blob similarity calculation
> that takes place in the diffcore-delta.c#hash_chars method.  In our
> case the problem was we were adjusting the whitespace at the front of
> each line which meant that the 64 byte segment hashes were
> different/mis-aligned between the 2 equivalent files.   This code
> already 'normalises' out CRLF/LF differences by skipping any CR
> characters when followed by LF so my question is that would it be
> considered wrong/evil to ignore *all* whitespace characters when -X
> ignore-all-space has been passed.

I think what you are proposing makes sense. If the diff which spawned the rename detection doesn't care about whitespace, then probably it should be ignored in the rename, too. That would go for merging with "-X ignore-all-space", but also for "git diff -w" (and probably "-b").

I do think it should be conditional on those, though. We _know_ that CRLF versus LF in a text file is irrelevant to the file's content. But whether whitespace is irrelevant varies from file to file.

So your patch is in the right direction, in my opinion.

-Peff