From: "Zbigniew Jędrzejewski-Szmek" <zbyszek@in.waw.pl>
To: Alexander Strasser <eclipse7@gmx.net>,
"git@vger.kernel.org" <git@vger.kernel.org>,
Junio C Hamano <gitster@pobox.com>
Cc: mj@ucw.cz, Johannes Sixt <j.sixt@viscovery.net>
Subject: Re: [eclipse7@gmx.net: [PATCH] diff: Only count lines in show_shortstats()]
Date: Thu, 07 Jun 2012 21:05:25 +0200 [thread overview]
Message-ID: <4FD0FB75.4090906@in.waw.pl> (raw)
In-Reply-To: <20120607122149.GA3070@akuma>
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 <eclipse7@gmx.net>
Small note: normally the paragraphs are not indented.
> ---
>
> 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
next parent reply other threads:[~2012-06-07 19:05 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20120607122149.GA3070@akuma>
2012-06-07 19:05 ` Zbigniew Jędrzejewski-Szmek [this message]
2012-06-07 20:04 ` [eclipse7@gmx.net: [PATCH] diff: Only count lines in show_shortstats()] Alexander Strasser
2012-06-07 20:29 ` Junio C Hamano
2012-06-14 19:07 ` Zbigniew Jędrzejewski-Szmek
2012-06-14 20:28 ` Junio C Hamano
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=4FD0FB75.4090906@in.waw.pl \
--to=zbyszek@in.waw.pl \
--cc=eclipse7@gmx.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=j.sixt@viscovery.net \
--cc=mj@ucw.cz \
/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).