On Fri, Aug 11, 2017 at 3:49 PM, Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote:
> Currently, MIN_BLOCK_LENGTH is only checked when diff encounters a line
> that does not belong to the current block. In particular, this means
> that MIN_BLOCK_LENGTH is not checked after all lines are encountered.
>
> Perform that check.
Thanks for spotting! This fix looks straightforward correct.
(Also thanks for factoring out the adjustment, I am tempted to
start a bike shedding discussion about its name, though. :P)
> Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx>
> ---
> diff.c | 29 ++++++++++++++++++++++-------
> t/t4015-diff-whitespace.sh | 35 +++++++++++++++++++++++++++++++++++
> 2 files changed, 57 insertions(+), 7 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 4965ffbc4..95620b130 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -858,6 +858,26 @@ static int shrink_potential_moved_blocks(struct moved_entry **pmb,
> return rp + 1;
> }
>
> +/*
> + * 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
> + * that block.
> + *
> + * The last block consists of the (n - block_length)'th line up to but not
> + * including the nth line.
> + */
> +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)
> + return;
> + for (i = 1; i < block_length + 1; i++)
> + o->emitted_symbols->buf[n - i].flags &= ~DIFF_SYMBOL_MOVED_LINE;
> +}
> +
> /* Find blocks of moved code, delegate actual coloring decision to helper */
> static void mark_color_as_moved(struct diff_options *o,
> struct hashmap *add_lines,
> @@ -893,13 +913,7 @@ static void mark_color_as_moved(struct diff_options *o,
> }
>
> if (!match) {
> - if (block_length < COLOR_MOVED_MIN_BLOCK_LENGTH &&
> - o->color_moved != COLOR_MOVED_PLAIN) {
> - for (i = 1; i < block_length + 1; i++) {
> - l = &o->emitted_symbols->buf[n - i];
> - l->flags &= ~DIFF_SYMBOL_MOVED_LINE;
> - }
> - }
> + adjust_last_block(o, n, block_length);
> pmb_nr = 0;
> block_length = 0;
> continue;
> @@ -941,6 +955,7 @@ static void mark_color_as_moved(struct diff_options *o,
> if (flipped_block)
> l->flags |= DIFF_SYMBOL_MOVED_LINE_ALT;
> }
> + adjust_last_block(o, n, block_length);
>
> free(pmb);
> }
> diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
> index c3b697411..6f7758e5c 100755
> --- a/t/t4015-diff-whitespace.sh
> +++ b/t/t4015-diff-whitespace.sh
> @@ -1382,6 +1382,41 @@ EOF
> test_cmp expected actual
> '
>
> +test_expect_success '--color-moved block at end of diff output respects MIN_BLOCK_LENGTH' '
> + git reset --hard &&
> + >bar &&
> + cat <<-\EOF >foo &&
> + irrelevant_line
> + line1
> + EOF
> + git add foo bar &&
> + git commit -m x &&
> +
> + cat <<-\EOF >bar &&
> + line1
> + EOF
> + cat <<-\EOF >foo &&
> + irrelevant_line
> + EOF
> +
> + git diff HEAD --color-moved=zebra --no-renames | grep -v "index" | test_decode_color >actual &&
> + cat >expected <<-\EOF &&
> + <BOLD>diff --git a/bar b/bar<RESET>
> + <BOLD>--- a/bar<RESET>
> + <BOLD>+++ b/bar<RESET>
> + <CYAN>@@ -0,0 +1 @@<RESET>
> + <GREEN>+<RESET><GREEN>line1<RESET>
> + <BOLD>diff --git a/foo b/foo<RESET>
> + <BOLD>--- a/foo<RESET>
> + <BOLD>+++ b/foo<RESET>
> + <CYAN>@@ -1,2 +1 @@<RESET>
> + irrelevant_line<RESET>
> + <RED>-line1<RESET>
> + EOF
> +
> + test_cmp expected actual
> +'
> +
> test_expect_success 'move detection with submodules' '
> test_create_repo bananas &&
> echo ripe >bananas/recipe &&
> --
> 2.14.0.434.g98096fd7a8-goog
>
