summaryrefslogtreecommitdiff
path: root/diff.c
diff options
context:
space:
mode:
authorJonathan Tan <jonathantanmy@google.com>2017-08-15 18:27:39 -0700
committerJunio C Hamano <gitster@pobox.com>2017-08-16 11:44:00 -0700
commitf0b8fb6e591b50b72b921f2c4cf120ebd284f510 (patch)
treeea681638a57ff4c00e3a9525fa85cfa99e7b0bde /diff.c
parent09153277f8948d3ed8cb6712f135a9caeaa006f6 (diff)
downloadgit-f0b8fb6e591b50b72b921f2c4cf120ebd284f510.tar.gz
diff: define block by number of alphanumeric charsjt/diff-color-move-fix
The existing behavior of diff --color-moved=zebra does not define the minimum size of a block at all, instead relying on a heuristic applied later to filter out sets of adjacent moved lines that are shorter than 3 lines long. This can be confusing, because a block could thus be colored as moved at the source but not at the destination (or vice versa), depending on its neighbors. Instead, teach diff that the minimum size of a block is 20 alphanumeric characters, the same heuristic used by "git blame". This allows diff to still exclude uninteresting lines appearing on their own (such as those solely consisting of one or a few closing braces), as was the intention of the adjacent-moved-line heuristic. This requires a change in some tests in that some of their lines are no longer considered to be part of a block, because they are too short. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'diff.c')
-rw-r--r--diff.c28
1 files changed, 22 insertions, 6 deletions
diff --git a/diff.c b/diff.c
index f598d8a3a0..c50fcc7ea5 100644
--- a/diff.c
+++ b/diff.c
@@ -864,19 +864,31 @@ static int shrink_potential_moved_blocks(struct moved_entry **pmb,
/*
* If o->color_moved is COLOR_MOVED_PLAIN, this function does nothing.
*
- * Otherwise, if the last block has fewer lines than
- * COLOR_MOVED_MIN_BLOCK_LENGTH, unset DIFF_SYMBOL_MOVED_LINE on all lines in
+ * Otherwise, if the last block has fewer alphanumeric characters than
+ * COLOR_MOVED_MIN_ALNUM_COUNT, unset DIFF_SYMBOL_MOVED_LINE on all lines in
* that block.
*
* The last block consists of the (n - block_length)'th line up to but not
* including the nth line.
+ *
+ * NEEDSWORK: This uses the same heuristic as blame_entry_score() in blame.c.
+ * Think of a way to unify them.
*/
static void adjust_last_block(struct diff_options *o, int n, int block_length)
{
- int i;
- if (block_length >= COLOR_MOVED_MIN_BLOCK_LENGTH ||
- o->color_moved == COLOR_MOVED_PLAIN)
+ int i, alnum_count = 0;
+ if (o->color_moved == COLOR_MOVED_PLAIN)
return;
+ for (i = 1; i < block_length + 1; i++) {
+ const char *c = o->emitted_symbols->buf[n - i].line;
+ for (; *c; c++) {
+ if (!isalnum(*c))
+ continue;
+ alnum_count++;
+ if (alnum_count >= COLOR_MOVED_MIN_ALNUM_COUNT)
+ return;
+ }
+ }
for (i = 1; i < block_length + 1; i++)
o->emitted_symbols->buf[n - i].flags &= ~DIFF_SYMBOL_MOVED_LINE;
}
@@ -923,7 +935,6 @@ static void mark_color_as_moved(struct diff_options *o,
}
l->flags |= DIFF_SYMBOL_MOVED_LINE;
- block_length++;
if (o->color_moved == COLOR_MOVED_PLAIN)
continue;
@@ -953,8 +964,13 @@ static void mark_color_as_moved(struct diff_options *o,
}
flipped_block = (flipped_block + 1) % 2;
+
+ adjust_last_block(o, n, block_length);
+ block_length = 0;
}
+ block_length++;
+
if (flipped_block)
l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT;
}