diff: handle diffstat of rewritten binary files
The logic in builtin_diffstat assumes that a complete_rewrite pair should have its lines counted. This is nonsensical for binary files and leads to confusing things like: $ git diff --stat --summary HEAD^ HEAD foo.rand | Bin 4096 -> 4096 bytes 1 files changed, 0 insertions(+), 0 deletions(-) $ git diff --stat --summary -B HEAD^ HEAD foo.rand | 34 +++++++++++++++------------------- 1 files changed, 15 insertions(+), 19 deletions(-) rewrite foo.rand (100%) So let's reorder the function to handle binary files first (which from diffstat's perspective look like complete rewrites anyway), then rewrites, then actual diffstats. There are two bonus prizes to this reorder: 1. It gets rid of a now-superfluous goto. 2. The binary case is at the top, which means we can further optimize it in the next patch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
parent
e923eaeb90
commit
ded0abc73c
24
diff.c
24
diff.c
@ -1811,26 +1811,31 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
|
|||||||
data->is_unmerged = 1;
|
data->is_unmerged = 1;
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (complete_rewrite) {
|
|
||||||
|
if (diff_filespec_is_binary(one) || diff_filespec_is_binary(two)) {
|
||||||
|
if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
|
||||||
|
die("unable to read files to diff");
|
||||||
|
data->is_binary = 1;
|
||||||
|
data->added = mf2.size;
|
||||||
|
data->deleted = mf1.size;
|
||||||
|
}
|
||||||
|
|
||||||
|
else if (complete_rewrite) {
|
||||||
diff_populate_filespec(one, 0);
|
diff_populate_filespec(one, 0);
|
||||||
diff_populate_filespec(two, 0);
|
diff_populate_filespec(two, 0);
|
||||||
data->deleted = count_lines(one->data, one->size);
|
data->deleted = count_lines(one->data, one->size);
|
||||||
data->added = count_lines(two->data, two->size);
|
data->added = count_lines(two->data, two->size);
|
||||||
goto free_and_return;
|
|
||||||
}
|
}
|
||||||
if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
|
|
||||||
die("unable to read files to diff");
|
|
||||||
|
|
||||||
if (diff_filespec_is_binary(one) || diff_filespec_is_binary(two)) {
|
else {
|
||||||
data->is_binary = 1;
|
|
||||||
data->added = mf2.size;
|
|
||||||
data->deleted = mf1.size;
|
|
||||||
} else {
|
|
||||||
/* Crazy xdl interfaces.. */
|
/* Crazy xdl interfaces.. */
|
||||||
xpparam_t xpp;
|
xpparam_t xpp;
|
||||||
xdemitconf_t xecfg;
|
xdemitconf_t xecfg;
|
||||||
xdemitcb_t ecb;
|
xdemitcb_t ecb;
|
||||||
|
|
||||||
|
if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
|
||||||
|
die("unable to read files to diff");
|
||||||
|
|
||||||
memset(&xpp, 0, sizeof(xpp));
|
memset(&xpp, 0, sizeof(xpp));
|
||||||
memset(&xecfg, 0, sizeof(xecfg));
|
memset(&xecfg, 0, sizeof(xecfg));
|
||||||
xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
|
xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
|
||||||
@ -1838,7 +1843,6 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
|
|||||||
&xpp, &xecfg, &ecb);
|
&xpp, &xecfg, &ecb);
|
||||||
}
|
}
|
||||||
|
|
||||||
free_and_return:
|
|
||||||
diff_free_filespec_data(one);
|
diff_free_filespec_data(one);
|
||||||
diff_free_filespec_data(two);
|
diff_free_filespec_data(two);
|
||||||
}
|
}
|
||||||
|
@ -44,6 +44,13 @@ test_expect_success 'rewrite diff can show binary patch' '
|
|||||||
grep "GIT binary patch" diff
|
grep "GIT binary patch" diff
|
||||||
'
|
'
|
||||||
|
|
||||||
|
test_expect_success 'rewrite diff --stat shows binary changes' '
|
||||||
|
git diff -B --stat --summary >diff &&
|
||||||
|
grep "Bin" diff &&
|
||||||
|
grep "0 insertions.*0 deletions" diff &&
|
||||||
|
grep " rewrite file" diff
|
||||||
|
'
|
||||||
|
|
||||||
{
|
{
|
||||||
echo "#!$SHELL_PATH"
|
echo "#!$SHELL_PATH"
|
||||||
cat <<'EOF'
|
cat <<'EOF'
|
||||||
|
Loading…
Reference in New Issue
Block a user