git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 22/26] diff.c: color moved lines differently

From
Stefan Beller <sbeller@google.com>
Date
Jun 20, 2017, 02:48 UTC
Message-ID
<20170620024816.20021-23-sbeller@google.com>
In-Reply-To
<20170620024816.20021-1-sbeller@google.com>

When a patch consists mostly of moving blocks of code around, it can be quite tedious to ensure that the blocks are moved verbatim, and not undesirably modified in the move. To that end, color blocks that are moved within the same patch differently. For example (OM, del, add, and NM are different colors):

    [OM]  -void sensitive_stuff(void)
    [OM]  -{
    [OM]  -        if (!is_authorized_user())
    [OM]  -                die("unauthorized");
    [OM]  -        sensitive_stuff(spanning,
    [OM]  -                        multiple,
    [OM]  -                        lines);
    [OM]  -}
           void another_function()
           {
    [del] -        printf("foo");
    [add] +        printf("bar");
           }
    [NM]  +void sensitive_stuff(void)
    [NM]  +{
    [NM]  +        if (!is_authorized_user())
    [NM]  +                die("unauthorized");
    [NM]  +        sensitive_stuff(spanning,
    [NM]  +                        multiple,
    [NM]  +                        lines);
    [NM]  +}

However adjacent blocks may be problematic. For example, in this potentially malicious patch, the swapping of blocks can be spotted:

    [OM]  -void sensitive_stuff(void)
    [OM]  -{
    [OMA] -        if (!is_authorized_user())
    [OMA] -                die("unauthorized");
    [OM]  -        sensitive_stuff(spanning,
    [OM]  -                        multiple,
    [OM]  -                        lines);
    [OMA] -}
           void another_function()
           {
    [del] -        printf("foo");
    [add] +        printf("bar");
           }
    [NM]  +void sensitive_stuff(void)
    [NM]  +{
    [NMA] +        sensitive_stuff(spanning,
    [NMA] +                        multiple,
    [NMA] +                        lines);
    [NM]  +        if (!is_authorized_user())
    [NM]  +                die("unauthorized");
    [NMA] +}

If the moved code is larger, it is easier to hide some permutation in the code, which is why some alternative coloring is needed.

This patch implements the first mode:
* basic alternating 'Zebra' mode
  This conveys all information needed to the user.  Defer customization to
  later patches.

First I implemented an alternative design, which would try to fingerprint a line by its neighbors to detect if we are in a block or at the boundary. This idea iss error prone as it inspected each line and its neighboring lines to determine if the line was (a) moved and (b) if was deep inside a hunk by having matching neighboring lines. This is unreliable as the we can construct hunks which have equal neighbors that just exceed the number of lines inspected. (Think of 'AXYZBXYZCXYZD..' with each letter as a line, that is permutated to AXYZCXYZBXYZD..').

Instead this provides a dynamic programming greedy algorithm that finds the largest moved hunk and then has several modes on highlighting bounds.

A note on the options '--submodule=diff' and '--color-words/--word-diff': In the conversion to use emit_line in the prior patches both submodules as well as word diff output carefully chose to call emit_line with sign=0. All output with sign=0 is ignored for move detection purposes in this patch, such that no weird looking output will be generated for these cases. This leads to another thought: We could pass on '--color-moved' to submodules such that they color up moved lines for themselves. If we'd do so only line moves within a repository boundary are marked up.

Helped-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
 diff.c                     | 304 ++++++++++++++++++++++++++++++++++++++++++---
 diff.h                     |  10 +-
 t/t4015-diff-whitespace.sh | 196 +++++++++++++++++++++++++++++
 3 files changed, 495 insertions(+), 15 deletions(-)
diff --git a/diff.c b/diff.c
index 35b5924ff2..20c1f9b99f 100644
--- a/diff.c
+++ b/diff.c
@@ -15,6 +15,7 @@
 #include "userdiff.h"
 #include "submodule-config.h"
 #include "submodule.h"
+#include "hashmap.h"
 #include "ll-merge.h"
 #include "string-list.h"
 #include "argv-array.h"
@@ -31,6 +32,7 @@ static int diff_indent_heuristic; /* experimental */
 static int diff_rename_limit_default = 400;
 static int diff_suppress_blank_empty;
 static int diff_use_color_default = -1;
+static int diff_color_moved_default;
 static int diff_context_default = 3;
 static int diff_interhunk_context_default;
 static const char *diff_word_regex_cfg;
@@ -55,6 +57,10 @@ static char diff_colors[][COLOR_MAXLEN] = {
 	GIT_COLOR_YELLOW,	/* COMMIT */
 	GIT_COLOR_BG_RED,	/* WHITESPACE */
 	GIT_COLOR_NORMAL,	/* FUNCINFO */
+	GIT_COLOR_MAGENTA,	/* OLD_MOVED */
+	GIT_COLOR_BLUE,		/* OLD_MOVED ALTERNATIVE */
+	GIT_COLOR_CYAN,		/* NEW_MOVED */
+	GIT_COLOR_YELLOW,	/* NEW_MOVED ALTERNATIVE */
 };
 
 static NORETURN void die_want_option(const char *option_name)
@@ -80,6 +86,14 @@ static int parse_diff_color_slot(const char *var)
 		return DIFF_WHITESPACE;
 	if (!strcasecmp(var, "func"))
 		return DIFF_FUNCINFO;
+	if (!strcasecmp(var, "oldmoved"))
+		return DIFF_FILE_OLD_MOVED;
+	if (!strcasecmp(var, "oldmovedalternative"))
+		return DIFF_FILE_OLD_MOVED_ALT;
+	if (!strcasecmp(var, "newmoved"))
+		return DIFF_FILE_NEW_MOVED;
+	if (!strcasecmp(var, "newmovedalternative"))
+		return DIFF_FILE_NEW_MOVED_ALT;
 	return -1;
 }
 
