"Elijah Newren via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
> diff --git a/diffcore-delta.c b/diffcore-delta.c
> index c30b56e983b..7136c3dd203 100644
> --- a/diffcore-delta.c
> +++ b/diffcore-delta.c
> @@ -159,6 +159,10 @@ static struct spanhash_top *hash_chars(struct repository *r,
> n = 0;
> accum1 = accum2 = 0;
> }
> + if (n > 0) {
> + hashval = (accum1 + accum2 * 0x61) % HASHBASE;
> + hash = add_spanhash(hash, hashval, n);
> + }
OK, so we were ignoring the final short bit that is not terminated
with LF due to the "continue". Nicely found.
> diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
> index 85be1367de6..29299acbce7 100755
> --- a/t/t4001-diff-rename.sh
> +++ b/t/t4001-diff-rename.sh
> @@ -286,4 +286,23 @@ test_expect_success 'basename similarity vs best similarity' '
> test_cmp expected actual
> '
>
> +test_expect_success 'last line matters too' '
> + test_write_lines a 0 1 2 3 4 5 6 7 8 9 >nonewline &&
> + printf "git ignores final up to 63 characters if not newline terminated" >>nonewline &&
> + git add nonewline &&
> + git commit -m "original version of file with no final newline" &&
I found it misleading that the file whose name is nonewline has
bunch of LF including on its last line ;-).
> + # Change ONLY the first character of the whole file
> + test_write_lines b 0 1 2 3 4 5 6 7 8 9 >nonewline &&
Same here, but it is too much to bother rewriting it ...
{
test_write_lines ...
printf ...
} >incomplete
... like so ("incomplete" stands for "file ending with an incomplete line"),
so I'll let it pass.
> + printf "git ignores final up to 63 characters if not newline terminated" >>nonewline &&
> + git add nonewline &&
> + git mv nonewline still-no-newline &&
> + git commit -a -m "rename nonewline -> still-no-newline" &&
> + git diff-tree -r -M01 --name-status HEAD^ HEAD >actual &&
> + cat >expected <<-\EOF &&
> + R097 nonewline still-no-newline
I am not very happy with the hardcoded 97. You are already using
the non-standard 10% threshold. If the delta detection that
forgets about the last line is so broken as your proposed log
message noted, shouldn't you be able to construct a sample pair of
preimage and postimage for which the broken version gives so low
similarity to be judged not worth treating as a rename, while the
fixed version gives reasonable similarity to be made into a rename,
by the default threshold? That way, the test only needs to see if
we got a rename (with any similarity) or a delete and an add.
