[PATCH/RFC v1 1/1] +5 cases (4 fail), diff whitespace tests

[PATCH/RFC v1 1/1] +5 cases (4 fail), diff whitespace tests

[Keith Cascio]

  +5 cases (4 fail), diff whitespace tests
  There are 2^3 = eight possible combinations of the three flags:
  -w -b --ignore-space-at-eol
  Three of those combinations were already being tested:
  [none]
  -w
  -b
  Add tests of the other five combinations,
  four of which fail with git
  3cf3b838c7b379824c68ee87799aaaa9028b36cd
  from Tue Jan 13 23:41:32 2009 -0800.

Signed-off-by: Keith Cascio <keith@cs.ucla.edu>
---

All four failures involve combining whitespace ignore options.  It's likely the 
fix will involve one or both of the following two functions in xdiff/xutils.c:
xdl_hash_record_with_whitespace()
xdl_recmatch()

I played around with it and discovered I could make
"git diff -b --ignore-space-at-eol" work by changing
if (flags & XDF_IGNORE_WHITESPACE_AT_EOL
to
else if (flags & XDF_IGNORE_WHITESPACE_AT_EOL
But I don't know if that would break something else.

                                          -- Keith

  t/t4015-diff-whitespace.sh |   27 +++++++++++++++++++++++++++
  1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index fc2307e..dbb608c 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -98,6 +98,12 @@ index d99af23..8b32fb5 100644
  EOF
  git diff -w > out
  test_expect_success 'another test, with -w' 'test_cmp expect out'
+git diff -w -b > out
+test_expect_failure 'another test, with -w -b' 'test_cmp expect out'
+git diff -w --ignore-space-at-eol > out
+test_expect_failure 'another test, with -w --ignore-space-at-eol' 'test_cmp expect out'
+git diff -w -b --ignore-space-at-eol > out
+test_expect_failure 'another test, with -w -b --ignore-space-at-eol' 'test_cmp expect out'

  tr 'Q' '\015' << EOF > expect
  diff --git a/x b/x
@@ -116,6 +122,27 @@ index d99af23..8b32fb5 100644
  EOF
  git diff -b > out
  test_expect_success 'another test, with -b' 'test_cmp expect out'
+git diff -b --ignore-space-at-eol > out
+test_expect_failure 'another test, with -b --ignore-space-at-eol' 'test_cmp expect out'
+
+tr 'Q' '\015' << EOF > expect
+diff --git a/x b/x
+index d99af23..8b32fb5 100644
+--- a/x
++++ b/x
+@@ -1,6 +1,6 @@
+-whitespace at beginning
+-whitespace change
+-whitespace in the middle
++	whitespace at beginning
++whitespace 	 change
++white space in the middle
+ whitespace at end
+ unchanged line
+ CR at endQ
+EOF
+git diff --ignore-space-at-eol > out
+test_expect_success 'another test, with --ignore-space-at-eol' 'test_cmp expect out'

  test_expect_success 'check mixed spaces and tabs in indent' '

-- 
1.6.1.137.gb17b6

Re: [PATCH/RFC v1 1/1] +5 cases (4 fail), diff whitespace tests

[Junio C Hamano]

Thanks, applied.

Re: [PATCH/RFC v1 1/1] +5 cases (4 fail), diff whitespace tests

[Junio C Hamano]

Keith Cascio <keith@CS.UCLA.EDU> writes:

>  +5 cases (4 fail), diff whitespace tests
>  There are 2^3 = eight possible combinations of the three flags:
>  -w -b --ignore-space-at-eol
>  Three of those combinations were already being tested:
>  [none]
>  -w
>  -b
>  Add tests of the other five combinations,

Hmm.  Are these three supposed to be orthogonal?

Re: [PATCH/RFC v1 1/1] +5 cases (4 fail), diff whitespace tests

[Keith Cascio]

On Sat, 17 Jan 2009, Junio C Hamano wrote:

> Hmm.  Are these three supposed to be orthogonal?

The semantics of those 3 flags are not orthogonal, no.  Their relationship 
amongst each other is one of transitive implication:

-w                    implies the other two
-b                    implies --ignore-space-at-eol
--ignore-space-at-eol implies only itself

Therefore, it is never *necessary* to specify more than one of these flags on 
the command line.  However, it is not hard to imagine scenarios where software 
wrappers around git (e.g. GUIs, etc), generate command lines with more than one 
of these flags.  I thought about it, and it seems unreasonable to make it an 
error to specify more than one, since a new user might not immediately grasp the 
way they imply each other.  I think Git could and should treat it as a legal 
case.  I contacted Dscho about fixing it, but he is busy so I will submit a fix 
patch myself.