git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Jeff King <peff@peff.net>
Date
Oct 19, 2017, 21:14 UTC
Message-ID
<20171019211435.jatotd34knltutco@sigill.intra.peff.net>
In-Reply-To
<CAGZ79kaszx26d8D0UrEj948Fam5b41hQWXcpC0V-5Rrom6M15w@mail.gmail.com>
On Thu, Oct 19, 2017 at 02:03:54PM -0700, Stefan Beller wrote:
Show 9 quoted lines
> > +
> > +       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. :)

Show 7 quoted lines
> > +       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.
Show 10 quoted lines
> > -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
Previous: Stefan BellerNext: Jeff King
Message 23 of 34 in “Out of memory with diff.colormoved enabled”
  1. Orgad ShanehOct 12, 2017
  2. Jeff KingOct 12, 2017
  3. Stefan BellerOct 12, 2017
  4. diff.c: increment buffer pointer in all code pathStefan Beller, Oct 12, 2017
  5. Jeff KingOct 13, 2017
  6. Jeff KingOct 13, 2017
  7. Stefan BellerOct 13, 2017
  8. Jeff KingOct 19, 2017
  9. Jeff KingOct 19, 2017
  10. Junio C HamanoOct 19, 2017
  11. Junio C HamanoOct 19, 2017
  12. Jeff KingOct 19, 2017
  13. Jeff KingOct 19, 2017
  14. Stefan BellerOct 19, 2017
  15. 0/5 fix "diff --color-moved --ignore-space-at-eol"Jeff King, Oct 19, 2017
  16. 1/5 t4015: refactor --color-moved whitespace testJeff King, Oct 19, 2017
  17. Stefan BellerOct 19, 2017
  18. Jeff KingOct 19, 2017
  19. 2/5 t4015: check "negative" case for "-w --color-moved"Jeff King, Oct 19, 2017
  20. Stefan BellerOct 19, 2017
  21. 3/5 t4015: test the output of "diff --color-moved -b"Jeff King, Oct 19, 2017
  22. Stefan BellerOct 19, 2017
  23. Jeff KingOct 19, 2017
  24. 4/5 diff: fix whitespace-skipping with --color-movedJeff King, Oct 19, 2017
  25. Stefan BellerOct 19, 2017
  26. Jeff KingOct 19, 2017
  27. Simon RuderichOct 20, 2017
  28. Jeff KingOct 20, 2017
  29. 5/5 diff: handle NULs in get_string_hash()Jeff King, Oct 19, 2017
  30. Stefan BellerOct 19, 2017
  31. Jeff KingOct 19, 2017
  32. Stefan BellerOct 19, 2017
  33. Stefan BellerOct 19, 2017
  34. Jeff KingOct 19, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.