Re: [PATCH 0/8] Reroll of sb/diff-color-move-more

[Jacob Keller]

On Thu, May 17, 2018 at 12:46 PM, Stefan Beller  wrote:
>>> * sb/diff-color-move-more (2018-04-25) 7 commits
>>...
>>>
>>>  Will merge to 'next'.
>>
>>I did not get around to fix it up, there are still review
>>comments outstanding. (The test is broken in the last commit.)
>
> This is a reroll of sb/diff-color-move-more, with the test fixed as well
> as another extra patch, that would have caught the bad test.
>
> The range diff is below.
>
> Thanks,
> Stefan
>
> Stefan Beller (8):
>   xdiff/xdiff.h: remove unused flags
>   xdiff/xdiffi.c: remove unneeded function declarations
>   diff.c: do not pass diff options as keydata to hashmap
>   diff.c: adjust hash function signature to match hashmap expectation
>   diff.c: add a blocks mode for moved code detection
>   diff.c: decouple white space treatment from move detection algorithm
>   diff.c: add --color-moved-ignore-space-delta option
>   diff: color-moved white space handling options imply color-moved
>

I've been using this locally, and it's really nice. One question I
had, are there plans to make the whitespace options configurable? I
really like the option for enabling lines to count as moved when the
whitespace changes uniformly, (it helps make changes more obvious when
doing indentation changes such as wrapping code within a block).
However, it's rather a long option name to type out. I didn't see any
obvious config options to enable it by default though.

Thanks,
Jake

Re: [PATCH 0/8] Reroll of sb/diff-color-move-more

[Jonathan Tan]

On Thu, 17 May 2018 12:46:45 -0700
Stefan Beller  wrote:

> Stefan Beller (8):
>   xdiff/xdiff.h: remove unused flags
>   xdiff/xdiffi.c: remove unneeded function declarations
>   diff.c: do not pass diff options as keydata to hashmap
>   diff.c: adjust hash function signature to match hashmap expectation
>   diff.c: add a blocks mode for moved code detection
>   diff.c: decouple white space treatment from move detection algorithm
>   diff.c: add --color-moved-ignore-space-delta option
>   diff: color-moved white space handling options imply color-moved

The test in patch 7 is indeed fixed, and patch 8 looks good to me.

There are still some review comments of mine outstanding [1] [2] but if
we decide that this series is good enough for now, that's OK.

[1] 
https://public-inbox.org/git/20180424153513.dc2404cd111c44ac78bd8...@google.com/
[2] 
https://public-inbox.org/git/20180424171123.7092788b94498908c25ec...@google.com/

[PATCH 0/8] Reroll of sb/diff-color-move-more

[Stefan Beller]

>> * sb/diff-color-move-more (2018-04-25) 7 commits
>...
>>
>>  Will merge to 'next'.
>
>I did not get around to fix it up, there are still review
>comments outstanding. (The test is broken in the last commit.)

This is a reroll of sb/diff-color-move-more, with the test fixed as well
as another extra patch, that would have caught the bad test. 

The range diff is below.

Thanks,
Stefan

Stefan Beller (8):
  xdiff/xdiff.h: remove unused flags
  xdiff/xdiffi.c: remove unneeded function declarations
  diff.c: do not pass diff options as keydata to hashmap
  diff.c: adjust hash function signature to match hashmap expectation
  diff.c: add a blocks mode for moved code detection
  diff.c: decouple white space treatment from move detection algorithm
  diff.c: add --color-moved-ignore-space-delta option
  diff: color-moved white space handling options imply color-moved

 Documentation/diff-options.txt |  25 -
 diff.c | 138 +++
 diff.h |   8 +-
 t/t4015-diff-whitespace.sh | 197 +++--
 xdiff/xdiff.h  |   8 --
 xdiff/xdiffi.c |  17 ---
 6 files changed, 336 insertions(+), 57 deletions(-)

-- 
2.17.0.582.gccdcbd54c44.dirty

1:  a7a7af6b76b = 1:  a7a7af6b76b xdiff/xdiff.h: remove unused flags
2:  a7b6aaf7bc0 = 2:  a7b6aaf7bc0 xdiff/xdiffi.c: remove unneeded function 
declarations
3:  d9e57cc6b05 = 3:  d9e57cc6b05 diff.c: do not pass diff options as keydata 
to hashmap
4:  87111ba726d = 4:  87111ba726d diff.c: adjust hash function signature to 
match hashmap expectation
5:  9559b8cb456 = 5:  9559b8cb456 diff.c: add a blocks mode for moved code 
detection
6:  41a70464209 = 6:  41a70464209 diff.c: decouple white space treatment from 
move detection algorithm
7:  c0114b2ce56 ! 7:  72bb8213cab diff.c: add --color-moved-ignore-space-delta 
option
@@ -6,7 +6,6 @@
 changes uniformly.
 
 Signed-off-by: Stefan Beller 
-Signed-off-by: Junio C Hamano 
 
 diff --git a/Documentation/diff-options.txt 
b/Documentation/diff-options.txt
 --- a/Documentation/diff-options.txt
@@ -237,7 +236,7 @@
 +  not adjust
 +  EOF
 +
-+  git diff --color --color-moved-ignore-space-prefix-delta |
++  git diff --color --color-moved --color-moved-ignore-space-prefix-delta |
 +  grep -v "index" |
 +  test_decode_color >actual &&
 +
@@ -246,20 +245,20 @@
 +  --- a/text.txt
 +  +++ b/text.txt
 +  @@ -1,7 +1,7 @@
-+  -QIndented
-+  -QText across
-+  -Qthree lines
++  -QIndented
++  -QText across
++  -Qthree lines
 +  -QBut! <- this stands out
-+  -Qthis one
-+  -QQline did
-+  -Qnot adjust
-+  +QQIndented
-+  +QQText across
-+  +QQthree lines
++  -Qthis one
++  -QQline did
++  -Qnot adjust
++  +QQIndented
++  +QQText across
++  +QQthree lines
 +  +QQQBut! <- this stands out
-+  +this one
-+  +Qline did
-+  +not adjust
++  +this one
++  +Qline did
++  +not adjust
 +  EOF
 +
 +  test_cmp expected actual
-:  --- > 8:  9e0508c8381 diff: color-moved white space handling 
options imply color-moved