It is useful to have moved lines colored, but there are annoying corner
cases, such as a single line moved, that is very common. For example
in a typical patch of C code, we have closing braces that end statement
blocks or functions.
While it is technically true that these lines are moved as they show up
elsewhere, it is harmful for the review as the reviewers attention is
drawn to such a minor side annoyance.
One of the first solutions considered, started off by these hypothesis':
(a) The more blocks of the same code we have, the less interesting it is.
(b) The shorter a block of moved code is the less need of markup there
is for review.
Introduce a heuristic which drops any potential moved blocks if their
length is shorter than the number of potential moved blocks.
This heuristic was chosen as it is agnostic of the content (in other
languages or contents to manage, we may have longer lines, e.g. in
shell the closing of a condition is already 2 characters. Thinking
about Latex documents tracked in Git, there can also be some
boilerplate code with lots of characters) while taking both
hypothesis' into account. An alternative considered was the number
of non-whitespace characters in a line for example.
Thinking further about this, a linear relation between number of moved
blocks and number of lines of code seems like a bad idea to start with.
So let's start with a simpler solution of hardcoding the number of lines
to 3.
Note, that the length is applied across all blocks to find the 'lonely'
blocks that pollute new code, but do not interfere with a permutated
block where each permutation has less lines than 3.
Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
---
diff.c | 11 ++++++++++-
diff.h | 1 +
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index 015c854530..1d93e98e3a 100644
--- a/diff.c
+++ b/diff.c
@@ -853,7 +853,8 @@ static void mark_color_as_moved(struct diff_options *o,
{
struct moved_entry **pmb = NULL; /* potentially moved blocks */
int pmb_nr = 0, pmb_alloc = 0;
- int n, flipped_block = 1;
+ int n, flipped_block = 1, block_length = 0;
+
for (n = 0; n < o->emitted_symbols->nr; n++) {
struct hashmap *hm = NULL;
@@ -880,11 +881,19 @@ static void mark_color_as_moved(struct diff_options *o,
}
if (!match) {
+ if (block_length < COLOR_MOVED_MIN_BLOCK_LENGTH) {
+ for (i = 0; i < block_length + 1; i++) {
+ l = &o->emitted_symbols->buf[n - i];
+ l->flags &= ~DIFF_SYMBOL_MOVED_LINE;
+ }
+ }
pmb_nr = 0;
+ block_length = 0;
continue;
}
l->flags |= DIFF_SYMBOL_MOVED_LINE;
+ block_length++;
if (o->color_moved == COLOR_MOVED_PLAIN)
continue;
diff --git a/diff.h b/diff.h
index 9298d211d7..cc1224a93b 100644
--- a/diff.h
+++ b/diff.h
@@ -195,6 +195,7 @@ struct diff_options {
COLOR_MOVED_DEFAULT = 2,
COLOR_MOVED_ZEBRA_DIM = 3,
} color_moved;
+ #define COLOR_MOVED_MIN_BLOCK_LENGTH 3
};
void diff_emit_submodule_del(struct diff_options *o, const char *line);
--
2.13.0.31.g9b732c453e
