diff.c: decouple white space treatment from move detection algorithm

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 <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Stefan Beller committed Jul 16, 2018 at 16:05 UTC b3095712f9d81572c3658d579e50db907b72f46d
4 files changed +113 -6
Documentation/diff-options.txt
+17
@@ -292,6 +292,23 @@ dimmed_zebra::
292 blocks are considered interesting, the rest is uninteresting.
293 --
294
295 +--color-moved-ws=<modes>::
296 + This configures how white spaces are ignored when performing the
297 + move detection for `--color-moved`. These modes can be given
298 + as a comma separated list:
299 ++
300 +--
301 +ignore-space-at-eol::
302 + Ignore changes in whitespace at EOL.
303 +ignore-space-change::
304 + Ignore changes in amount of whitespace. This ignores whitespace
305 + at line end, and considers all other sequences of one or
306 + more whitespace characters to be equivalent.
307 +ignore-all-space::
308 + Ignore whitespace when comparing lines. This ignores differences
309 + even if one line has whitespace where the other line has none.
310 +--
311 +
312 --word-diff[=<mode>]::
313 Show a word diff, using the <mode> to delimit changed words.
314 By default, words are delimited by whitespace; see
diff.c
+37 -2
@@ -283,6 +283,36 @@ static int parse_color_moved(const char *arg)
283 return error(_("color moved setting must be one of 'no', 'default', 'blocks', 'zebra', 'dimmed_zebra', 'plain'"));
284 }
285
286 +static int parse_color_moved_ws(const char *arg)
287 +{
288 + int ret = 0;
289 + struct string_list l = STRING_LIST_INIT_DUP;
290 + struct string_list_item *i;
291 +
292 + string_list_split(&l, arg, ',', -1);
293 +
294 + for_each_string_list_item(i, &l) {
295 + struct strbuf sb = STRBUF_INIT;
296 + strbuf_addstr(&sb, i->string);
297 + strbuf_trim(&sb);
298 +
299 + if (!strcmp(sb.buf, "ignore-space-change"))
300 + ret |= XDF_IGNORE_WHITESPACE_CHANGE;
301 + else if (!strcmp(sb.buf, "ignore-space-at-eol"))
302 + ret |= XDF_IGNORE_WHITESPACE_AT_EOL;
303 + else if (!strcmp(sb.buf, "ignore-all-space"))
304 + ret |= XDF_IGNORE_WHITESPACE;
305 + else
306 + error(_("ignoring unknown color-moved-ws mode '%s'"), sb.buf);
307 +
308 + strbuf_release(&sb);
309 + }
310 +
311 + string_list_clear(&l, 0);
312 +
313 + return ret;
314 +}
315 +
316 int git_diff_ui_config(const char *var, const char *value, void *cb)
317 {
318 if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
@@ -717,10 +747,12 @@ static int moved_entry_cmp(const void *hashmap_cmp_fn_data,
747 const struct diff_options *diffopt = hashmap_cmp_fn_data;
748 const struct moved_entry *a = entry;
749 const struct moved_entry *b = entry_or_key;
750 + unsigned flags = diffopt->color_moved_ws_handling
751 + & XDF_WHITESPACE_FLAGS;
752
753 return !xdiff_compare_lines(a->es->line, a->es->len,
754 b->es->line, b->es->len,
723 - diffopt->xdl_opts);
755 + flags);
756 }
757
758 static struct moved_entry *prepare_entry(struct diff_options *o,
@@ -728,8 +760,9 @@ static struct moved_entry *prepare_entry(struct diff_options *o,
760 {
761 struct moved_entry *ret = xmalloc(sizeof(*ret));
762 struct emitted_diff_symbol *l = &o->emitted_symbols->buf[line_no];
763 + unsigned flags = o->color_moved_ws_handling & XDF_WHITESPACE_FLAGS;
764
732 - ret->ent.hash = xdiff_hash_string(l->line, l->len, o->xdl_opts);
765 + ret->ent.hash = xdiff_hash_string(l->line, l->len, flags);
766 ret->es = l;
767 ret->next_line = NULL;
768
@@ -4710,6 +4743,8 @@ int diff_opt_parse(struct diff_options *options,
4743 if (cm < 0)
4744 die("bad --color-moved argument: %s", arg);
4745 options->color_moved = cm;
4746 + } else if (skip_prefix(arg, "--color-moved-ws=", &arg)) {
4747 + options->color_moved_ws_handling = parse_color_moved_ws(arg);
4748 } else if (skip_to_optional_arg_default(arg, "--color-words", &options->word_regex, NULL)) {
4749 options->use_color = 1;
4750 options->word_diff = DIFF_WORDS_COLOR;
diff.h
+1
@@ -214,6 +214,7 @@ struct diff_options {
214 } color_moved;
215 #define COLOR_MOVED_DEFAULT COLOR_MOVED_ZEBRA
216 #define COLOR_MOVED_MIN_ALNUM_COUNT 20
217 + int color_moved_ws_handling;
218 };
219
220 void diff_emit_submodule_del(struct diff_options *o, const char *line);
t/t4015-diff-whitespace.sh
+58 -4
@@ -1460,7 +1460,8 @@ test_expect_success 'move detection ignoring whitespace ' '
1460 EOF
1461 test_cmp expected actual &&
1462
1463 - git diff HEAD --no-renames -w --color-moved --color >actual.raw &&
1463 + git diff HEAD --no-renames --color-moved --color \
1464 + --color-moved-ws=ignore-all-space >actual.raw &&
1465 grep -v "index" actual.raw | test_decode_color >actual &&
1466 cat <<-\EOF >expected &&
1467 <BOLD>diff --git a/lines.txt b/lines.txt<RESET>
@@ -1522,7 +1523,8 @@ test_expect_success 'move detection ignoring whitespace changes' '
1523 EOF
1524 test_cmp expected actual &&
1525
1525 - git diff HEAD --no-renames -b --color-moved --color >actual.raw &&
1526 + git diff HEAD --no-renames --color-moved --color \
1527 + --color-moved-ws=ignore-space-change >actual.raw &&
1528 grep -v "index" actual.raw | test_decode_color >actual &&
1529 cat <<-\EOF >expected &&
1530 <BOLD>diff --git a/lines.txt b/lines.txt<RESET>
@@ -1587,7 +1589,8 @@ test_expect_success 'move detection ignoring whitespace at eol' '
1589 EOF
1590 test_cmp expected actual &&
1591
1590 - git diff HEAD --no-renames --ignore-space-at-eol --color-moved --color >actual.raw &&
1592 + git diff HEAD --no-renames --color-moved --color \
1593 + --color-moved-ws=ignore-space-at-eol >actual.raw &&
1594 grep -v "index" actual.raw | test_decode_color >actual &&
1595 cat <<-\EOF >expected &&
1596 <BOLD>diff --git a/lines.txt b/lines.txt<RESET>
@@ -1757,7 +1760,58 @@ test_expect_success 'move detection with submodules' '
1760
1761 # nor did we mess with it another way
1762 git diff --submodule=diff --color | test_decode_color >expect &&
1760 - test_cmp expect decoded_actual
1763 + test_cmp expect decoded_actual &&
1764 + rm -rf bananas &&
1765 + git submodule deinit bananas
1766 +'
1767 +
1768 +test_expect_success 'only move detection ignores white spaces' '
1769 + git reset --hard &&
1770 + q_to_tab <<-\EOF >text.txt &&
1771 + a long line to exceed per-line minimum
1772 + another long line to exceed per-line minimum
1773 + original file
1774 + EOF
1775 + git add text.txt &&
1776 + git commit -m "add text" &&
1777 + q_to_tab <<-\EOF >text.txt &&
1778 + Qa long line to exceed per-line minimum
1779 + Qanother long line to exceed per-line minimum
1780 + new file
1781 + EOF
1782 +
1783 + # Make sure we get a different diff using -w
1784 + git diff --color --color-moved -w >actual.raw &&
1785 + grep -v "index" actual.raw | test_decode_color >actual &&
1786 + q_to_tab <<-\EOF >expected &&
1787 + <BOLD>diff --git a/text.txt b/text.txt<RESET>
1788 + <BOLD>--- a/text.txt<RESET>
1789 + <BOLD>+++ b/text.txt<RESET>
1790 + <CYAN>@@ -1,3 +1,3 @@<RESET>
1791 + Qa long line to exceed per-line minimum<RESET>
1792 + Qanother long line to exceed per-line minimum<RESET>
1793 + <RED>-original file<RESET>
1794 + <GREEN>+<RESET><GREEN>new file<RESET>
1795 + EOF
1796 + test_cmp expected actual &&
1797 +
1798 + # And now ignoring white space only in the move detection
1799 + git diff --color --color-moved \
1800 + --color-moved-ws=ignore-all-space,ignore-space-change,ignore-space-at-eol >actual.raw &&
1801 + grep -v "index" actual.raw | test_decode_color >actual &&
1802 + q_to_tab <<-\EOF >expected &&
1803 + <BOLD>diff --git a/text.txt b/text.txt<RESET>
1804 + <BOLD>--- a/text.txt<RESET>
1805 + <BOLD>+++ b/text.txt<RESET>
1806 + <CYAN>@@ -1,3 +1,3 @@<RESET>
1807 + <BOLD;MAGENTA>-a long line to exceed per-line minimum<RESET>
1808 + <BOLD;MAGENTA>-another long line to exceed per-line minimum<RESET>
1809 + <RED>-original file<RESET>
1810 + <BOLD;YELLOW>+<RESET>Q<BOLD;YELLOW>a long line to exceed per-line minimum<RESET>
1811 + <BOLD;YELLOW>+<RESET>Q<BOLD;YELLOW>another long line to exceed per-line minimum<RESET>
1812 + <GREEN>+<RESET><GREEN>new file<RESET>
1813 + EOF
1814 + test_cmp expected actual
1815 '
1816
1817 test_done