Johannes Schindelin (Johannes.Schindelin@xxxxxx) wrote on Aug 5, 2009:
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.
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:
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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html