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

Re: Help/Advice needed on diff bug in xutils.c

From
Thell Fowler <git@tbfowler.name>
Date
Aug 10, 2009, 18:54 UTC
Message-ID
<alpine.DEB.2.00.0908101211590.32213@GWPortableVCS>
In-Reply-To
<alpine.DEB.1.00.0908052239180.8306@pacific.mpi-cbg.de>
Johannes Schindelin (Johannes.Schindelin@gmx.de) wrote on Aug 5, 2009:
Show 5 quoted lines
> On Tue, 4 Aug 2009, Thell Fowler wrote:
>
>> There is a bug in git diff (ignoring whitespace) that does not take into
>> account a trailing space at the end of a line at the end of a file when
>> no new line follows.

First, please forgive my hubris at thinking I had _found_ a bug when my ignorance of the issue was so obvious. I am definitely humbled after reading every post in the archive having to do with whitespace and diff.

Show 25 quoted lines
>>
>> Here is the example of the bug:
>> mkdir test_ws_eof
>> cd test_ws_eof
>> git init
>> echo -n "Test" > test.txt
>> git add .
>> git commit -m'test'
>> git symbolic-ref HEAD refs/heads/with_space
>> rm .git/index
>> git clean -f
>> echo -n "Test ">test.txt
>> git add .
>> git commit -m'test'
>> # Ignoring all whitespace there shouldn't be a diff.
>> git diff -w master -- test.txt
>> # Ignoring space at eol there shouldn't be a diff
>> git diff --ignore-space-at-eol master -- test.txt
>> # Ignoring with -b might have a case for a diff showing.
>> git diff -b master -- test.txt
>
> If you turn that into a patch to, say, t/t4015-diff-whitespace.sh (adding
> a test_expect_failure for a known bug), it is much easier to convince
> developers to work on the issue.
>
Thank you.  In progress.

I am curious if t4015 is planned to be to be rewritten to follow what Junio had outlined and Giuseppe implemented for t4107-apply-ignore-whitespace.sh to make the spaces more obvious to the reader:

One other question on the making of the test in regards to the following quote from: http://article.gmane.org/gmane.comp.version-control.git/124765 where Junio C Hamano wrote:

Show 11 quoted lines
>>>>	sed -e 's/Z/ /g' >patch3.patch <<\EOF
>>>>        ...
>>>>        +Z 	print_int(func(i));Z
>>>>        EOF
>>>>
>>>> to make invisible SP stand out more for the benefit of people reading 
>>>> the test script (I know you did not have leading SP before HT in 
>>>> yours, but the above illustrates the visibility issues).  For other 
>>>> tests with test vector patches, visibility of whitespace is not much 
>>>> an issue, but this script is _all about_ whitespace, so anything that 
>>>> clarifies what is going on better would help.
The test being implemented was t4107-apply-ignore-whitespace.sh.

Are there any plans to have t4015-diff-whitespace.sh tests rewritten in the same fashion?

