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

Johannes Schindelin (Johannes.Schindelin@gmx.de) wrote on Aug 5, 2009:

On Tue, 4 Aug 2009, Thell Fowler wrote:

quoted

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.

quoted

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:

quoted

quoted

quoted

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?

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.

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