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

Keith Cascio· Jan 15, 2009, 00:48 UTC · lore

  +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(-)

Show changes to t/t4015-diff-whitespace.sh +26 −0

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

Junio C Hamano· Jan 18, 2009, 07:45 UTC · re: Keith Cascio · lore

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

Thanks, applied.

Junio C Hamano· Jan 18, 2009, 07:47 UTC · re: Keith Cascio · lore

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

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

Show 8 quoted lines

>  +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?

Keith Cascio· Jan 18, 2009, 19:25 UTC · re: Junio C Hamano · lore

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

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.

RFC patch+5 cases (4 fail), diff whitespace tests