@@ -228,12 +242,29 @@ int git_diff_heuristic_config(const char *var, const char *value, void *cb)
 	return 0;
 }
 
+static int parse_color_moved(const char *arg)
+{
+	if (!strcmp(arg, "no"))
+		return COLOR_MOVED_NO;
+	else if (!strcmp(arg, "zebra"))
+		return COLOR_MOVED_ZEBRA;
+	else
+		return -1;
+}
+
 int git_diff_ui_config(const char *var, const char *value, void *cb)
 {
 	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
 		diff_use_color_default = git_config_colorbool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.colormoved")) {
+		int cm = parse_color_moved(value);
+		if (cm < 0)
+			return -1;
+		diff_color_moved_default = cm;
+		return 0;
+	}
 	if (!strcmp(var, "diff.context")) {
 		diff_context_default = git_config_int(var, value);
 		if (diff_context_default < 0)
@@ -600,7 +631,9 @@ enum diff_symbol {
  * 13-15 are WSEH_NEW | WSEH_OLD | WSEH_CONTEXT
  * 16 is marking if the line is blank at EOF
  */
-#define DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF (1<<16)
+#define DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF	(1<<16)
+#define DIFF_SYMBOL_MOVED_LINE			(1<<17)
+#define DIFF_SYMBOL_MOVED_LINE_ZEBRA		(1<<18)
 #define DIFF_SYMBOL_CONTENT_WS_MASK (WSEH_NEW | WSEH_OLD | WSEH_CONTEXT | WS_RULE_MASK)
 
 /*
@@ -643,6 +676,213 @@ static void append_emitted_diff_symbol(struct diff_options *o,
 	f->line = e->line ? xmemdupz(e->line, e->len) : NULL;
 }
 
+struct moved_entry {
+	struct hashmap_entry ent;
+	const struct emitted_diff_symbol *es;
+	struct moved_entry *next_line;
+};
+
+static void get_ws_cleaned_string(const struct emitted_diff_symbol *l,
+				  struct strbuf *out)
+{
+	int i;
+	for (i = 0; i < l->len; i++) {
+		if (isspace(l->line[i]))
+			continue;
+		strbuf_addch(out, l->line[i]);
+	}
+}
+
+static int emitted_symbol_cmp_no_ws(const struct emitted_diff_symbol *a,
+				    const struct emitted_diff_symbol *b,
+				    const void *keydata)
+{
+	int ret;
+	struct strbuf sba = STRBUF_INIT;
+	struct strbuf sbb = STRBUF_INIT;
+
+	get_ws_cleaned_string(a, &sba);
+	get_ws_cleaned_string(b, &sbb);
+	ret = sba.len != sbb.len || strncmp(sba.buf, sbb.buf, sba.len);
+
+	strbuf_release(&sba);
+	strbuf_release(&sbb);
+	return ret;
+}
+
+static int emitted_symbol_cmp(const struct emitted_diff_symbol *a,
+			      const struct emitted_diff_symbol *b,
+			      const void *keydata)
+{
+	return a->len != b->len || strncmp(a->line, b->line, a->len);
+}
+
+static int moved_entry_cmp(const struct moved_entry *a,
+			   const struct moved_entry *b,
+			   const void *keydata)
+{
+	return emitted_symbol_cmp(a->es, b->es, keydata);
+}
+
+static int moved_entry_cmp_no_ws(const struct moved_entry *a,
+				 const struct moved_entry *b,
+				 const void *keydata)
+{
+	return emitted_symbol_cmp_no_ws(a->es, b->es, keydata);
+}
+
+static unsigned get_string_hash(struct emitted_diff_symbol *es, unsigned ignore_ws)
+{
+	static struct strbuf sb = STRBUF_INIT;
+
+	if (ignore_ws) {
+		strbuf_reset(&sb);
+		get_ws_cleaned_string(es, &sb);
+		return memhash(sb.buf, sb.len);
+	} else {
+		return memhash(es->line, es->len);
+	}
+}
+
+static struct moved_entry *prepare_entry(struct diff_options *o,
+					 int line_no)
+{
+	struct moved_entry *ret = xmalloc(sizeof(*ret));
+	unsigned ignore_ws = DIFF_XDL_TST(o, IGNORE_WHITESPACE);
+	struct emitted_diff_symbol *l = &o->emitted_symbols->buf[line_no];
+
+	ret->ent.hash = get_string_hash(l, ignore_ws);
+	ret->es = l;
+	ret->next_line = NULL;
+
+	return ret;
+}
+
+static void add_lines_to_move_detection(struct diff_options *o,
+					struct hashmap *add_lines,
+					struct hashmap *del_lines)
+{
+	struct moved_entry *prev_line = NULL;
+
+	int n;
+	for (n = 0; n < o->emitted_symbols->nr; n++) {
+		struct hashmap *hm;
+		struct moved_entry *key;
+
+		switch (o->emitted_symbols->buf[n].s) {
+		case DIFF_SYMBOL_PLUS:
+			hm = add_lines;
+			break;
+		case DIFF_SYMBOL_MINUS:
+			hm = del_lines;
+			break;
+		default:
+			prev_line = NULL;
+			continue;
+		}
+
+		key = prepare_entry(o, n);
+		if (prev_line && prev_line->es->s == o->emitted_symbols->buf[n].s)
+			prev_line->next_line = key;
+
+		hashmap_add(hm, key);
+		prev_line = key;
+	}
+}
+
+/* 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,
+				struct hashmap *del_lines)
+{
+	struct moved_entry **pmb = NULL; /* potentially moved blocks */
+	int pmb_nr = 0, pmb_alloc = 0;
+	int n, flipped_block = 1;
+
+	for (n = 0; n < o->emitted_symbols->nr; n++) {
+		struct hashmap *hm = NULL;
+		struct moved_entry *key;
+		struct moved_entry *match = NULL;
+		struct emitted_diff_symbol *l = &o->emitted_symbols->buf[n];
+		int i, lp, rp;
+
+		switch (l->s) {
+		case DIFF_SYMBOL_PLUS:
+			hm = del_lines;
+			key = prepare_entry(o, n);
+			match = hashmap_get(hm, key, o);
+			free(key);
+			break;
+		case DIFF_SYMBOL_MINUS:
+			hm = add_lines;
+			key = prepare_entry(o, n);
+			match = hashmap_get(hm, key, o);
+			free(key);
+			break;
+		default:
+			flipped_block = 1;
+		}
+
+		if (!match) {
+			pmb_nr = 0;
+			continue;
+		}
+
+		l->flags |= DIFF_SYMBOL_MOVED_LINE;
+
+		/* Check any potential block runs, advance each or nullify */
+		for (i = 0; i < pmb_nr; i++) {
+			struct moved_entry *p = pmb[i];
+			struct moved_entry *pnext = (p && p->next_line) ?
+					p->next_line : NULL;
+			if (pnext &&
+			    !emitted_symbol_cmp(pnext->es, l, o)) {
+				pmb[i] = p->next_line;
+			} else {
+				pmb[i] = NULL;
+			}
+		}
+
+		/* Shrink the set of potential block to the remaining running */
+		for (lp = 0, rp = pmb_nr - 1; lp <= rp;) {
+			while (lp < pmb_nr && pmb[lp])
+				lp++;
+			/* lp points at the first NULL now */
+
+			while (rp > -1 && !pmb[rp])
+				rp--;
+			/* rp points at the last non-NULL */
+
+			if (lp < pmb_nr && rp > -1 && lp < rp) {
+				pmb[lp] = pmb[rp];
+				pmb[rp] = NULL;
+				rp--;
+				lp++;
+			}
+		}
+
+		/* Remember the number of running sets */
+		pmb_nr = rp + 1;
+
+		if (pmb_nr == 0) {
+			/*
+			 * The current line is the start of a new block.
+			 * Setup the set of potential blocks.
+			 */
+			for (; match; match = hashmap_get_next(hm, match)) {
+				ALLOC_GROW(pmb, pmb_nr + 1, pmb_alloc);
+				pmb[pmb_nr++] = match;
+			}
+
+			flipped_block = (flipped_block + 1) % 2;
+		}
+
+		if (flipped_block)
+			l->flags |= DIFF_SYMBOL_MOVED_LINE_ZEBRA;
+	}
+
+	free(pmb);
+}
 
 static void emit_line_ws_markup(struct diff_options *o,
 				const char *set, const char *reset,
@@ -715,14 +955,24 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
 		emit_line(o, context, reset, line, len);
 		break;
 	case DIFF_SYMBOL_PLUS:
-		set = diff_get_color_opt(o, DIFF_FILE_NEW);
+		if (flags & DIFF_SYMBOL_MOVED_LINE_ZEBRA)
+			set = diff_get_color_opt(o, DIFF_FILE_NEW_MOVED_ALT);
+		else if (flags & DIFF_SYMBOL_MOVED_LINE)
+			set = diff_get_color_opt(o, DIFF_FILE_NEW_MOVED);
+		else
+			set = diff_get_color_opt(o, DIFF_FILE_NEW);
 		reset = diff_get_color_opt(o, DIFF_RESET);
 		emit_line_ws_markup(o, set, reset, line, len, '+',
 				    flags & DIFF_SYMBOL_CONTENT_WS_MASK,
 				    flags & DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF);
 		break;
 	case DIFF_SYMBOL_MINUS:
-		set = diff_get_color_opt(o, DIFF_FILE_OLD);
+		if (flags & DIFF_SYMBOL_MOVED_LINE_ZEBRA)
+			set = diff_get_color_opt(o, DIFF_FILE_OLD_MOVED_ALT);
+		else if (flags & DIFF_SYMBOL_MOVED_LINE)
+			set = diff_get_color_opt(o, DIFF_FILE_OLD_MOVED);
+		else
+			set = diff_get_color_opt(o, DIFF_FILE_OLD);
 		reset = diff_get_color_opt(o, DIFF_RESET);
 		emit_line_ws_markup(o, set, reset, line, len, '-',
 				    flags & DIFF_SYMBOL_CONTENT_WS_MASK, 0);
@@ -3750,6 +4000,8 @@ void diff_setup(struct diff_options *options)
 		options->a_prefix = "a/";
 		options->b_prefix = "b/";
 	}
+
+	options->color_moved = diff_color_moved_default;
 }
 
 void diff_setup_done(struct diff_options *options)
