Actually, the logic to single out "binary vs binary" feels wrong, as one of the cases the output that was overly squelched is this one:
/* Quite common confusing case */
if (mf1.size == mf2.size &&
!memcmp(mf1.ptr, mf2.ptr, mf1.size))
goto free_ab_and_return;
fprintf(o->file, "%s", header.buf);
strbuf_reset(&header);Notice that we have accumulated the rename/copy/new/old in header but if the blobs are the same, we discard it.
Another case that 3e97c7c6 handled completely incorrectly is when -w/-b is in effect and there is a rename or copy. Because there won't be textual diff emitted by fn_out_consume() at all, we lose the header information as well.
So I would say we would need something like this on top of your patch.
--
diff.c | 27 +++++++++++++++++----------
t/t4015-diff-whitespace.sh | 37 +++++++++++++++++++++++++++++++++++++
2 files changed, 54 insertions(+), 10 deletions(-)
Show changes to 2 files +54 −10
diff.c, t/t4015-diff-whitespace.sh
diff --git a/diff.c b/diff.c
index 0b00271..ad31b00 100644
--- a/diff.c
+++ b/diff.c
@@ -1656,6 +1656,7 @@ static void builtin_diff(const char *name_a,
struct diff_filespec *one,
struct diff_filespec *two,
const char *xfrm_msg,
+ int must_show_header,
struct diff_options *o,
int complete_rewrite)
{
@@ -1708,16 +1709,19 @@ static void builtin_diff(const char *name_a,
strbuf_addf(&header, "%snew file mode %06o%s\n", set, two->mode, reset);
if (xfrm_msg && xfrm_msg[0])
strbuf_addf(&header, "%s%s%s\n", set, xfrm_msg, reset);
+ must_show_header = 1;
}
else if (lbl[1][0] == '/') {
strbuf_addf(&header, "%sdeleted file mode %06o%s\n", set, one->mode, reset);
if (xfrm_msg && xfrm_msg[0])
strbuf_addf(&header, "%s%s%s\n", set, xfrm_msg, reset);
+ must_show_header = 1;
}
else {
if (one->mode != two->mode) {
strbuf_addf(&header, "%sold mode %06o%s\n", set, one->mode, reset);
strbuf_addf(&header, "%snew mode %06o%s\n", set, two->mode, reset);
+ must_show_header = 1;
}
if (xfrm_msg && xfrm_msg[0])
strbuf_addf(&header, "%s%s%s\n", set, xfrm_msg, reset);
@@ -1737,10 +1741,6 @@ static void builtin_diff(const char *name_a,
textconv_one, textconv_two, o);
o->found_changes = 1;
goto free_ab_and_return;
- } else if (diff_filespec_is_binary(one) &&
- diff_filespec_is_binary(two)) {
- fprintf(o->file, "%s", header.buf);
- strbuf_reset(&header);
}
}
@@ -1751,8 +1751,11 @@ static void builtin_diff(const char *name_a,
die("unable to read files to diff");
/* Quite common confusing case */
if (mf1.size == mf2.size &&
- !memcmp(mf1.ptr, mf2.ptr, mf1.size))
+ !memcmp(mf1.ptr, mf2.ptr, mf1.size)) {
+ if (must_show_header)
+ fprintf(o->file, "%s", header.buf);
goto free_ab_and_return;
+ }
fprintf(o->file, "%s", header.buf);
strbuf_reset(&header);
if (DIFF_OPT_TST(o, BINARY))
@@ -1770,7 +1773,7 @@ static void builtin_diff(const char *name_a,
struct emit_callback ecbdata;
const struct userdiff_funcname *pe;
- if (!DIFF_XDL_TST(o, WHITESPACE_FLAGS)) {
+ if (!DIFF_XDL_TST(o, WHITESPACE_FLAGS) || must_show_header) {
fprintf(o->file, "%s", header.buf);
strbuf_reset(&header);
}
@@ -2383,8 +2386,10 @@ static void fill_metainfo(struct strbuf *msg,
struct diff_filespec *one,
struct diff_filespec *two,
struct diff_options *o,
- struct diff_filepair *p)
+ struct diff_filepair *p,
+ int *must_show_header)
{
+ *must_show_header = 1;
strbuf_init(msg, PATH_MAX * 2 + 300);
switch (p->status) {
case DIFF_STATUS_COPIED:
@@ -2412,7 +2417,7 @@ static void fill_metainfo(struct strbuf *msg,
/* fallthru */
default:
/* nothing */
- ;
+ *must_show_header = 0;
}
if (one && two && hashcmp(one->sha1, two->sha1)) {
int abbrev = DIFF_OPT_TST(o, FULL_INDEX) ? 40 : DEFAULT_ABBREV;
@@ -2446,9 +2451,10 @@ static void run_diff_cmd(const char *pgm,
{
const char *xfrm_msg = NULL;
int complete_rewrite = (p->status == DIFF_STATUS_MODIFIED) && p->score;
+ int must_show_header = 0;
if (msg) {
- fill_metainfo(msg, name, other, one, two, o, p);
+ fill_metainfo(msg, name, other, one, two, o, p, &must_show_header);
xfrm_msg = msg->len ? msg->buf : NULL;
}
@@ -2467,7 +2473,8 @@ static void run_diff_cmd(const char *pgm,
}
if (one && two)
builtin_diff(name, other ? other : name,
- one, two, xfrm_msg, o, complete_rewrite);
+ one, two, xfrm_msg, must_show_header,
+ o, complete_rewrite);
else
fprintf(o->file, "* Unmerged path %s\n", name);
}
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index e92eab0..935d101 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -438,6 +438,43 @@ test_expect_success 'whitespace-only changes not reported' '
test_cmp expect actual
'
+cat <<EOF >expect
+diff --git a/x b/z
+similarity index NUM%
+rename from x
+rename to z
+index 380c32a..a97b785 100644
+EOF
+test_expect_success 'whitespace-only changes reported across renames' '
+ git reset --hard &&
+ for i in 1 2 3 4 5 6 7 8 9; do echo "$i$i$i$i$i$i"; done >x &&
+ git add x &&
+ git commit -m "base" &&
+ sed -e "5s/^/ /" x >z &&
+ git rm x &&
+ git add z &&
+ git diff -w -M --cached |
+ sed -e "/^similarity index /s/[0-9][0-9]*/NUM/" >actual &&
+ test_cmp expect actual
+'
+
+cat >expected <<\EOF
+diff --git a/empty b/void
+similarity index 100%
+rename from empty
+rename to void
+EOF
+
+test_expect_success 'rename empty' '
+ git reset --hard &&
+ >empty &&
+ git add empty &&
+ git commit -m empty &&
+ git mv empty void &&
+ git diff -w --cached -M >current &&
+ test_cmp expected current
+'
+
test_expect_success 'combined diff with autocrlf conversion' '
git reset --hard &&
Christian Couder