From: Stefan Beller <sbeller@google.com> To: sbeller@google.com Cc: git@vger.kernel.org, gitster@pobox.com, jonathantanmy@google.com Subject: [PATCH 6/8] diff.c: decouple white space treatment from move detection algorithm Date: Thu, 17 May 2018 12:46:51 -0700 Message-ID: <20180517194653.48928-7-sbeller@google.com> (raw) In-Reply-To: <20180517194653.48928-1-sbeller@google.com> 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 whitespace 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 (namely, --color-moved-[no-]ignore-space-at-eol --color-moved-[no-]ignore-space-change --color-moved-[no-]ignore-all-space) that affect only the color of the output, and making the existing --ignore-space-at-eol, -b, and -w options no longer affect the color of the output. As we change the default, we'll adjust the tests. For now we do not infer any options to treat whitespaces in the move detection from the generic white space options given to diff. This can be tuned later to reasonable default. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/diff-options.txt | 13 +++++ diff.c | 19 ++++++- diff.h | 1 + t/t4015-diff-whitespace.sh | 90 +++++++++++++++++++++++++++++++--- 4 files changed, 114 insertions(+), 9 deletions(-) diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index bb9f1b7cd82..7b2527b9a19 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -292,6 +292,19 @@ dimmed_zebra:: blocks are considered interesting, the rest is uninteresting. -- +--color-moved-[no-]ignore-space-at-eol:: + Ignore changes in whitespace at EOL when performing the move + detection for --color-moved. +--color-moved-[no-]ignore-space-change:: + Ignore changes in amount of whitespace when performing the move + detection for --color-moved. This ignores whitespace + at line end, and considers all other sequences of one or + more whitespace characters to be equivalent. +--color-moved-[no-]ignore-all-space:: + Ignore whitespace when comparing lines when performing the move + detection for --color-moved. 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..b5819dd538f 100644 --- a/diff.c +++ b/diff.c @@ -717,10 +717,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 +730,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; @@ -4638,6 +4641,18 @@ int diff_opt_parse(struct diff_options *options, DIFF_XDL_SET(options, IGNORE_CR_AT_EOL); else if (!strcmp(arg, "--ignore-blank-lines")) DIFF_XDL_SET(options, IGNORE_BLANK_LINES); + else if (!strcmp(arg, "--color-moved-no-ignore-all-space")) + options->color_moved_ws_handling &= ~XDF_IGNORE_WHITESPACE; + else if (!strcmp(arg, "--color-moved-no-ignore-space-change")) + options->color_moved_ws_handling &= ~XDF_IGNORE_WHITESPACE_CHANGE; + else if (!strcmp(arg, "--color-moved-no-ignore-space-at-eol")) + options->color_moved_ws_handling &= ~XDF_IGNORE_WHITESPACE_AT_EOL; + else if (!strcmp(arg, "--color-moved-ignore-all-space")) + options->color_moved_ws_handling |= XDF_IGNORE_WHITESPACE; + else if (!strcmp(arg, "--color-moved-ignore-space-change")) + options->color_moved_ws_handling |= XDF_IGNORE_WHITESPACE_CHANGE; + else if (!strcmp(arg, "--color-moved-ignore-space-at-eol")) + options->color_moved_ws_handling |= XDF_IGNORE_WHITESPACE_AT_EOL; else if (!strcmp(arg, "--indent-heuristic")) DIFF_XDL_SET(options, INDENT_HEURISTIC); else if (!strcmp(arg, "--no-indent-heuristic")) 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 45091abb192..751fc478dde 100755 --- a/t/t4015-diff-whitespace.sh +++ b/t/t4015-diff-whitespace.sh @@ -1441,7 +1441,10 @@ test_expect_success 'move detection ignoring whitespace ' ' line 4 line 5 EOF - git diff HEAD --no-renames --color-moved --color | + git diff HEAD --no-renames --color-moved --color \ + --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 && cat <<-\EOF >expected && @@ -1465,7 +1468,10 @@ test_expect_success 'move detection ignoring whitespace ' ' EOF test_cmp expected actual && - git diff HEAD --no-renames -w --color-moved --color | + git diff HEAD --no-renames --color-moved --color \ + --color-moved-ignore-all-space \ + --color-moved-no-ignore-space-change \ + --color-moved-no-ignore-space-at-eol | grep -v "index" | test_decode_color >actual && cat <<-\EOF >expected && @@ -1505,7 +1511,10 @@ test_expect_success 'move detection ignoring whitespace changes' ' line 5 EOF - git diff HEAD --no-renames --color-moved --color | + git diff HEAD --no-renames --color-moved --color \ + --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 && cat <<-\EOF >expected && @@ -1529,7 +1538,10 @@ test_expect_success 'move detection ignoring whitespace changes' ' EOF test_cmp expected actual && - git diff HEAD --no-renames -b --color-moved --color | + git diff HEAD --no-renames --color-moved --color \ + --color-moved-no-ignore-all-space \ + --color-moved-no-ignore-space-at-eol \ + --color-moved-ignore-space-change | grep -v "index" | test_decode_color >actual && cat <<-\EOF >expected && @@ -1572,7 +1584,10 @@ test_expect_success 'move detection ignoring whitespace at eol' ' # avoid cluttering the output with complaints about our eol whitespace test_config core.whitespace -blank-at-eol && - git diff HEAD --no-renames --color-moved --color | + git diff HEAD --no-renames --color-moved --color \ + --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 && cat <<-\EOF >expected && @@ -1596,7 +1611,10 @@ 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 | + git diff HEAD --no-renames --color-moved --color \ + --color-moved-no-ignore-all-space \ + --color-moved-no-ignore-space-change \ + --color-moved-ignore-space-at-eol | grep -v "index" | test_decode_color >actual && cat <<-\EOF >expected && @@ -1768,7 +1786,65 @@ 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 +' + +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 ' test_done -- 2.17.0.582.gccdcbd54c44.dirty
next prev parent reply other threads:[~2018-05-17 19:47 UTC|newest] Thread overview: 95+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-05-17 6:01 What's cooking in git.git (May 2018, #02; Thu, 17) Junio C Hamano 2018-05-17 6:39 ` jk/branch-l-0-deprecation (was Re: What's cooking in git.git (May 2018, #02; Thu, 17)) Kaartic Sivaraam 2018-05-17 9:48 ` Ævar Arnfjörð Bjarmason 2018-05-17 11:00 ` Kaartic Sivaraam 2018-05-17 12:02 ` Ævar Arnfjörð Bjarmason 2018-05-17 13:36 ` Jeff King 2018-05-24 15:10 ` Kaartic Sivaraam 2018-05-24 19:22 ` Jeff King 2018-05-24 19:31 ` [PATCH] branch: issue "-l" deprecation warning after pager starts Jeff King 2018-05-25 1:55 ` Junio C Hamano 2018-05-25 2:40 ` Jeff King 2018-05-25 8:56 ` Junio C Hamano 2018-05-25 9:14 ` Junio C Hamano 2018-05-25 17:10 ` Jeff King 2018-05-26 2:37 ` Junio C Hamano 2018-05-25 21:00 ` [RFC PATCH 0/3] usage: prefix all lines in `vreportf()`, not just the first Martin Ågren 2018-05-25 21:00 ` [RFC PATCH 1/3] usage: extract `prefix_suffix_lines()` from `advise()` Martin Ågren 2018-05-28 8:27 ` Junio C Hamano 2018-05-28 18:40 ` Duy Nguyen 2018-05-29 21:33 ` Jeff King 2018-05-29 21:39 ` Jeff King 2018-05-30 1:42 ` Junio C Hamano 2018-05-30 6:00 ` Junio C Hamano 2018-05-30 10:26 ` Martin Ågren 2018-05-31 6:07 ` Jeff King 2018-05-25 21:00 ` [RFC PATCH 2/3] usage: prefix all lines in `vreportf()`, not just the first Martin Ågren 2018-05-28 9:25 ` Junio C Hamano 2018-05-28 18:45 ` Duy Nguyen 2018-05-28 21:45 ` Junio C Hamano 2018-05-29 4:49 ` Martin Ågren 2018-05-29 5:50 ` Junio C Hamano 2018-05-29 10:30 ` Martin Ågren 2018-05-29 12:08 ` Junio C Hamano 2018-05-29 15:50 ` Duy Nguyen 2018-05-30 10:19 ` Martin Ågren 2018-05-29 21:32 ` Jeff King 2018-05-30 10:20 ` Martin Ågren 2018-05-25 21:00 ` [RFC PATCH 3/3] usage: translate the "error: "-prefix and others Martin Ågren 2018-05-26 2:32 ` [PATCH] branch: issue "-l" deprecation warning after pager starts Junio C Hamano 2018-05-26 2:33 ` Junio C Hamano 2018-05-29 21:20 ` Jeff King 2018-05-29 21:21 ` Jeff King 2018-05-30 2:48 ` Junio C Hamano 2018-05-31 5:44 ` Jeff King 2018-05-26 19:39 ` Kaartic Sivaraam 2018-06-02 4:46 ` Duy Nguyen 2018-06-02 8:10 ` Jeff King 2018-05-26 18:45 ` Kaartic Sivaraam 2018-05-29 21:15 ` Jeff King 2018-05-30 2:52 ` Junio C Hamano 2018-05-31 5:51 ` Jeff King 2018-06-01 1:35 ` Junio C Hamano 2018-05-31 5:52 ` Kaartic.Sivaraam 2018-05-17 13:22 ` What's cooking in git.git (May 2018, #02; Thu, 17) Derrick Stolee 2018-05-17 18:20 ` Stefan Beller 2018-05-17 18:29 ` [PATCH 0/2] Reroll 2 last commits of sb/object-store-replace Stefan Beller 2018-05-17 18:29 ` [PATCH 1/2] object.c: free replace map in raw_object_store_clear Stefan Beller 2018-05-17 18:29 ` [PATCH 2/2] replace-object.c: remove the_repository from prepare_replace_object Stefan Beller 2018-05-17 18:40 ` [PATCH] merge-recursive: give notice when submodule commit gets fast-forwarded Stefan Beller 2018-05-18 19:43 ` [PATCH v2 0/1] rebased: inform about auto submodule ff Leif Middelschulte 2018-05-18 19:48 ` [PATCH v3 " Leif Middelschulte 2018-05-18 19:48 ` [PATCH 1/1] Inform about fast-forwarding of submodules during merge Leif Middelschulte 2018-05-18 21:25 ` Elijah Newren 2018-05-21 4:12 ` Junio C Hamano 2018-05-17 19:46 ` [PATCH 0/8] Reroll of sb/diff-color-move-more Stefan Beller 2018-05-17 19:46 ` [PATCH 1/8] xdiff/xdiff.h: remove unused flags Stefan Beller 2018-05-17 19:46 ` [PATCH 2/8] xdiff/xdiffi.c: remove unneeded function declarations Stefan Beller 2018-05-17 19:46 ` [PATCH 3/8] diff.c: do not pass diff options as keydata to hashmap Stefan Beller 2018-05-17 19:46 ` [PATCH 4/8] diff.c: adjust hash function signature to match hashmap expectation Stefan Beller 2018-05-17 19:46 ` [PATCH 5/8] diff.c: add a blocks mode for moved code detection Stefan Beller 2018-05-17 19:46 ` Stefan Beller [this message] 2018-05-18 4:00 ` [PATCH 6/8] diff.c: decouple white space treatment from move detection algorithm Simon Ruderich 2018-05-18 19:25 ` Stefan Beller 2018-05-17 19:46 ` [PATCH 7/8] diff.c: add --color-moved-ignore-space-delta option Stefan Beller 2018-05-17 19:46 ` [PATCH 8/8] diff: color-moved white space handling options imply color-moved Stefan Beller 2018-05-17 22:53 ` [PATCH 0/8] Reroll of sb/diff-color-move-more Jonathan Tan 2018-06-07 23:54 ` Jacob Keller 2018-05-17 22:36 ` What's cooking in git.git (May 2018, #02; Thu, 17) Junio C Hamano 2018-05-17 22:39 ` Stefan Beller 2018-05-17 22:56 ` Junio C Hamano 2018-05-17 22:58 ` Stefan Beller 2018-05-21 1:57 ` brian m. carlson 2018-05-21 17:36 ` Stefan Beller 2018-05-25 12:28 ` sb/submodule-move-nested breaks t7411 under GIT_FSMONITOR_TEST Ævar Arnfjörð Bjarmason 2018-05-25 17:27 ` Stefan Beller 2018-05-25 19:49 ` Stefan Beller 2018-09-06 12:31 ` Ævar Arnfjörð Bjarmason 2018-09-06 16:57 ` Stefan Beller 2018-09-06 19:03 ` Ben Peart 2018-09-06 20:14 ` Stefan Beller 2018-09-06 20:34 ` [PATCH] git-mv: allow submodules and fsmonitor to work together Stefan Beller 2018-09-10 15:58 ` Ben Peart 2018-09-10 16:29 ` [PATCH v1] " Ben Peart 2018-09-10 17:07 ` Stefan Beller 2018-09-10 19:38 ` Ben Peart
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20180517194653.48928-7-sbeller@google.com \ --to=sbeller@google.com \ --cc=git@vger.kernel.org \ --cc=gitster@pobox.com \ --cc=jonathantanmy@google.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
git@vger.kernel.org list mirror (unofficial, one of many) This inbox may be cloned and mirrored by anyone: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V1 git git/ https://public-inbox.org/git \ git@vger.kernel.org public-inbox-index git Example config snippet for mirrors. Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.io/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ code repositories for the project(s) associated with this inbox: https://80x24.org/mirrors/git.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git