[PATCH v2] diff: ensure consistent diff behavior with -I<regex> across output formats

[Lidong Yan <yldhome2d2@gmail.com>]

`git diff -I<regex>` option is inconsistently applied across various
output formats. In some cases, files would appear in the `--name-only`
output but not in the accompanying `--stat` or `-p` outputs, despite
the user explicitly requesting to ignore certain changes using
`-I<regex>`. Not only for `-I<regex>`, but this inconsistency also
exists for other output formats that have `.diff_from_content` set
(e.g. `-w`, `--ignore-space-at-eol` and `--ignore-space-change`).

To provide this consistency, introduces diffcore_ignore() to filter
out file pairs which only contains changes marked for ignoring before
flush diff output. Add test cases in t4015 and t4013 to verify that
files ignored by each ignore option do not appear in the output when
using the `--name-only` and `--name-status` options.

Signed-off-by: Lidong Yan <yldhome2d2@gmail.com>
---
 Makefile                   |   1 +
 diff.c                     |   2 +
 diffcore-ignore.c          | 151 +++++++++++++++++++++++++++++++++++++
 diffcore.h                 |   1 +
 meson.build                |   1 +
 t/t4013-diff-various.sh    |  53 +++++++++++++
 t/t4015-diff-whitespace.sh |  10 ++-
 7 files changed, 217 insertions(+), 2 deletions(-)
 create mode 100644 diffcore-ignore.c

diff --git a/Makefile b/Makefile
index 5f7dd79dfa..2cdbd5d3fb 100644
--- a/Makefile
+++ b/Makefile
@@ -1014,6 +1014,7 @@ LIB_OBJS += diff-no-index.o
 LIB_OBJS += diff.o
 LIB_OBJS += diffcore-break.o
 LIB_OBJS += diffcore-delta.o
+LIB_OBJS += diffcore-ignore.o
 LIB_OBJS += diffcore-order.o
 LIB_OBJS += diffcore-pickaxe.o
 LIB_OBJS += diffcore-rename.o
diff --git a/diff.c b/diff.c
index dca87e164f..1f905b5354 100644
--- a/diff.c
+++ b/diff.c
@@ -7088,6 +7088,8 @@ void diffcore_std(struct diff_options *options)
 	}
 	if (options->pickaxe_opts & DIFF_PICKAXE_KINDS_MASK)
 		diffcore_pickaxe(options);
+	if (options->flags.diff_from_contents)
+		diffcore_ignore(options);
 	if (options->orderfile)
 		diffcore_order(options->orderfile);
 	if (options->rotate_to)
