Re: [PATCH v4 7/9] diff.c: decouple white space treatment from move detection algorithm
On 06/28, Stefan Beller wrote:
> 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 white space 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 by introducing the option --color-moved-ws=
> with the modes named "ignore-space-at-eol", "ignore-space-change" and
> "ignore-all-space", which is used only during the move detection phase.
>
> As we change the default, we'll adjust the tests.
>
> For now we do not infer any options to treat white spaces in the move
> detection from the generic white space options given to diff.
> This can be tuned later to reasonable default.
>
> As we plan on adding more white space related options in a later patch,
> that interferes with the current white space options, use a flag field
> and clamp it down to XDF_WHITESPACE_FLAGS, as that (a) allows to easily
> check at parse time if we give invalid combinations and (b) can reuse
> parts of this patch.
>
> By having the white space treatment in its own option, we'll also
> make it easier for a later patch to have an config option for
> spaces in the move detection.
>
> Signed-off-by: Stefan Beller
> Signed-off-by: Junio C Hamano
> ---
> Documentation/diff-options.txt | 17 +
> diff.c | 39 +++--
> diff.h | 1 +
> t/t4015-diff-whitespace.sh | 64 +++---
> 4 files changed, 115 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index ba56169de31..80e29e39854 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -292,6 +292,23 @@ dimmed_zebra::
> blocks are considered interesting, the rest is uninteresting.
> --
>
> +--color-moved-ws=::
> + This configures how white spaces are ignored when performing the
> + move detection for `--color-moved`. These modes can be given
> + as a comma separated list:
> ++
> +--
> +ignore-space-at-eol::
> + Ignore changes in whitespace at EOL.
> +ignore-space-change::
> + Ignore changes in amount of whitespace. This ignores whitespace
> + at line end, and considers all other sequences of one or
> + more whitespace characters to be equivalent.
> +ignore-all-space::
> + Ignore whitespace when comparing lines. This ignores differences
> + even if one line has whitespace where the other line has none.
> +--
> +
> --word-diff[=]::
> Show a word diff, using the to delimit changed words.
> By default, words are delimited by whitespace; see
> diff --git a/diff.c b/diff.c
> index 95c51c0b7df..70eeb40c5fd 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -283,6 +283,36 @@ static int parse_color_moved(const char *arg)
> return error(_("color moved setting must be one of 'no',
> 'default', 'blocks', 'zebra', 'dimmed_zebra', 'plain'"));
> }
>
> +static int parse_color_moved_ws(const char *arg)
> +{
> + int ret = 0;
> + struct string_list l = STRING_LIST_INIT_DUP;
> + struct string_list_item *i;
> +
> + string_list_split(, arg, ',', -1);
> +
> + for_each_string_list_item(i, ) {
> + struct strbuf sb = STRBUF_INIT;
> + strbuf_addstr(, i->string);
> + strbuf_trim();
> +
> + if (!strcmp(sb.buf, "ignore-space-change"))
> + ret |= XDF_IGNORE_WHITESPACE_CHANGE;
> + else if (!strcmp(sb.buf, "ignore-space-at-eol"))
> + ret |= XDF_IGNORE_WHITESPACE_AT_EOL;
> + else if (!strcmp(sb.buf, "ignore-all-space"))
> + ret |= XDF_IGNORE_WHITESPACE;
> + else
> + error(_("ignoring unknown color-moved-ws mode '%s'"),
> sb.buf);
> +
> + strbuf_release();
> + }
> +
> + string_list_clear(, 0);
> +
> + return ret;
> +}
> +
> int git_diff_ui_config(const char *var, const char *value, void *cb)
> {
> if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
> @@ -717,10 +747,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
[PATCH v4 7/9] diff.c: decouple white space treatment from move detection algorithm
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 white space 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 by introducing the option --color-moved-ws=
with the modes named "ignore-space-at-eol", "ignore-space-change" and
"ignore-all-space", which is used only during the move detection phase.
As we change the default, we'll adjust the tests.
For now we do not infer any options to treat white spaces in the move
detection from the generic white space options given to diff.
This can be tuned later to reasonable default.
As we plan on adding more white space related options in a later patch,
that interferes with the current white space options, use a flag field
and clamp it down to XDF_WHITESPACE_FLAGS, as that (a) allows to easily
check at parse time if we give invalid combinations and (b) can reuse
parts of this patch.
By having the white space treatment in its own option, we'll also
make it easier for a later patch to have an config option for
spaces in the move detection.
Signed-off-by: Stefan Beller
Signed-off-by: Junio C Hamano
---
Documentation/diff-options.txt | 17 +
diff.c | 39 +++--
diff.h | 1 +
t/t4015-diff-whitespace.sh | 64 +++---
4 files changed, 115 insertions(+), 6 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index ba56169de31..80e29e39854 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -292,6 +292,23 @@ dimmed_zebra::
blocks are considered interesting, the rest is uninteresting.
--
+--color-moved-ws=::
+ This configures how white spaces are ignored when performing the
+ move detection for `--color-moved`. These modes can be given
+ as a comma separated list:
++
+--
+ignore-space-at-eol::
+ Ignore changes in whitespace at EOL.
+ignore-space-change::
+ Ignore changes in amount of whitespace. This ignores whitespace
+ at line end, and considers all other sequences of one or
+ more whitespace characters to be equivalent.
+ignore-all-space::
+ Ignore whitespace when comparing lines. This ignores differences
+ even if one line has whitespace where the other line has none.
+--
+
--word-diff[=]::
Show a word diff, using the to delimit changed words.
By default, words are delimited by whitespace; see
diff --git a/diff.c b/diff.c
index 95c51c0b7df..70eeb40c5fd 100644
--- a/diff.c
+++ b/diff.c
@@ -283,6 +283,36 @@ static int parse_color_moved(const char *arg)
return error(_("color moved setting must be one of 'no',
'default', 'blocks', 'zebra', 'dimmed_zebra', 'plain'"));
}
+static int parse_color_moved_ws(const char *arg)
+{
+ int ret = 0;
+ struct string_list l = STRING_LIST_INIT_DUP;
+ struct string_list_item *i;
+
+ string_list_split(, arg, ',', -1);
+
+ for_each_string_list_item(i, ) {
+ struct strbuf sb = STRBUF_INIT;
+ strbuf_addstr(, i->string);
+ strbuf_trim();
+
+ if (!strcmp(sb.buf, "ignore-space-change"))
+ ret |= XDF_IGNORE_WHITESPACE_CHANGE;
+ else if (!strcmp(sb.buf, "ignore-space-at-eol"))
+ ret |= XDF_IGNORE_WHITESPACE_AT_EOL;
+ else if (!strcmp(sb.buf, "ignore-all-space"))
+ ret |= XDF_IGNORE_WHITESPACE;
+ else
+ error(_("ignoring unknown color-moved-ws mode '%s'"),
sb.buf);
+
+ strbuf_release();
+ }
+
+ string_list_clear(, 0);
+
+ return ret;
+}
+
int git_diff_ui_config(const char *var, const char *value, void *cb)
{
if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
@@ -717,10 +747,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 +760,9 @@ static struct moved_entry *prepare_entry(struct
diff_options *o,
{
struct moved_entry *ret = xmalloc(sizeof(*ret));
