On Tue, 24 Apr 2018 14:03:29 -0700 Stefan Beller <sbeller@xxxxxxxxxx> wrote: > As we change the default, we'll adjust the tests. This statement is probably better written as: In some existing tests, options like --ignore-space-at-eol were used to control the color of the output. They have been updated to use options like --color-moved-ignore-space-at-eol instead. > + unsigned flags = diffopt->color_moved_ws_handling > + & XDF_WHITESPACE_FLAGS; No need for "& XDF_WHITESPACE_FLAGS". > + unsigned flags = o->color_moved_ws_handling & XDF_WHITESPACE_FLAGS; Same here. > @@ -214,6 +214,7 @@ struct diff_options { > } color_moved; > #define COLOR_MOVED_DEFAULT COLOR_MOVED_ZEBRA > #define COLOR_MOVED_MIN_ALNUM_COUNT 20 > + int color_moved_ws_handling; > }; Should the "int" be "unsigned"? I noticed that the flag-like xdl_opts is signed, but I think it's better for flags to be unsigned. Also, document what this stores. (And also, I would limit the bits.) > +test_expect_success 'only move detection ignores white spaces' ' > + git reset --hard && > + q_to_tab <<-\EOF >text.txt && > + a long line to exceed per-line minimum > + another long line to exceed per-line minimum > + original file > + EOF > + git add text.txt && > + git commit -m "add text" && > + q_to_tab <<-\EOF >text.txt && > + Qa long line to exceed per-line minimum > + Qanother long line to exceed per-line minimum > + new file > + EOF > + > + # Make sure we get a different diff using -w > + git diff --color --color-moved -w \ > + --color-moved-no-ignore-all-space \ > + --color-moved-no-ignore-space-change \ > + --color-moved-no-ignore-space-at-eol | > + grep -v "index" | > + test_decode_color >actual && > + q_to_tab <<-\EOF >expected && > + <BOLD>diff --git a/text.txt b/text.txt<RESET> > + <BOLD>--- a/text.txt<RESET> > + <BOLD>+++ b/text.txt<RESET> > + <CYAN>@@ -1,3 +1,3 @@<RESET> > + Qa long line to exceed per-line minimum<RESET> > + Qanother long line to exceed per-line minimum<RESET> > + <RED>-original file<RESET> > + <GREEN>+<RESET><GREEN>new file<RESET> > + EOF > + test_cmp expected actual && > + > + # And now ignoring white space only in the move detection > + git diff --color --color-moved \ > + --color-moved-ignore-all-space \ > + --color-moved-ignore-space-change \ > + --color-moved-ignore-space-at-eol | > + grep -v "index" | > + test_decode_color >actual && > + q_to_tab <<-\EOF >expected && > + <BOLD>diff --git a/text.txt b/text.txt<RESET> > + <BOLD>--- a/text.txt<RESET> > + <BOLD>+++ b/text.txt<RESET> > + <CYAN>@@ -1,3 +1,3 @@<RESET> > + <BOLD;MAGENTA>-a long line to exceed per-line minimum<RESET> > + <BOLD;MAGENTA>-another long line to exceed per-line minimum<RESET> > + <RED>-original file<RESET> > + <BOLD;YELLOW>+<RESET>Q<BOLD;YELLOW>a long line to exceed per-line minimum<RESET> > + <BOLD;YELLOW>+<RESET>Q<BOLD;YELLOW>another long line to exceed per-line minimum<RESET> > + <GREEN>+<RESET><GREEN>new file<RESET> > + EOF > + test_cmp expected actual > ' I know I suggested "per-line minimum", but I don't think there is one - I think we only have a per-block minimum. Maybe delete "per-line" in each of the lines.