diff: Only count lines in show_shortstats
PATCH v2 1/1 diff: Only count lines in show_shortstats
Do not mix byte and line counts. Binary files have byte counts; skip them when accumulating line insertions/deletions.
The regression was introduced in e18872b.
---
Zbigniew, Junio:
I hope I did submit the patch correctly this time.
This is a reroll with the following differences to v1:
* I changed the additional test for t4012 to adhere to modern style on request by Zbigniew. I had the impression this might be in conflict with Junio's comment
"Style fixes should be done later after dust from more important
changes (e.g. a bugfix) settles."
But maybe that was directed at modernizing the remaining of parts of that test file.
* I deleted the 2-space indent in the commit message paragraphs
* I omitted the parenthesis in the subject message
The above points are the reason I resent this for discussion to
the list.
I apologize for the long delay, some misunderstandings on my side
made me think the initial submission was considered for inclusion.
diff.c | 2 +- t/t4012-diff-binary.sh | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff.c +1 -1
@@ -1700,7 +1700,7 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *optioncontinue ;if (!data->files[i]->is_renamed && (added + deleted ==0 )) { total_files--; - }else { + }else if (!data->files[i]->is_binary) {/* don't count bytes */ adds += added; dels += deleted; }
t/t4012-diff-binary.sh +6 -0
@@ -36,6 +36,12 @@ test_expect_success '"apply --stat" output for binary file change' ' test_i18ncmp expected current ' +test_expect_success 'diff --shortstat output for binary file change' ' + echo " 4 files changed, 2 insertions(+), 2 deletions(-)" >expected && + git diff --shortstat >current && + test_i18ncmp expected current +' + test_expect_success 'apply --numstat notices binary file change' ' git diff >diff && git apply --numstat <diff >current &&
Signature · 1 line
1.7.10.2.552.gaa3bb87
Quoted 7 lines · Alexander Strasser
Do not mix byte and line counts. Binary files have byte counts; skip them when accumulating line insertions/deletions.
The regression was introduced in e18872b.
Signed-off-by Alexander Strasser <[email protected]>---
Administrivia.
Please do not use Mail-Followup-To: header to deflect direct response to you away to other people. When I want to reply to you and Cc: others, I do not want to see other people's name on To:
field---I have to move them manually to the Cc: line in my editor. When somebody else wants to reply to you, I do not want to see my name on its To: line, as such a message that is addressed to you may not be of immediate interest for me.
Quoted 6 lines
Zbigniew, Junio:
I hope I did submit the patch correctly this time.This is a reroll with the following differences to v1:
Quoted 7 lines
* I changed the additional test for t4012 to adhere to modern style on request by Zbigniew. I had the impression this might be in conflict with Junio's comment
"Style fixes should be done later after dust from more important
changes (e.g. a bugfix) settles."
But maybe that was directed at modernizing the remaining of parts of that test file.
Yes, that "maybe" is correct.
Quoted 1 line
* I deleted the 2-space indent in the commit message paragraphs
OK.
Quoted 1 line
* I omitted the parenthesis in the subject message
OK.
Quoted 22 lines
diff.c | 2 +- t/t4012-diff-binary.sh | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-)diff.c +1
-1@@ -1700,7 +1700,7 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *optioncontinue ;if (!data->files[i]->is_renamed && (added + deleted ==0 )) { total_files--; - }else { + }else if (!data->files[i]->is_binary) {/* don't count bytes */ adds += added; dels += deleted; }t/t4012-diff-binary.sh +0
-0
t/t4012-diff-binary.sh test_expect_success '"apply --stat" output for binary file change' ' · line 44
Quoted 9 lines
t/t4012-diff-binary.sh +6
-0@@ -36,6 +36,12 @@ test_expect_success '"apply --stat" output for binary file change' ' test_i18ncmp expected current ' +test_expect_success 'diff --shortstat output for binary file change' ' + echo " 4 files changed, 2 insertions(+), 2 deletions(-)" >expected && + git diff --shortstat >current && + test_i18ncmp expected current +' +
It would also have been interesting if we can see the result for a diff that involves _only_ binary files, no?
t/t4012-diff-binary.sh test_expect_success '"apply --stat" output for binary file change' ' · line 47
Quoted 3 lines
t/t4012-diff-binary.sh +0
-0test_expect_success 'apply --numstat notices binary file change' ' git diff >diff && git apply --numstat <diff >current &&
Thanks.
Hi,
Quoted 5 lines · Junio C Hamano
Quoted 3 lines · Alexander Strasser
Do not mix byte and line counts. Binary files have byte counts; skip them when accumulating line insertions/deletions.
Quoted 8 lines
Quoted 4 lines
The regression was introduced in e18872b.
Signed-off-by Alexander Strasser <[email protected]>---
Administrivia.
Please do not use Mail-Followup-To: header to deflect direct
I did not know about that mail header. I am not sure about the
exact ramifications but I hope I told my MUA to stop inserting that header behind my back.
[...]
Quoted 9 lines
t/t4012-diff-binary.sh test_expect_success '"apply --stat" output for binary file change' ' · line 44
Quoted 6 lines
t/t4012-diff-binary.sh +6
-0+test_expect_success 'diff --shortstat output for binary file change' ' + echo " 4 files changed, 2 insertions(+), 2 deletions(-)" >expected && + git diff --shortstat >current && + test_i18ncmp expected current +' +It would also have been interesting if we can see the result for a diff that involves _only_ binary files, no?
Seems like an interesting test to me. I will add it and send as v3
in a moment.
[...]
Alexander