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