Show 8 quoted lines
> First, your coding style is different from the surrounding code.  I think
> it goes without saying that this should be fixed.
>
> Second, you do not need the parentheses at all (and therefore they should
> go).
>
> Third, libxdiff does not assume to be fed NUL delimited strings.
>
You're absolutely right, I'll be more aware in the future.
Show 6 quoted lines
> Fourth, that condition "ptr + 1 < top" is already doing what you tried to
> accomplish here.
>
> So I guess that you need to do add "ptr + 1 < top" checks
> instead.
>
I'll give it another go.
Thank you for the advice Dscho.
-- 
Thell
Previous: Johannes SchindelinNext: Thell Fowler
Message 3 of 63 in “Help/Advice needed on diff bug in xutils.c”
  1. Thell FowlerAug 4, 2009
  2. Johannes SchindelinAug 5, 2009
  3. Thell FowlerAug 10, 2009
  4. Add diff tests for trailing-space and now newlineThell Fowler, Aug 12, 2009
  5. 0/6 Series to correct xutils incomplete line handling.Thell Fowler, Aug 19, 2009
  6. Thell FowlerAug 21, 2009
  7. Alex RiesenAug 21, 2009
  8. Thell FowlerAug 22, 2009
  9. 0/6 improvements for trailing-space processing on incomplete linesThell Fowler, Aug 23, 2009
  10. 1/6 Add supplemental test for trailing-whitespace on incomplete linesThell Fowler, Aug 23, 2009
  11. 2/6 xutils: fix hash with whitespace on incomplete lineThell Fowler, Aug 23, 2009
  12. Junio C HamanoAug 23, 2009
  13. Thell FowlerAug 23, 2009
  14. 3/6 xutils: fix ignore-all-space on incomplete lineThell Fowler, Aug 23, 2009
  15. Junio C HamanoAug 23, 2009
  16. Nanako ShiraishiAug 23, 2009
  17. Junio C HamanoAug 23, 2009
  18. Nanako ShiraishiAug 23, 2009
  19. Junio C HamanoAug 23, 2009
  20. Thell FowlerAug 23, 2009
  21. Junio C HamanoAug 23, 2009
  22. Teach mailinfo to ignore everything before -- >8 -- markNicolas Sebrecht, Aug 24, 2009
  23. Junio C HamanoAug 24, 2009
  24. Junio C HamanoAug 24, 2009
  25. Re: Teach mailinfo to ignore everything before -- >8 -- markNicolas Sebrecht, Aug 24, 2009
  26. Junio C HamanoAug 24, 2009
  27. Re: Teach mailinfo to ignore everything before -- >8 -- markNicolas Sebrecht, Aug 24, 2009
  28. Don ZickusAug 24, 2009
  29. Junio C HamanoAug 24, 2009
  30. Nanako ShiraishiAug 24, 2009
  31. Re: Teach mailinfo to ignore everything before -- >8 -- markNicolas Sebrecht, Aug 24, 2009
  32. Re: Teach mailinfo to ignore everything before -- >8 -- markNicolas Sebrecht, Aug 24, 2009
  33. Junio C HamanoAug 24, 2009
  34. Nicolas SebrechtAug 25, 2009
  35. Junio C HamanoAug 26, 2009
  36. Junio C HamanoAug 26, 2009
  37. Re: Teach mailinfo to ignore everything before -- >8 -- markNicolas Sebrecht, Aug 26, 2009
  38. Jakub NarebskiAug 26, 2009
  39. Johannes SchindelinAug 26, 2009
  40. Junio C HamanoAug 27, 2009
  41. Johannes SchindelinAug 27, 2009
  42. Junio C HamanoAug 26, 2009
  43. Re: Teach mailinfo to ignore everything before -- >8 -- markNicolas Sebrecht, Aug 26, 2009
  44. Nanako ShiraishiAug 24, 2009
  45. Thell FowlerAug 23, 2009
  46. Junio C HamanoAug 23, 2009
  47. Thell FowlerAug 23, 2009
  48. Junio C HamanoAug 23, 2009
  49. Thell FowlerAug 24, 2009
  50. Junio C HamanoAug 24, 2009
  51. Thell FowlerAug 24, 2009
  52. Thell FowlerAug 25, 2009
  53. 4/6 xutils: fix ignore-space-change on incomplete lineThell Fowler, Aug 23, 2009
  54. 5/6 xutils: fix ignore-space-at-eol on incomplete lineThell Fowler, Aug 23, 2009
  55. 6/6 t4015: add tests for trailing-space on incomplete lineThell Fowler, Aug 23, 2009
  56. 1/6 Add supplemental test for trailing-whitespace on incomplete lines.Thell Fowler, Aug 19, 2009
  57. 2/6 Make xdl_hash_record_with_whitespace ignore eofThell Fowler, Aug 19, 2009
  58. 3/6 Make diff -w handle trailing-spaces on incomplete lines.Thell Fowler, Aug 19, 2009
  59. Thell FowlerAug 20, 2009
  60. 4/6 Make diff -b handle trailing-spaces on incomplete lines.Thell Fowler, Aug 19, 2009
  61. 5/6 Make diff --ignore-space-at-eol handle incomplete lines.Thell Fowler, Aug 19, 2009
  62. 6/6 Add diff tests for trailing-space on incomplete linesThell Fowler, Aug 19, 2009
  63. Junio C HamanoAug 26, 2009

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.