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

Re: [PATCH] git diff ignore-space options should ignore missing EOL at EOF differences

From
Ddemerphq <demerphq@gmail.com>
Date
Feb 24, 2009, 21:43 UTC
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>:
Show 48 quoted lines
> 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/"
Previous: Johannes SchindelinNext: Junio C Hamano
Message 5 of 6 in “git diff ignore-space options should ignore missing EOL at EOF differences”
  1. git diff ignore-space options should ignore missing EOL at EOF differencesdemerphq, Feb 15, 2009
  2. Johannes SchindelinFeb 15, 2009
  3. demerphqFeb 15, 2009
  4. Johannes SchindelinFeb 15, 2009
  5. demerphqFeb 24, 2009
  6. Junio C HamanoFeb 25, 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.