From: demerphq <demerphq@gmail.com>
Date: Tue, 24 Feb 2009 21:43:37 GMT
Subject: Re: [PATCH] git diff ignore-space options should ignore missing EOL at EOF differences
Message-ID: <9b18b3110902241343v11bf015ftad5c90259007a243@mail.gmail.com>
In-Reply-To: <alpine.DEB.1.00.0902151615400.10279@pacific.mpi-cbg.de>

2009/2/15 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
> Hi,
>
> On Sun, 15 Feb 2009, demerphq wrote:
>
>> 2009/2/15 Johannes Schindelin <Johannes.Schindelin@gmx.de>:
>> > Hi,
>> >
>> > On Sun, 15 Feb 2009, demerphq wrote:
>> >
>> >>  t/t4015-diff-whitespace.sh             |   79 ++++++++++++++++++++++++++++++++
>> >
>> > Phew, you certainly want to make sure that it works...
>>
>> Yeah, Exhaustive testing is good. (When it doesn't take hours and
>> hours to run :-)
>
> You read my mind.
>
>> >> @@ -33,7 +33,14 @@ extern "C" {
>> >>  #define XDF_IGNORE_WHITESPACE_CHANGE (1 << 3)
>> >>  #define XDF_IGNORE_WHITESPACE_AT_EOL (1 << 4)
>> >>  #define XDF_PATIENCE_DIFF (1 << 5)
>> >> -#define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE |
>> >> XDF_IGNORE_WHITESPACE_CHANGE | XDF_IGNORE_WHITESPACE_AT_EOL)
>> >> +#define XDF_IGNORE_WHITESPACE_AT_EOF (1 << 6)
>> >> +/*
>> >> + * note this is deliberately a different define from XDF_WHITESPACE_FLAGS as
>> >> + * there could be a new whitespace related flag which would not be part of
>> >> + * the XDF_IGNORE_WHITESPACE_AT_EOF_ANY flags.
>> >> + */
>> >> +#define XDF_IGNORE_WHITESPACE_AT_EOF_ANY
>> >> (XDF_IGNORE_WHITESPACE_AT_EOL | XDF_IGNORE_WHITESPACE_CHANGE |
>> >> XDF_IGNORE_WHITESPACE | XDF_IGNORE_WHITESPACE_AT_EOF)
>> >> +#define XDF_WHITESPACE_FLAGS (XDF_IGNORE_WHITESPACE |
>> >> XDF_IGNORE_WHITESPACE_CHANGE | XDF_IGNORE_WHITESPACE_AT_EOL |
>> >> XDF_IGNORE_WHITESPACE_AT_EOF)
>> >
>> > As I told you on IRC, I do not follow that reasoning.  Rather, I would add
>> > the exceptions to xemit.c, when -- and if(!) -- they are needed.
>>
>> Yeah I know you said that, and I *think* I followed all your advice
>> (much appreciated by the way) except for that point as I've been
>> nailed by inappropriate addition of flags to masks before, and well,
>> you know, once bitten twice shy, and patchers perogative and all that
>> eh? :-)
>
> I understand that, but IMHO it is overengineered.  I am not really
> convinced that ignore-whitespace-at-sol makes sense, either...

Well, if there is a consensus that it is overengineered to add a new
define that will prevent hard to detect future bugs, then ill change
the code. Although id feel more comfortable with hearing this from
Junio himself. But before I put together a new patch is there any
other feedback?

Yves

-- 
perl -Mre=debug -e "/just|another|perl|hacker/"