@@ -3859,6 +4111,9 @@ void diff_setup_done(struct diff_options *options)
 
 	if (DIFF_OPT_TST(options, FOLLOW_RENAMES) && options->pathspec.nr != 1)
 		die(_("--follow requires exactly one pathspec"));
+
+	if (!options->use_color || external_diff())
+		options->color_moved = 0;
 }
 
 static int opt_arg(const char *arg, int arg_short, const char *arg_long, int *val)
@@ -4283,7 +4538,16 @@ int diff_opt_parse(struct diff_options *options,
 	}
 	else if (!strcmp(arg, "--no-color"))
 		options->use_color = 0;
-	else if (!strcmp(arg, "--color-words")) {
+	else if (!strcmp(arg, "--color-moved"))
+		options->color_moved = COLOR_MOVED_ZEBRA;
+	else if (!strcmp(arg, "--no-color-moved"))
+		options->color_moved = COLOR_MOVED_NO;
+	else if (skip_prefix(arg, "--color-moved=", &arg)) {
+		int cm = parse_color_moved(arg);
+		if (cm < 0)
+			die("bad --color-moved argument: %s", arg);
+		options->color_moved = cm;
+	} else if (!strcmp(arg, "--color-words")) {
 		options->use_color = 1;
 		options->word_diff = DIFF_WORDS_COLOR;
 	}
@@ -5094,16 +5358,9 @@ static void diff_flush_patch_all_file_pairs(struct diff_options *o)
 	int i;
 	static struct emitted_diff_symbols esm = EMITTED_DIFF_SYMBOLS_INIT;
 	struct diff_queue_struct *q = &diff_queued_diff;
-	/*
-	 * For testing purposes we want to make sure the diff machinery
-	 * works completely with the buffer. If there is anything emitted
-	 * outside the emit_string, then the order is screwed
-	 * up and the tests will fail.
-	 *
-	 * TODO (later in this series):
-	 * We'll unset this pointer in a later patch.
-	 */
-	o->emitted_symbols = &esm;
+
+	if (o->color_moved)
+		o->emitted_symbols = &esm;
 
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
@@ -5112,6 +5369,24 @@ static void diff_flush_patch_all_file_pairs(struct diff_options *o)
 	}
 
 	if (o->emitted_symbols) {
+		if (o->color_moved) {
+			struct hashmap add_lines, del_lines;
+			unsigned ignore_ws = DIFF_XDL_TST(o, IGNORE_WHITESPACE);
+
+			hashmap_init(&del_lines, ignore_ws ?
+				(hashmap_cmp_fn)moved_entry_cmp_no_ws :
+				(hashmap_cmp_fn)moved_entry_cmp, 0);
+			hashmap_init(&add_lines, ignore_ws ?
+				(hashmap_cmp_fn)moved_entry_cmp_no_ws :
+				(hashmap_cmp_fn)moved_entry_cmp, 0);
+
+			add_lines_to_move_detection(o, &add_lines, &del_lines);
+			mark_color_as_moved(o, &add_lines, &del_lines);
+
+			hashmap_free(&add_lines, 0);
+			hashmap_free(&del_lines, 0);
+		}
+
 		for (i = 0; i < esm.nr; i++)
 			emit_diff_symbol_from_struct(o, &esm.buf[i]);
 
@@ -5195,6 +5470,7 @@ void diff_flush(struct diff_options *options)
 		if (!options->file)
 			die_errno("Could not open /dev/null");
 		options->close_file = 1;
+		options->color_moved = 0;
 		for (i = 0; i < q->nr; i++) {
 			struct diff_filepair *p = q->queue[i];
 			if (check_pair_status(p))
diff --git a/diff.h b/diff.h
index 65fc9dbb4b..7726ad255c 100644
--- a/diff.h
+++ b/diff.h
@@ -188,6 +188,10 @@ struct diff_options {
 	int diff_path_counter;
 
 	struct emitted_diff_symbols *emitted_symbols;
+	enum {
+		COLOR_MOVED_NO = 0,
+		COLOR_MOVED_ZEBRA = 2,
+	} color_moved;
 };
 
 void diff_emit_submodule_del(struct diff_options *o, const char *line);
@@ -208,7 +212,11 @@ enum color_diff {
 	DIFF_FILE_NEW = 5,
 	DIFF_COMMIT = 6,
 	DIFF_WHITESPACE = 7,
-	DIFF_FUNCINFO = 8
+	DIFF_FUNCINFO = 8,
+	DIFF_FILE_OLD_MOVED = 9,
+	DIFF_FILE_OLD_MOVED_ALT = 10,
+	DIFF_FILE_NEW_MOVED = 11,
+	DIFF_FILE_NEW_MOVED_ALT = 12
 };
 const char *diff_get_color(int diff_use_color, enum color_diff ix);
 #define diff_get_color_opt(o, ix) \
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 289806d0c7..4a03766f1f 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -972,4 +972,200 @@ test_expect_success 'option overrides diff.wsErrorHighlight' '
 
 '
 
+test_expect_success 'detect moved code, complete file' '
+	git reset --hard &&
+	cat <<-\EOF >test.c &&
+	#include<stdio.h>
+	main()
+	{
+	printf("Hello World");
+	}
+	EOF
+	git add test.c &&
+	git commit -m "add main function" &&
+	git mv test.c main.c &&
+	test_config color.diff.oldMoved "normal red" &&
+	test_config color.diff.newMoved "normal green" &&
+	git diff HEAD --color-moved --no-renames | test_decode_color >actual &&
+	cat >expected <<-\EOF &&
+	<BOLD>diff --git a/main.c b/main.c<RESET>
+	<BOLD>new file mode 100644<RESET>
+	<BOLD>index 0000000..a986c57<RESET>
+	<BOLD>--- /dev/null<RESET>
+	<BOLD>+++ b/main.c<RESET>
+	<CYAN>@@ -0,0 +1,5 @@<RESET>
+	<BGREEN>+<RESET><BGREEN>#include<stdio.h><RESET>
+	<BGREEN>+<RESET><BGREEN>main()<RESET>
+	<BGREEN>+<RESET><BGREEN>{<RESET>
+	<BGREEN>+<RESET><BGREEN>printf("Hello World");<RESET>
+	<BGREEN>+<RESET><BGREEN>}<RESET>
+	<BOLD>diff --git a/test.c b/test.c<RESET>
+	<BOLD>deleted file mode 100644<RESET>
+	<BOLD>index a986c57..0000000<RESET>
+	<BOLD>--- a/test.c<RESET>
+	<BOLD>+++ /dev/null<RESET>
+	<CYAN>@@ -1,5 +0,0 @@<RESET>
+	<BRED>-#include<stdio.h><RESET>
+	<BRED>-main()<RESET>
+	<BRED>-{<RESET>
+	<BRED>-printf("Hello World");<RESET>
+	<BRED>-}<RESET>
+	EOF
+
+	test_cmp expected actual
+'
+
+test_expect_success 'detect malicious moved code, inside file' '
+	test_config color.diff.oldMoved "normal red" &&
+	test_config color.diff.newMoved "normal green" &&
+	test_config color.diff.oldMovedAlternative "blue" &&
+	test_config color.diff.newMovedAlternative "yellow" &&
+	git reset --hard &&
+	cat <<-\EOF >main.c &&
+		#include<stdio.h>
+		int stuff()
+		{
+			printf("Hello ");
+			printf("World\n");
+		}
+
+		int secure_foo(struct user *u)
+		{
+			if (!u->is_allowed_foo)
+				return;
+			foo(u);
+		}
+
+		int main()
+		{
+			foo();
+		}
+	EOF
+	cat <<-\EOF >test.c &&
+		#include<stdio.h>
+		int bar()
+		{
+			printf("Hello World, but different\n");
+		}
+
+		int another_function()
+		{
+			bar();
+		}
+	EOF
+	git add main.c test.c &&
+	git commit -m "add main and test file" &&
+	cat <<-\EOF >main.c &&
+		#include<stdio.h>
+		int stuff()
+		{
+			printf("Hello ");
+			printf("World\n");
+		}
+
+		int main()
+		{
+			foo();
+		}
+	EOF
+	cat <<-\EOF >test.c &&
+		#include<stdio.h>
+		int bar()
+		{
+			printf("Hello World, but different\n");
+		}
+
+		int secure_foo(struct user *u)
+		{
+			foo(u);
+			if (!u->is_allowed_foo)
+				return;
+		}
+
+		int another_function()
+		{
+			bar();
+		}
+	EOF
+	git diff HEAD --no-renames --color-moved=zebra| test_decode_color >actual &&
+	cat <<-\EOF >expected &&
+	<BOLD>diff --git a/main.c b/main.c<RESET>
+	<BOLD>index 27a619c..7cf9336 100644<RESET>
+	<BOLD>--- a/main.c<RESET>
+	<BOLD>+++ b/main.c<RESET>
+	<CYAN>@@ -5,13 +5,6 @@<RESET> <RESET>printf("Hello ");<RESET>
+	 printf("World\n");<RESET>
+	 }<RESET>
+	 <RESET>
+	<BRED>-int secure_foo(struct user *u)<RESET>
+	<BRED>-{<RESET>
+	<BLUE>-if (!u->is_allowed_foo)<RESET>
+	<BLUE>-return;<RESET>
+	<BRED>-foo(u);<RESET>
+	<BLUE>-}<RESET>
+	<BLUE>-<RESET>
+	 int main()<RESET>
+	 {<RESET>
+	 foo();<RESET>
+	<BOLD>diff --git a/test.c b/test.c<RESET>
+	<BOLD>index 1dc1d85..2bedec9 100644<RESET>
+	<BOLD>--- a/test.c<RESET>
+	<BOLD>+++ b/test.c<RESET>
+	<CYAN>@@ -4,6 +4,13 @@<RESET> <RESET>int bar()<RESET>
+	 printf("Hello World, but different\n");<RESET>
+	 }<RESET>
+	 <RESET>
+	<BGREEN>+<RESET><BGREEN>int secure_foo(struct user *u)<RESET>
+	<BGREEN>+<RESET><BGREEN>{<RESET>
+	<YELLOW>+<RESET><YELLOW>foo(u);<RESET>
+	<BGREEN>+<RESET><BGREEN>if (!u->is_allowed_foo)<RESET>
+	<BGREEN>+<RESET><BGREEN>return;<RESET>
+	<YELLOW>+<RESET><YELLOW>}<RESET>
+	<YELLOW>+<RESET>
+	 int another_function()<RESET>
+	 {<RESET>
+	 bar();<RESET>
+	EOF
+
+	test_cmp expected actual
+'
+
+test_expect_success 'no effect from --color-moved with --word-diff' '
+	cat <<-\EOF >text.txt &&
+	Lorem Ipsum is simply dummy text of the printing and typesetting industry.
+	EOF
+	git add text.txt &&
+	git commit -a -m "clean state" &&
+	cat <<-\EOF >text.txt &&
+	simply Lorem Ipsum dummy is text of the typesetting and printing industry.
+	EOF
+	git diff --color-moved --word-diff >actual &&
+	git diff --word-diff >expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'move detection with submodules' '
+	test_create_repo bananas &&
+	echo ripe >bananas/recipe &&
+	git -C bananas add recipe &&
+	test_commit fruit &&
+	test_commit -C bananas recipe &&
+	git submodule add ./bananas &&
+	git add bananas &&
+	git commit -a -m "bananas are like a heavy library?" &&
+	echo foul >bananas/recipe &&
+	echo ripe >fruit.t &&
+
+	git diff --submodule=diff --color-moved >actual &&
+
+	# no move detection as the moved line is across repository boundaries.
+	test_decode_color <actual >decoded_actual &&
+	! grep BGREEN decoded_actual &&
+	! grep BRED decoded_actual &&
+
+	# nor did we mess with it another way
+	git diff --submodule=diff | test_decode_color >expect &&
+	test_cmp expect decoded_actual
+'
+
 test_done
-- 
2.12.2.575.gb14f27f917
Previous: Stefan BellerNext: Jonathan Tan
Message 89 of 125 in “[PATCHv5 00/17] Diff machine: highlight moved lines.”
  1. Stefan BellerMay 24, 2017
  2. 01/17 diff: readability fixStefan Beller, May 24, 2017
  3. 03/17 diff.c: factor out diff_flush_patch_all_file_pairsStefan Beller, May 24, 2017
  4. 09/17 submodule.c: convert show_submodule_summary to use emit_line_fmtStefan Beller, May 24, 2017
  5. 04/17 diff: introduce more flexible emit functionStefan Beller, May 24, 2017
  6. Jonathan TanJun 13, 2017
  7. Stefan BellerJun 13, 2017
  8. Jonathan TanJun 13, 2017
  9. 05/17 diff.c: convert fn_out_consume to use emit_lineStefan Beller, May 24, 2017
  10. 02/17 diff: move line ending check into emit_hunk_headerStefan Beller, May 24, 2017
  11. 07/17 diff.c: convert emit_rewrite_diff to use emit_line_*Stefan Beller, May 24, 2017
  12. 15/17 diff.c: emit_line includes whitespace highlightingStefan Beller, May 24, 2017
  13. 11/17 diff.c: convert show_stats to use emit_line_*Stefan Beller, May 24, 2017
  14. 17/17 diff.c: color moved lines differentlyStefan Beller, May 24, 2017
  15. Junio C HamanoMay 25, 2017
  16. Stefan BellerMay 25, 2017
  17. 10/17 diff.c: convert emit_binary_diff_body to use emit_line_*Stefan Beller, May 24, 2017
  18. 08/17 diff.c: convert emit_rewrite_lines to use emit_line_*Stefan Beller, May 24, 2017
  19. 16/17 diff: buffer all output if asked toStefan Beller, May 24, 2017
  20. Junio C HamanoMay 25, 2017
  21. Stefan BellerMay 25, 2017
  22. Junio C HamanoMay 26, 2017
  23. Jonathan TanJun 13, 2017
  24. Stefan BellerJun 14, 2017
  25. 12/17 diff.c: convert word diffing to use emit_line_*Stefan Beller, May 24, 2017
  26. 14/17 diff.c: convert diff_summary to use emit_line_*Stefan Beller, May 24, 2017
  27. 13/17 diff.c: convert diff_flush to use emit_line_*Stefan Beller, May 24, 2017
  28. 06/17 diff.c: convert builtin_diff to use emit_line_*Stefan Beller, May 24, 2017
  29. Junio C HamanoMay 25, 2017
  30. Stefan BellerMay 25, 2017
  31. Junio C HamanoMay 26, 2017
  32. Stefan BellerMay 26, 2017
  33. 0/1 Diff machine: highlight moved lines.Stefan Beller, May 27, 2017
  34. 1/1 diff.c: color moved lines differentlyStefan Beller, May 27, 2017
  35. Philip OakleyMay 27, 2017
  36. Stefan BellerMay 30, 2017
  37. diff.c: color moved lines differentlyStefan Beller, Jun 1, 2017
  38. Jonathan TanJun 13, 2017
  39. Stefan BellerJun 14, 2017
  40. 00/26 reroll of sb/diff-color-movedStefan Beller, Jun 20, 2017
  41. 02/26 diff.c: move line ending check into emit_hunk_headerStefan Beller, Jun 20, 2017
  42. 03/26 diff.c: factor out diff_flush_patch_all_file_pairsStefan Beller, Jun 20, 2017
  43. 06/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_FRAGINFOStefan Beller, Jun 20, 2017
  44. 07/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_NO_LF_EOFStefan Beller, Jun 20, 2017
  45. 08/26 diff.c: migrate emit_line_checked to use emit_diff_symbolStefan Beller, Jun 20, 2017
  46. Junio C HamanoJun 21, 2017
  47. Stefan BellerJun 22, 2017
  48. Stefan BellerJun 22, 2017
  49. Junio C HamanoJun 23, 2017
  50. 11/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_FILEPAIRStefan Beller, Jun 20, 2017
  51. Jonathan TanJun 20, 2017
  52. Junio C HamanoJun 21, 2017
  53. Stefan BellerJun 22, 2017
  54. 04/26 diff.c: introduce emit_diff_symbolStefan Beller, Jun 20, 2017
  55. Junio C HamanoJun 21, 2017
  56. Stefan BellerJun 21, 2017
  57. Junio C HamanoJun 21, 2017
  58. Junio C HamanoJun 21, 2017
  59. Stefan BellerJun 21, 2017
  60. Junio C HamanoJun 21, 2017
  61. Stefan BellerJun 21, 2017
  62. Brandon WilliamsJun 21, 2017
  63. Junio C HamanoJun 21, 2017
  64. Brandon WilliamsJun 21, 2017
  65. 15/26 submodule.c: migrate diff output to use emit_diff_symbolStefan Beller, Jun 20, 2017
  66. Jonathan TanJun 20, 2017
  67. 19/26 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_STAT_SEPStefan Beller, Jun 20, 2017
  68. 12/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_HEADERStefan Beller, Jun 20, 2017
  69. 17/26 diff.c: convert show_stats to use emit_diff_symbolStefan Beller, Jun 20, 2017
  70. Brandon WilliamsJun 21, 2017
  71. Stefan BellerJun 21, 2017
  72. 18/26 diff.c: convert word diffing to use emit_diff_symbolStefan Beller, Jun 20, 2017
  73. 25/26 diff: document the new --color-moved settingStefan Beller, Jun 20, 2017
  74. 14/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_REWRITE_DIFFStefan Beller, Jun 20, 2017
  75. 26/26 diff.c: have a "machine parseable" move coloringStefan Beller, Jun 20, 2017
  76. Stefan BellerJun 20, 2017
  77. Ævar Arnfjörð BjarmasonJun 23, 2017
  78. Brandon WilliamsJun 21, 2017
  79. Junio C HamanoJun 21, 2017
  80. Stefan BellerJun 21, 2017
  81. 20/26 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARYStefan Beller, Jun 20, 2017
  82. 21/26 diff.c: buffer all output if asked toStefan Beller, Jun 20, 2017
  83. 24/26 diff.c: add dimming to moved line detectionStefan Beller, Jun 20, 2017
  84. Junio C HamanoJun 21, 2017
  85. 16/26 diff.c: convert emit_binary_diff_body to use emit_diff_symbolStefan Beller, Jun 20, 2017
  86. Junio C HamanoJun 21, 2017
  87. 23/26 diff.c: color moved lines differently, plain modeStefan Beller, Jun 20, 2017
  88. 09/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_WORDS{_PORCELAIN}Stefan Beller, Jun 20, 2017
  89. 22/26 diff.c: color moved lines differentlyStefan Beller, Jun 20, 2017
  90. Jonathan TanJun 20, 2017
  91. Stefan BellerJun 20, 2017
  92. 05/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_MARKERStefan Beller, Jun 20, 2017
  93. 10/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_INCOMPLETEStefan Beller, Jun 20, 2017
  94. 13/26 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_BINARY_FILESStefan Beller, Jun 20, 2017
  95. Junio C HamanoJun 21, 2017
  96. Stefan BellerJun 21, 2017
  97. 01/26 diff.c: readability fixStefan Beller, Jun 20, 2017
  98. 00/25 reroll of sb/diff-color-movedStefan Beller, Jun 23, 2017
  99. 02/25 diff.c: move line ending check into emit_hunk_headerStefan Beller, Jun 23, 2017
  100. 04/25 diff.c: introduce emit_diff_symbolStefan Beller, Jun 23, 2017
  101. Junio C HamanoJun 23, 2017
  102. Stefan BellerJun 23, 2017
  103. 06/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_FRAGINFOStefan Beller, Jun 23, 2017
  104. 03/25 diff.c: factor out diff_flush_patch_all_file_pairsStefan Beller, Jun 23, 2017
  105. 05/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_MARKERStefan Beller, Jun 23, 2017
  106. 09/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_WORDS[_PORCELAIN]Stefan Beller, Jun 23, 2017
  107. 07/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_NO_LF_EOFStefan Beller, Jun 23, 2017
  108. 13/25 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_BINARY_FILESStefan Beller, Jun 23, 2017
  109. 12/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_HEADERStefan Beller, Jun 23, 2017
  110. 14/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_REWRITE_DIFFStefan Beller, Jun 23, 2017
  111. 23/25 diff.c: color moved lines differently, plain modeStefan Beller, Jun 23, 2017
  112. 22/25 diff.c: color moved lines differentlyStefan Beller, Jun 23, 2017
  113. 15/25 submodule.c: migrate diff output to use emit_diff_symbolStefan Beller, Jun 23, 2017
  114. 25/25 diff: document the new --color-moved settingStefan Beller, Jun 23, 2017
  115. 11/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_FILEPAIR_{PLUS, MINUS}Stefan Beller, Jun 23, 2017
  116. 24/25 diff.c: add dimming to moved line detectionStefan Beller, Jun 23, 2017
  117. 01/25 diff.c: readability fixStefan Beller, Jun 23, 2017
  118. 10/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_INCOMPLETEStefan Beller, Jun 23, 2017
  119. 21/25 diff.c: buffer all output if asked toStefan Beller, Jun 23, 2017
  120. 08/25 diff.c: migrate emit_line_checked to use emit_diff_symbolStefan Beller, Jun 23, 2017
  121. 18/25 diff.c: convert word diffing to use emit_diff_symbolStefan Beller, Jun 23, 2017
  122. 17/25 diff.c: convert show_stats to use emit_diff_symbolStefan Beller, Jun 23, 2017
  123. 19/25 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_STAT_SEPStefan Beller, Jun 23, 2017
  124. 20/25 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARYStefan Beller, Jun 23, 2017
  125. 16/25 diff.c: convert emit_binary_diff_body to use emit_diff_symbolStefan Beller, Jun 23, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.