Re: [eclipse7@gmx.net: [PATCH] diff: Only count lines in show_shortstats()]
From: Zbigniew Jędrzejewski-Szmek <hidden>
Date: 2016-06-15 22:54:00
On 06/07/2012 02:21 PM, Alexander Strasser wrote:
Hello Zbigniew, could you have a look at the patch below? I submitted to it to the Git mailing list and you could probably comment there?
Hi Alexander, sure, thanks for finding (and fixing) the bug.
I think I should have put you in CC. But I am not so sure about Git patch submission policies.
The policy is to CC everyone who might be interested, and also to add TO:gitster@pobox.com, if the patch is intended for merging, as yours is. So basically taking the address list from the discussion of e18872b would be the simplest and most effective choice.
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.
Yeah, it seems that the condition for !binary was lost in the refactoring of the code.
Signed-off-by: Alexander Strasser <redacted>
Small note: normally the paragraphs are not indented.
quoted hunk
--- I hope this does retain the original intent of e18872b while not messing up the insertions/deletions output by --shortstat. Output of --stat was never affected AFAICT. diff.c | 2 +- t/t4012-diff-binary.sh | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-)diff --git a/diff.c b/diff.c index 77edd50..1a594df 100644 --- a/diff.c +++ b/diff.c@@ -1700,7 +1700,7 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option continue; 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; }diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh index 8b4e80d..1a994f0 100755 --- a/t/t4012-diff-binary.sh +++ b/t/t4012-diff-binary.sh@@ -36,6 +36,14 @@ test_expect_success '"apply --stat" output for binary file change' ' test_i18ncmp expected current ' +cat > expected <<\EOF + 4 files changed, 2 insertions(+), 2 deletions(-) +EOF +test_expect_success 'diff with --shortstat' ' + git diff --shortstat >current && + test_cmp expected current +' +
The test is OK, and follows the style of surrounding tests, but current style is slightly different: - no space after '>' - expected output is inlined if it is short - test_i18ncmp is used, even if the message is not yet i18n-ized Something like this: test_expect_success 'diff --shortstat output for binary file change' ' echo " 4 files changed, 2 insertions(+), 2 deletions(-)" >expect && git diff --shortstat >current && test_i18ncmp expect current '
test_expect_success 'apply --numstat notices binary file change' ' git diff >diff && git apply --numstat <diff >current &&
Zbyszek