On 06/28, Stefan Beller wrote:
> In the original implementation of the move detection logic the choice for
> ignoring white space changes is the same for the move detection as it is
> for the regular diff. Some cases came up where different treatment would
> have been nice.
>
> Allow the user to specify that white space should be ignored differently
> during detection of moved lines than during generation of added and removed
> lines. This is done by providing analogs to the --ignore-space-at-eol,
> -b, and -w options by introducing the option --color-moved-ws=<modes>
> with the modes named "ignore-space-at-eol", "ignore-space-change" and
> "ignore-all-space", which is used only during the move detection phase.
>
> As we change the default, we'll adjust the tests.
>
> For now we do not infer any options to treat white spaces in the move
> detection from the generic white space options given to diff.
> This can be tuned later to reasonable default.
>
> As we plan on adding more white space related options in a later patch,
> that interferes with the current white space options, use a flag field
> and clamp it down to XDF_WHITESPACE_FLAGS, as that (a) allows to easily
> check at parse time if we give invalid combinations and (b) can reuse
> parts of this patch.
>
> By having the white space treatment in its own option, we'll also
> make it easier for a later patch to have an config option for
> spaces in the move detection.
>
> Signed-off-by: Stefan Beller <sbel...@google.com>
> Signed-off-by: Junio C Hamano <gits...@pobox.com>
> ---
> Documentation/diff-options.txt | 17 +++++++++
> diff.c | 39 +++++++++++++++++++--
> diff.h | 1 +
> t/t4015-diff-whitespace.sh | 64 +++++++++++++++++++++++++++++++---
> 4 files changed, 115 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index ba56169de31..80e29e39854 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -292,6 +292,23 @@ dimmed_zebra::
> blocks are considered interesting, the rest is uninteresting.
> --
>
> +--color-moved-ws=<modes>::
> + This configures how white spaces are ignored when performing the
> + move detection for `--color-moved`. These modes can be given
> + as a comma separated list:
> ++
> +--
> +ignore-space-at-eol::
> + Ignore changes in whitespace at EOL.
> +ignore-space-change::
> + Ignore changes in amount of whitespace. This ignores whitespace
> + at line end, and considers all other sequences of one or
> + more whitespace characters to be equivalent.
> +ignore-all-space::
> + Ignore whitespace when comparing lines. This ignores differences
> + even if one line has whitespace where the other line has none.
> +--
> +
> --word-diff[=<mode>]::
> Show a word diff, using the <mode> to delimit changed words.
> By default, words are delimited by whitespace; see
> diff --git a/diff.c b/diff.c
> index 95c51c0b7df..70eeb40c5fd 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -283,6 +283,36 @@ static int parse_color_moved(const char *arg)
> return error(_("color moved setting must be one of 'no',
> 'default', 'blocks', 'zebra', 'dimmed_zebra', 'plain'"));
> }
>
> +static int parse_color_moved_ws(const char *arg)
> +{
> + int ret = 0;
> + struct string_list l = STRING_LIST_INIT_DUP;
> + struct string_list_item *i;
> +
> + string_list_split(&l, arg, ',', -1);
> +
> + for_each_string_list_item(i, &l) {
> + struct strbuf sb = STRBUF_INIT;
> + strbuf_addstr(&sb, i->string);
> + strbuf_trim(&sb);
> +
> + if (!strcmp(sb.buf, "ignore-space-change"))
> + ret |= XDF_IGNORE_WHITESPACE_CHANGE;
> + else if (!strcmp(sb.buf, "ignore-space-at-eol"))
> + ret |= XDF_IGNORE_WHITESPACE_AT_EOL;
> + else if (!strcmp(sb.buf, "ignore-all-space"))
> + ret |= XDF_IGNORE_WHITESPACE;
> + else
> + error(_("ignoring unknown color-moved-ws mode '%s'"),
> sb.buf);
> +
> + strbuf_release(&sb);
> + }
> +
> + string_list_clear(&l, 0);
> +
> + return ret;
> +}
> +
> int git_diff_ui_config(const char *var, const char *value, void *cb)
> {
> if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
> @@ -717,10 +747,12 @@ static int moved_entry_cmp(const void
> *hashmap_cmp_fn_data,
> const struct diff_options *diffopt = hashmap_cmp_fn_data;
> const struct moved_entry *a = entry;
> const struct moved_entry *b = entry_or_key;
> + unsigned flags = diffopt->color_moved_ws_handling
> + & XDF_WHITESPACE_FLAGS;
>
> return !xdiff_compare_lines(a->es->line, a->es->len,
> b->es->line, b->es->len,
> - diffopt->xdl_opts);
> + flags);
> }
>
> static struct moved_entry *prepare_entry(struct diff_options *o,
> @@ -728,8 +760,9 @@ static struct moved_entry *prepare_entry(struct
> diff_options *o,
> {
> struct moved_entry *ret = xmalloc(sizeof(*ret));
> struct emitted_diff_symbol *l = &o->emitted_symbols->buf[line_no];
> + unsigned flags = o->color_moved_ws_handling & XDF_WHITESPACE_FLAGS;
>
> - ret->ent.hash = xdiff_hash_string(l->line, l->len, o->xdl_opts);
> + ret->ent.hash = xdiff_hash_string(l->line, l->len, flags);
> ret->es = l;
> ret->next_line = NULL;
>
> @@ -4710,6 +4743,8 @@ int diff_opt_parse(struct diff_options *options,
> if (cm < 0)
> die("bad --color-moved argument: %s", arg);
> options->color_moved = cm;
> + } else if (skip_prefix(arg, "--color-moved-ws=", &arg)) {
> + options->color_moved_ws_handling = parse_color_moved_ws(arg);
> } else if (skip_to_optional_arg_default(arg, "--color-words",
> &options->word_regex, NULL)) {
> options->use_color = 1;
> options->word_diff = DIFF_WORDS_COLOR;
> diff --git a/diff.h b/diff.h
> index 7bd4f182c33..de5dc680051 100644
> --- a/diff.h
> +++ b/diff.h
> @@ -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;
> };
>
> void diff_emit_submodule_del(struct diff_options *o, const char *line);
> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
> index e54529f026d..0c737a47cf8 100755
> --- a/t/t4015-diff-whitespace.sh
> +++ b/t/t4015-diff-whitespace.sh
> @@ -1460,7 +1460,8 @@ test_expect_success 'move detection ignoring whitespace
> ' '
> EOF
> test_cmp expected actual &&
>
> - git diff HEAD --no-renames -w --color-moved --color >actual.raw &&
> + git diff HEAD --no-renames --color-moved --color \
> + --color-moved-ws=ignore-all-space >actual.raw &&
> grep -v "index" actual.raw | test_decode_color >actual &&
> cat <<-\EOF >expected &&
> <BOLD>diff --git a/lines.txt b/lines.txt<RESET>
> @@ -1522,7 +1523,8 @@ test_expect_success 'move detection ignoring whitespace
> changes' '
> EOF
> test_cmp expected actual &&
>
> - git diff HEAD --no-renames -b --color-moved --color >actual.raw &&
> + git diff HEAD --no-renames --color-moved --color \
> + --color-moved-ws=ignore-space-change >actual.raw &&
> grep -v "index" actual.raw | test_decode_color >actual &&
> cat <<-\EOF >expected &&
> <BOLD>diff --git a/lines.txt b/lines.txt<RESET>
> @@ -1587,7 +1589,8 @@ test_expect_success 'move detection ignoring whitespace
> at eol' '
> EOF
> test_cmp expected actual &&
>
> - git diff HEAD --no-renames --ignore-space-at-eol --color-moved --color
> >actual.raw &&
> + git diff HEAD --no-renames --color-moved --color \
> + --color-moved-ws=ignore-space-at-eol >actual.raw &&
> grep -v "index" actual.raw | test_decode_color >actual &&
> cat <<-\EOF >expected &&
> <BOLD>diff --git a/lines.txt b/lines.txt<RESET>
> @@ -1757,7 +1760,60 @@ test_expect_success 'move detection with submodules' '
>
> # nor did we mess with it another way
> git diff --submodule=diff --color | test_decode_color >expect &&
> - test_cmp expect decoded_actual
> + test_cmp expect decoded_actual &&
> + rm -rf bananas &&
> + git submodule deinit bananas
Maybe you want to use a test_when_finished call for this instead of
doing this at the end of the test? I guess this comes up as a point of
debate: Do you have each test clean up after itself or do you ensure
that subsequent tests makes sure its env is ready before testing.
Anyway this is just a suggestion.
> +'
> +
> +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 |
> + 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-ws=ignore-all-space,ignore-space-change,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
> '
>
> test_done
> --
> 2.18.0.399.gad0ab374a1-goog
>
--
Brandon Williams
