From: Thomas Guyot-Sionnest <tguyot@gmail.com>
To: git@vger.kernel.org
Cc: dermoth@aei.ca, me@ttaylorr.com, gitster@pobox.com,
Johannes.Schindelin@gmx.de, peff@peff.net,
Thomas Guyot-Sionnest <tguyot@gmail.com>
Subject: [PATCH v3] diff: Fix modified lines stats with --stat and --numstat
Date: Thu, 24 Sep 2020 01:24:07 -0400 [thread overview]
Message-ID: <20200924052406.11349-1-tguyot@gmail.com> (raw)
In-Reply-To: <1d0a60c3-d15e-bcbb-f042-2f8ae06f0de1@gmail.com>
Only skip diffstats when both oids are valid and identical. This check
was causing both false-positives (files included in diffstats with no
actual changes (0 lines modified) and false-negatives (showing 0 lines
modified in stats when files had actually changed).
Also renamed same_contents to same_file to avoid confusion.
Signed-off-by: Thomas Guyot-Sionnest <tguyot@gmail.com>
---
Interdiff:
diff --git a/diff.c b/diff.c
index 2e47bf824e..77e0bd772e 100644
--- a/diff.c
+++ b/diff.c
@@ -3663,7 +3663,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
{
mmfile_t mf1, mf2;
struct diffstat_file *data;
- int same_contents;
+ int same_file;
int complete_rewrite = 0;
if (!DIFF_PAIR_UNMERGED(p)) {
@@ -3681,19 +3681,14 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
return;
}
- /* What is_stdin really means is that the file's content is only
- * in the filespec's buffer and its oid is zero. We can't compare
- * oid's if both are null and we can just diff the buffers */
- if (one->is_stdin && two->is_stdin)
- same_contents = (one->size == two->size ?
- !memcmp(one->data, two->data, one->size) : 0);
- else
- same_contents = oideq(&one->oid, &two->oid);
+ /* saves some reads if true, not a guarantee of diff outcome */
+ same_file = one->oid_valid && two->oid_valid &&
+ oideq(&one->oid, &two->oid);
if (diff_filespec_is_binary(o->repo, one) ||
diff_filespec_is_binary(o->repo, two)) {
data->is_binary = 1;
- if (same_contents) {
+ if (same_file) {
data->added = 0;
data->deleted = 0;
} else {
@@ -3709,7 +3704,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
data->added = count_lines(two->data, two->size);
}
- else if (!same_contents) {
+ else if (!same_file) {
/* Crazy xdl interfaces.. */
xpparam_t xpp;
xdemitconf_t xecfg;
@@ -3734,7 +3729,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
diffstat->files[diffstat->nr - 1];
/*
* Omit diffstats of modified files where nothing changed.
- * Even if !same_contents, this might be the case due to
+ * Even if !same_file, this might be the case due to
* ignoring whitespace changes, etc.
*
* But note that we special-case additions, deletions,
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 4715e75b68..6eb344be03 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -252,11 +252,7 @@ test_expect_success 'changed commit with --stat diff option' '
git range-diff --no-color --stat topic...changed >actual &&
cat >expect <<-EOF &&
1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/
- a => b | 0
- 1 file changed, 0 insertions(+), 0 deletions(-)
2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/
- a => b | 0
- 1 file changed, 0 insertions(+), 0 deletions(-)
3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/
a => b | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff.c | 12 +++++++-----
t/t3206-range-diff.sh | 12 ++++--------
2 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/diff.c b/diff.c
index ee8e8189e9..77e0bd772e 100644
--- a/diff.c
+++ b/diff.c
@@ -3663,7 +3663,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
{
mmfile_t mf1, mf2;
struct diffstat_file *data;
- int same_contents;
+ int same_file;
int complete_rewrite = 0;
if (!DIFF_PAIR_UNMERGED(p)) {
@@ -3681,12 +3681,14 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
return;
}
- same_contents = oideq(&one->oid, &two->oid);
+ /* saves some reads if true, not a guarantee of diff outcome */
+ same_file = one->oid_valid && two->oid_valid &&
+ oideq(&one->oid, &two->oid);
if (diff_filespec_is_binary(o->repo, one) ||
diff_filespec_is_binary(o->repo, two)) {
data->is_binary = 1;
- if (same_contents) {
+ if (same_file) {
data->added = 0;
data->deleted = 0;
} else {
@@ -3702,7 +3704,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
data->added = count_lines(two->data, two->size);
}
- else if (!same_contents) {
+ else if (!same_file) {
/* Crazy xdl interfaces.. */
xpparam_t xpp;
xdemitconf_t xecfg;
@@ -3727,7 +3729,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
diffstat->files[diffstat->nr - 1];
/*
* Omit diffstats of modified files where nothing changed.
- * Even if !same_contents, this might be the case due to
+ * Even if !same_file, this might be the case due to
* ignoring whitespace changes, etc.
*
* But note that we special-case additions, deletions,
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index e024cff65c..6eb344be03 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -252,17 +252,13 @@ test_expect_success 'changed commit with --stat diff option' '
git range-diff --no-color --stat topic...changed >actual &&
cat >expect <<-EOF &&
1: $(test_oid t1) = 1: $(test_oid c1) s/5/A/
- a => b | 0
- 1 file changed, 0 insertions(+), 0 deletions(-)
2: $(test_oid t2) = 2: $(test_oid c2) s/4/A/
- a => b | 0
- 1 file changed, 0 insertions(+), 0 deletions(-)
3: $(test_oid t3) ! 3: $(test_oid c3) s/11/B/
- a => b | 0
- 1 file changed, 0 insertions(+), 0 deletions(-)
+ a => b | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
4: $(test_oid t4) ! 4: $(test_oid c4) s/12/B/
- a => b | 0
- 1 file changed, 0 insertions(+), 0 deletions(-)
+ a => b | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
EOF
test_cmp expect actual
'
--
2.20.1
next prev parent reply other threads:[~2020-09-24 5:26 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-18 11:32 Allow passing pipes to diff --no-index + bugfix Thomas Guyot-Sionnest
2020-09-18 11:32 ` [PATCH 1/2] diff: Fix modified lines stats with --stat and --numstat Thomas Guyot-Sionnest
2020-09-18 14:46 ` Taylor Blau
2020-09-18 15:10 ` Thomas Guyot-Sionnest
2020-09-18 17:37 ` Jeff King
2020-09-18 18:00 ` Thomas Guyot-Sionnest
2020-09-20 4:53 ` Thomas Guyot
2020-09-18 17:27 ` Jeff King
2020-09-18 17:52 ` Thomas Guyot-Sionnest
2020-09-18 18:06 ` Junio C Hamano
2020-09-23 19:16 ` Johannes Schindelin
2020-09-23 19:23 ` Junio C Hamano
2020-09-23 20:44 ` Johannes Schindelin
2020-09-24 4:49 ` Thomas Guyot
2020-09-24 5:24 ` Thomas Guyot-Sionnest [this message]
2020-09-24 7:41 ` [PATCH v4] " Thomas Guyot-Sionnest
2020-09-24 6:40 ` [PATCH 1/2] " Junio C Hamano
2020-09-24 7:13 ` Thomas Guyot
2020-09-24 17:19 ` Junio C Hamano
2020-09-24 17:38 ` Junio C Hamano
2020-09-23 15:05 ` Johannes Schindelin
2020-09-20 13:09 ` [PATCH v2] " Thomas Guyot-Sionnest
2020-09-20 15:39 ` Taylor Blau
2020-09-20 16:38 ` Thomas Guyot
2020-09-20 19:11 ` Junio C Hamano
2020-09-20 20:08 ` Junio C Hamano
2020-09-20 20:36 ` Junio C Hamano
2020-09-20 22:15 ` Junio C Hamano
2020-09-21 19:26 ` Jeff King
2020-09-21 21:51 ` Junio C Hamano
2020-09-21 22:20 ` Jeff King
2020-09-21 22:37 ` Junio C Hamano
2020-09-18 11:32 ` [PATCH 2/2] Allow passing pipes for input pipes to diff --no-index Thomas Guyot-Sionnest
2020-09-18 14:36 ` Taylor Blau
2020-09-18 16:34 ` Thomas Guyot-Sionnest
2020-09-18 17:19 ` Jeff King
2020-09-18 17:21 ` Jeff King
2020-09-18 17:39 ` Thomas Guyot-Sionnest
2020-09-18 17:48 ` Junio C Hamano
2020-09-18 18:02 ` Jeff King
2020-09-20 12:54 ` Thomas Guyot
2020-09-21 19:31 ` Jeff King
2020-09-21 20:14 ` Junio C Hamano
2020-09-18 17:58 ` Taylor Blau
2020-09-18 18:05 ` Jeff King
2020-09-18 17:20 ` Jeff King
2020-09-18 18:00 ` Taylor Blau
2020-09-18 21:56 ` brian m. carlson
2020-09-18 17:51 ` Allow passing pipes to diff --no-index + bugfix Junio C Hamano
2020-09-18 18:24 ` Thomas Guyot-Sionnest
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200924052406.11349-1-tguyot@gmail.com \
--to=tguyot@gmail.com \
--cc=Johannes.Schindelin@gmx.de \
--cc=dermoth@aei.ca \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=me@ttaylorr.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).