On Tue, 24 Apr 2018 14:03:29 -0700
Stefan Beller <sbel...@google.com> 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.