diff --git a/diffcore-ignore.c b/diffcore-ignore.c
new file mode 100644
index 0000000000..2b80911449
--- /dev/null
+++ b/diffcore-ignore.c
@@ -0,0 +1,151 @@
+#include "git-compat-util.h"
+#include "diff.h"
+#include "diffcore.h"
+#include "xdiff-interface.h"
+
+#define KEEP   0
+#define IGNORE 1
+
+struct diffignore_cb {
+	regex_t **regex;
+	size_t sz;
+	int miss;
+};
+
+/*
+ * Check for any added or deleted line that does not match the ignore pattern.
+ * If found, mark data->miss=1 and return early.
+ */
+static int diffignore_consume(void *priv, char *line, unsigned long len)
+{
+	struct diffignore_cb *data = priv;
+
+	if (line[0] != '+' && line[0] != '-')
+		return 0;
+	if (data->miss)
+		BUG("Already matched in diffignore_consume! Broken xdiff_emit_line_fn?");
+
+	/* check if any of the regexes match */
+	data->miss = 1;
+	for (size_t nr = 0; nr < data->sz; nr++) {
+		regmatch_t regmatch;
+		regex_t *regex = data->regex[nr];
+
+		if (!regexec_buf(regex, line + 1, len - 1, 1,
+				 &regmatch, 0)) {
+			data->miss = 0;
+			break;
+		}
+	}
+
+	/* stop looking for miss */
+	if (data->miss)
+		return 1;
+	return 0;
+}
+
+/* return IGNORE to ignore this change, return KEEP to keep it. */
+static int diff_ignore(mmfile_t *one, mmfile_t *two, struct diff_options *o)
+{
+	struct diffignore_cb ecbdata;
+	xpparam_t xpp;
+	xdemitconf_t xecfg;
+	int ret;
+
+	/*
+	 * We have both sides; need to run textual diff and check if
+	 * there are any unmatched (non-ignored) added or deleted lines.
+	 */
+	memset(&xpp, 0, sizeof(xpp));
+	memset(&xecfg, 0, sizeof(xecfg));
+	ecbdata.regex = o->ignore_regex;
+	ecbdata.sz = o->ignore_regex_nr;
+	ecbdata.miss = 0;
+	xecfg.flags = XDL_EMIT_NO_HUNK_HDR;
+	xecfg.ctxlen = 0;
+	xecfg.interhunkctxlen = 0;
+	xpp.flags = o->xdl_opts & XDF_WHITESPACE_FLAGS;
+
+	/*
+	 * An xdiff error might be our "data->miss" from above. See the
+	 * comment for xdiff_emit_line_fn in xdiff-interface.h
+	 */
+	ret = xdi_diff_outf(one, two, NULL, diffignore_consume,
+			    &ecbdata, &xpp, &xecfg);
+
+	/* error happened, keep this change */
+	if (ret)
+		return KEEP;
+	/* If no regex matches, keep this change */
+	return ecbdata.miss ? KEEP : IGNORE;
+}
+
+/* return IGNORE to ignore this change, return KEEP to keep it. */
+static int ignore_match(struct diff_filepair *p, struct diff_options *o)
+{
+	struct userdiff_driver *textconv_one = NULL;
+	struct userdiff_driver *textconv_two = NULL;
+	mmfile_t mf1, mf2;
+	int ret;
+
+	/* keep unmerged */
+	if (DIFF_PAIR_UNMERGED(p))
+		return KEEP;
+
+	/* keep add/delete */
+	if (!DIFF_FILE_VALID(p->one) || !DIFF_FILE_VALID(p->two))
+		return KEEP;
+
+	/* keep mode changed pair */
+	if (DIFF_PAIR_MODE_CHANGED(p))
+		return KEEP;
+
+	if (o->flags.allow_textconv) {
+		textconv_one = get_textconv(o->repo, p->one);
+		textconv_two = get_textconv(o->repo, p->two);
+	}
+
+	/* unmodified pair will be ignored anyway */
+	if (textconv_one == textconv_two && diff_unmodified_pair(p))
+		return IGNORE;
+
+	/* keep binary files if diff cannot be performed */
+	if (!o->flags.text &&
+	    ((!textconv_one && diff_filespec_is_binary(o->repo, p->one)) ||
+	     (!textconv_two && diff_filespec_is_binary(o->repo, p->two))))
+		return KEEP;
+
+	mf1.size = fill_textconv(o->repo, textconv_one, p->one, &mf1.ptr);
+	mf2.size = fill_textconv(o->repo, textconv_two, p->two, &mf2.ptr);
+
+	ret = diff_ignore(&mf1, &mf2, o);
+
+	if (textconv_one)
+		free(mf1.ptr);
+	if (textconv_two)
+		free(mf2.ptr);
+	diff_free_filespec_data(p->one);
+	diff_free_filespec_data(p->two);
+
+	return ret;
+}
+
+void diffcore_ignore(struct diff_options *o)
+{
+	struct diff_queue_struct *q = &diff_queued_diff;
+	struct diff_queue_struct outq = DIFF_QUEUE_INIT;
+
+	if (!(o->output_format & (DIFF_FORMAT_NAME | DIFF_FORMAT_NAME_STATUS)))
+		return;
+
+	for (int i = 0; i < q->nr; i++) {
+		struct diff_filepair *p = q->queue[i];
+		if (ignore_match(p, o))
+			diff_free_filepair(p);
+		else
+			diff_q(&outq, p);
+	}
+
+	free(q->queue);
+	*q = outq;
+}
diff --git a/diffcore.h b/diffcore.h
index 9c0a0e7aaf..97e6e3553b 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -189,6 +189,7 @@ void diffcore_rename_extended(struct diff_options *options,
 			      struct strmap *cached_pairs);
 void diffcore_merge_broken(void);
 void diffcore_pickaxe(struct diff_options *);
