Re: [PATCH] diffcore-delta: avoid ignoring final 'line' of file

>
> "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> 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.