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

Hi,

On Sun, 15 Feb 2009, demerphq wrote:

2009/2/15 Johannes Schindelin [off-list ref]:

quoted

Hi,

On Sun, 15 Feb 2009, demerphq wrote:

quoted

 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.

quoted

quoted

@@ -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...

Ciao,
Dscho