+void diffcore_ignore(struct diff_options *);
 void diffcore_order(const char *orderfile);
 void diffcore_rotate(struct diff_options *);
 
diff --git a/meson.build b/meson.build
index 92d62695e3..c95c9b92bd 100644
--- a/meson.build
+++ b/meson.build
@@ -329,6 +329,7 @@ libgit_sources = [
   'diffcore-delta.c',
   'diffcore-order.c',
   'diffcore-pickaxe.c',
+  'diffcore-ignore.c',
   'diffcore-rename.c',
   'diffcore-rotate.c',
   'dir-iterator.c',
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 8ebd170451..72eb7ae703 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -643,6 +643,59 @@ test_expect_success 'diff -I<regex> --stat' '
 	test_cmp expect actual
 '
 
+test_expect_success 'diff -I<regex>: setup file1' '
+	test_seq 50 >file1 &&
+	git add file1 &&
+	test_seq 50 | sed -e "s/13/ten and three/" -e "s/^[124-9].*/& /" >file1
+'
+
+test_expect_success 'diff -I<regex>: ignore file1' '
+	git diff --ignore-blank-lines -I"ten.*e" -I"^[124-9]" >actual &&
+	cat >expect <<-\EOF &&
+	diff --git a/file0 b/file0
+	--- a/file0
+	+++ b/file0
+	@@ -34,7 +31,6 @@
+	 34
+	 35
+	 36
+	-37
+	 38
+	 39
+	 40
+	EOF
+	compare_diff_patch expect actual &&
+
+	git diff --stat --ignore-blank-lines -I"ten.*e" -I"^[124-9]" >actual &&
+	cat >expect <<-\EOF &&
+	 file0 | 1 -
+	 1 file changed, 1 deletion(-)
+	EOF
+	test_cmp expect actual &&
+
+	git diff --name-status --ignore-blank-lines -I"ten.*e" -I"^[124-9]" >actual &&
+	cat >expect <<-\EOF &&
+	M	file0
+	EOF
+	test_cmp expect actual
+'
+
+test_expect_success 'diff -I<regex> --raw: --raw ignores -I<regex>' '
+	git diff --raw >expect &&
+	git diff --raw --ignore-blank-lines -I"ten.*e" -I"^[124-9]" >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'diff -I<regex> --check: --check ignores -I<regex>' '
+	test_must_fail git diff --check 2>&1 >expect &&
+	test_must_fail git diff --check --ignore-blank-lines -I"ten.*e" -I"^[124-9]" 2>&1 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'diff -I<regex>: rm file1' '
+	git rm -f file1
+'
+
 test_expect_success 'diff -I<regex>: detect malformed regex' '
 	test_expect_code 129 git diff --ignore-matching-lines="^[124-9" 2>error &&
 	test_grep "invalid regex given to -I: " error
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 52e3e476ff..6e46a070e5 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -11,7 +11,7 @@ test_description='Test special whitespace in diff engine.
 . "$TEST_DIRECTORY"/lib-diff.sh
 
 for opt_res in --patch --quiet -s --stat --shortstat --dirstat=lines \
-	       --raw! --name-only! --name-status!
+	       --name-only --name-status --raw!
 do
 	opts=${opt_res%!} expect_failure=
 	test "$opts" = "$opt_res" ||
@@ -43,7 +43,13 @@ do
 		echo foo >x &&
 		git add x &&
 		echo " foo" >x &&
-		$expect_failure git diff -w $opts --exit-code x
+		$expect_failure git diff -w $opts --exit-code x &&
+		echo "foo " >x &&
+		$expect_failure git diff --ignore-space-at-eol $opts --exit-code x &&
+		echo "fo o" >x &&
+		git add x &&
+		echo "fo  o " >x &&
+		$expect_failure git diff --ignore-space-change $opts --exit-code x
 	'
 done
 
-- 
2.39.5 (Apple Git-154)