Re: [PATCH 3/5] t4015: test the output of "diff --color-moved -b"

[Jeff King <peff@peff.net>]

On Thu, Oct 19, 2017 at 02:03:54PM -0700, Stefan Beller wrote:

> > +
> > +       git diff HEAD --no-renames --color-moved --color |
> > +               grep -v "index" |
> > +               test_decode_color >actual &&
> 
> The -v index grep makes it future proof (for a new hash)!
> I like that. What I do not like is the pipe at the end of
> git-diff as we certainly want to inspect the return code
> (or for a segfault ;)

I can claim neither credit nor failure here. These lines are copied
straight from your existing test. :)

> > +       git diff HEAD --no-renames -b --color-moved --color |
> > +               grep -v "index" |
> > +               test_decode_color >actual &&
> 
> Do we need (or rather want) to give --color additionally,
> instead of having --color-moved imply it? I guess this is
> extra carefulness from the "color always" series?

I do think --color-moved probably ought to imply --color (or at least
--color=auto). But AFAIK it doesn't do so now, so we need --color (and
if it implied "auto", we'd have to overcome that anyway).

But again, this comes straight from the existing test.

> > -test_expect_success 'move detection with whitespace changes' '
> > -       test_when_finished "git reset --hard" &&
> > -       test_seq 10 >test &&
> > -       git add test &&
> > -       sed s/3/42/ <test >test.tmp &&
> > -       mv test.tmp test &&
> > -       git -c diff.colormoved diff --ignore-space-change -- test
> > -'
> 
> Ok, this is covered above in testing for the unchanged lines (1-5)

Yes, though I think it's actually the _changed_ lines which do it (the
problem was in next_byte(), so any diff with "--color-moved -b", whether
it had moves or not, would infinite loop as long as it had lines without
whitespace at the end).

-Peff