On Mon, 2 Apr 2018 15:48:54 -0700
Stefan Beller <sbel...@google.com> wrote:
> +struct ws_delta {
> + int deltachars;
> + char firstchar;
> +};
I'll just make some overall design comments.
Shouldn't this be a string of characters (or a char* and len) and
whether it was added or removed? If you're only checking the first
character, this would not work if the other characters were different.
> @@ -717,10 +752,20 @@ 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 & XDF_WHITESPACE_FLAGS;
> +
> + if (diffopt->color_moved & COLOR_MOVED_DELTA_WHITESPACES)
> + /*
> + * As there is not specific white space config given,
> + * we'd need to check for a new block, so ignore all
> + * white space. The setup of the white space
> + * configuration for the next block is done else where
> + */
> + flags |= XDF_IGNORE_WHITESPACE;
>
> return !xdiff_compare_lines(a->es->line, a->es->len,
> b->es->line, b->es->len,
> - diffopt->color_moved &
> XDF_WHITESPACE_FLAGS);
> + flags);
> }
I think we should just prohibit combining this with any of the
whitespace ignoring flags except for the space-at-eol one. They seem to
contradict anyway.
> +test_expect_success 'compare whitespace delta across moved blocks' '
> +
> + git reset --hard &&
> + q_to_tab <<-\EOF >text.txt &&
> + QIndented
> + QText
> + Qacross
> + Qfive
> + Qlines
> + QBut!
> + Qthis
> + QQone
> + Qline
> + QQdid
> + Qnot
> + QQadjust
> + EOF
Do we need 5 lines? I thought 2 would suffice. (It's the ALNUM_COUNT
that matters, as far as I know.) This makes it hard to see that the
"But!" line is the one that counts.
> +test_expect_success 'compare whitespace delta across moved blocks with
> multiple indentation levels' '
> + # alternative: "python programmers would love the move detection, too"
> +
> + git reset --hard &&
> + q_to_tab <<-\EOF >test.py &&
> + class test:
> + Qdef f(x):
> + QQ"""
> + QQA simple python function
> + QQthat returns the square of a number
> + QQAlthough it may not be pythonic
> + QQ"""
> + QQdef g(x):
> + QQQ"""
> + QQQNested function that returns the same number
> + QQQWe just multiply by 1.0
> + QQQso we can write a comment about it
> + QQQas we need longer blocks
> + QQQ"""
> + QQQreturn 1.0 * x
> + QQ# Another comment for f(x)
> + QQ# also spanning multiple lines
> + QQ# to make a block
> + QQreturn g(x) * g(x)
> + Qdef h(x):
> + QQ# Another function unrelated to the previous
> + QQ# but building a block,
> + QQ# long enough to call it a block
> + QQreturn x * 1.0
> + EOF
If the objective it just to show that the functions f and g are treated
as one unit despite their lines being of multiple indentation levels,
the test file could be much shorter.
