"git diff --ignore-space-change --stat" lists...

"git diff --ignore-space-change --stat" lists files with only whitespace differences as "changed"

From: Matt McCutchen <hidden>
Date: 2017-01-18 02:18:19

A bug report: I noticed that "git diff --ignore-space-change --stat"
lists files with only whitespace differences as having changed with 0
differing lines.  This is inconsistent with the behavior without --
stat, which doesn't list such files at all.  (Same behavior with all
the --ignore*space* flags.)  I can reproduce this with the current
"next", af746e4.  Quick test case:

echo ' ' >test1 && echo '  ' >test2 &&
git diff --stat --no-index --ignore-space-change test1 test2

This caused me some inconvenience in the following scenario: I was
reading a commit diff that had a bulk license change in all files
combined with code changes.  I attempted to revert the bulk license
change locally using "sed" to more easily read the code diff, but my
reversion left some whitespace diffs where the original files had
inconsistent whitespace.  So the diffstat after my reversion was
cluttered with these "0" entries.

Regards,
Matt

Re: "git diff --ignore-space-change --stat" lists files with only whitespace differences as "changed"

From: Jeff King <hidden>
Date: 2017-01-18 11:19:12

On Tue, Jan 17, 2017 at 09:01:55PM -0500, Matt McCutchen wrote:

A bug report: I noticed that "git diff --ignore-space-change --stat"
lists files with only whitespace differences as having changed with 0
differing lines.  This is inconsistent with the behavior without --
stat, which doesn't list such files at all.  (Same behavior with all
the --ignore*space* flags.)  I can reproduce this with the current
"next", af746e4.  Quick test case:

Hmm. This is pretty easy to do naively, but the special-casing for
addition/deletion (which I think we _do_ need, and which certainly we
fail t4205 without) makes me feel dirty. I'd worry there are other
cases, too (perhaps renames?). And I also notice that the
binary-diffstat code path just above my changes explicitly creates 0/0
diffstats, but I'm not even sure how one would trigger that.

So I dunno. A sensible rule to me is "iff -p would show a diff header,
then --stat should mention it". I think we'd want to somehow extract the
logic from builtin_diff() and reuse it.

---

diff --git a/diff.c b/diff.c
index e2eb6d66a..57ff5c1dc 100644
--- a/diff.c
+++ b/diff.c

@@ -2105,17 +2105,20 @@ static void show_dirstat_by_line(struct diffstat_t *data, struct diff_options *o
 	gather_dirstat(options, &dir, changed, "", 0);
 }
 
+static void free_diffstat_file(struct diffstat_file *f)
+{
+	if (f->name != f->print_name)
+		free(f->print_name);
+	free(f->name);
+	free(f->from_name);
+	free(f);
+}
+
 static void free_diffstat_info(struct diffstat_t *diffstat)
 {
 	int i;
-	for (i = 0; i < diffstat->nr; i++) {
-		struct diffstat_file *f = diffstat->files[i];
-		if (f->name != f->print_name)
-			free(f->print_name);
-		free(f->name);
-		free(f->from_name);
-		free(f);
-	}
+	for (i = 0; i < diffstat->nr; i++)
+		free_diffstat_file(diffstat->files[i]);
 	free(diffstat->files);
 }

@@ -2603,6 +2606,23 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
 		if (xdi_diff_outf(&mf1, &mf2, diffstat_consume, diffstat,
 				  &xpp, &xecfg))
 			die("unable to generate diffstat for %s", one->path);
+
+		/*
+		 * Omit diffstats where nothing changed. Even if
+		 * !same_contents, this might be the case due to ignoring
+		 * whitespace changes, etc.
+		 *
+		 * But note that we special-case additions and deletions,
+		 * as adding an empty file, for example, is still of interest.
+		 */
+		if (DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two)) {
+			struct diffstat_file *file =
+				diffstat->files[diffstat->nr - 1];
+			if (!file->added && !file->deleted) {
+				free_diffstat_file(file);
+				diffstat->nr--;
+			}
+		}
 	}
 
 	diff_free_filespec_data(one);

diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 289806d0c..2805db411 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh

@@ -736,7 +736,7 @@ test_expect_success 'checkdiff allows new blank lines' '
 
 cat <<EOF >expect
 EOF
-test_expect_success 'whitespace-only changes not reported' '
+test_expect_success 'whitespace-only changes not reported (diff)' '
 	git reset --hard &&
 	echo >x "hello world" &&
 	git add x &&

@@ -746,6 +746,12 @@ test_expect_success 'whitespace-only changes not reported' '
 	test_cmp expect actual
 '
 
+test_expect_success 'whitespace-only changes not reported (diffstat)' '
+	# reuse state from previous test
+	git diff --stat -b >actual &&
+	test_cmp expect actual
+'
+
 cat <<EOF >expect
 diff --git a/x b/z
 similarity index NUM%

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help