{"thread":{"id":"30820","subject":"[PATCH v2] diff: Only count lines in show_shortstats","startedAt":"2012-06-15T19:02:48Z","lastAt":"2012-06-15T21:19:41Z","messageCount":3,"participants":["Alexander Strasser","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"193722","messageId":"20120615190248.GA28377@akuma","threadId":"30820","inReplyTo":null,"subject":"[PATCH v2] diff: Only count lines in show_shortstats","fromName":"Alexander Strasser","fromEmail":"eclipse7@gmx.net","sentAt":"2012-06-15T19:02:48Z","receivedAt":"2012-06-15T19:02:48Z","isPatch":true,"sender":{"key":"eclipse7@gmx.net","avatar":"https://avatars.githubusercontent.com/u/4342576?v=4"},"body":"Do not mix byte and line counts. Binary files have byte counts;\nskip them when accumulating line insertions/deletions.\n\nThe regression was introduced in e18872b.\n\nSigned-off-by: Alexander Strasser <eclipse7@gmx.net>\n---\n\n Zbigniew, Junio:\n   I hope I did submit the patch correctly this time.\n\n   This is a reroll with the following differences to v1:\n\n   * I changed the additional test for t4012 to adhere to modern\n     style on request by Zbigniew. I had the impression this might\n     be in conflict with Junio's comment\n     \"Style fixes should be done later after dust from more important\n      changes (e.g. a bugfix) settles.\"\n     But maybe that was directed at modernizing the remaining of\n     parts of that test file.\n   * I deleted the 2-space indent in the commit message paragraphs\n   * I omitted the parenthesis in the subject message\n\n   The above points are the reason I resent this for discussion to\n the list.\n\n   I apologize for the long delay, some misunderstandings on my side\n made me think the initial submission was considered for inclusion.\n\n diff.c                 | 2 +-\n t/t4012-diff-binary.sh | 6 ++++++\n 2 files changed, 7 insertions(+), 1 deletion(-)\n\ndiff --git a/diff.c b/diff.c\nindex 77edd50..1a594df 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1700,7 +1700,7 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option\n \t\t\tcontinue;\n \t\tif (!data->files[i]->is_renamed && (added + deleted == 0)) {\n \t\t\ttotal_files--;\n-\t\t} else {\n+\t\t} else if (!data->files[i]->is_binary) { /* don't count bytes */\n \t\t\tadds += added;\n \t\t\tdels += deleted;\n \t\t}\ndiff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh\nindex 8b4e80d..7d03c1d 100755\n--- a/t/t4012-diff-binary.sh\n+++ b/t/t4012-diff-binary.sh\n@@ -36,6 +36,12 @@ test_expect_success '\"apply --stat\" output for binary file change' '\n \ttest_i18ncmp expected current\n '\n \n+test_expect_success 'diff --shortstat output for binary file change' '\n+\techo \" 4 files changed, 2 insertions(+), 2 deletions(-)\" >expected &&\n+\tgit diff --shortstat >current &&\n+\ttest_i18ncmp expected current\n+'\n+\n test_expect_success 'apply --numstat notices binary file change' '\n \tgit diff >diff &&\n \tgit apply --numstat <diff >current &&\n-- \n1.7.10.2.552.gaa3bb87\n"},{"id":"193727","messageId":"7vr4tg9xhr.fsf@alter.siamese.dyndns.org","threadId":"30820","inReplyTo":"20120615190248.GA28377@akuma","subject":"Re: [PATCH v2] diff: Only count lines in show_shortstats","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-06-15T19:38:24Z","receivedAt":"2012-06-15T19:38:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alexander Strasser <eclipse7@gmx.net> writes:\n\n> Do not mix byte and line counts. Binary files have byte counts;\n> skip them when accumulating line insertions/deletions.\n>\n> The regression was introduced in e18872b.\n>\n> Signed-off-by: Alexander Strasser <eclipse7@gmx.net>\n> ---\n\nAdministrivia.\n\nPlease do not use Mail-Followup-To: header to deflect direct\nresponse to you away to other people.  When I want to reply to you\nand Cc: others, I do not want to see other people's name on To:\nfield---I have to move them manually to the Cc: line in my editor.\nWhen somebody else wants to reply to you, I do not want to see my\nname on its To: line, as such a message that is addressed to you may\nnot be of immediate interest for me.\n\n>\n>  Zbigniew, Junio:\n>    I hope I did submit the patch correctly this time.\n>\n>    This is a reroll with the following differences to v1:\n>\n>    * I changed the additional test for t4012 to adhere to modern\n>      style on request by Zbigniew. I had the impression this might\n>      be in conflict with Junio's comment\n>      \"Style fixes should be done later after dust from more important\n>       changes (e.g. a bugfix) settles.\"\n>      But maybe that was directed at modernizing the remaining of\n>      parts of that test file.\n\nYes, that \"maybe\" is correct.\n\n>    * I deleted the 2-space indent in the commit message paragraphs\n\nOK.\n\n>    * I omitted the parenthesis in the subject message\n\nOK.\n\n>  diff.c                 | 2 +-\n>  t/t4012-diff-binary.sh | 6 ++++++\n>  2 files changed, 7 insertions(+), 1 deletion(-)\n>\n> diff --git a/diff.c b/diff.c\n> index 77edd50..1a594df 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1700,7 +1700,7 @@ static void show_shortstats(struct diffstat_t *data, struct diff_options *option\n>  \t\t\tcontinue;\n>  \t\tif (!data->files[i]->is_renamed && (added + deleted == 0)) {\n>  \t\t\ttotal_files--;\n> -\t\t} else {\n> +\t\t} else if (!data->files[i]->is_binary) { /* don't count bytes */\n>  \t\t\tadds += added;\n>  \t\t\tdels += deleted;\n>  \t\t}\n> diff --git a/t/t4012-diff-binary.sh b/t/t4012-diff-binary.sh\n> index 8b4e80d..7d03c1d 100755\n> --- a/t/t4012-diff-binary.sh\n> +++ b/t/t4012-diff-binary.sh\n> @@ -36,6 +36,12 @@ test_expect_success '\"apply --stat\" output for binary file change' '\n>  \ttest_i18ncmp expected current\n>  '\n>  \n> +test_expect_success 'diff --shortstat output for binary file change' '\n> +\techo \" 4 files changed, 2 insertions(+), 2 deletions(-)\" >expected &&\n> +\tgit diff --shortstat >current &&\n> +\ttest_i18ncmp expected current\n> +'\n> +\n\nIt would also have been interesting if we can see the result for a\ndiff that involves _only_ binary files, no?\n\n>  test_expect_success 'apply --numstat notices binary file change' '\n>  \tgit diff >diff &&\n>  \tgit apply --numstat <diff >current &&\n\nThanks.\n"},{"id":"193741","messageId":"20120615211941.GA26486@akuma","threadId":"30820","inReplyTo":"7vr4tg9xhr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] diff: Only count lines in show_shortstats","fromName":"Alexander Strasser","fromEmail":"eclipse7@gmx.net","sentAt":"2012-06-15T21:19:41Z","receivedAt":"2012-06-15T21:19:41Z","isPatch":true,"sender":{"key":"eclipse7@gmx.net","avatar":"https://avatars.githubusercontent.com/u/4342576?v=4"},"body":"Hi,\n\nJunio C Hamano wrote:\n> Alexander Strasser <eclipse7@gmx.net> writes:\n> \n> > Do not mix byte and line counts. Binary files have byte counts;\n> > skip them when accumulating line insertions/deletions.\n> >\n> > The regression was introduced in e18872b.\n> >\n> > Signed-off-by: Alexander Strasser <eclipse7@gmx.net>\n> > ---\n> \n> Administrivia.\n> \n> Please do not use Mail-Followup-To: header to deflect direct\n\n  I did not know about that mail header. I am not sure about the\nexact ramifications but I hope I told my MUA to stop inserting\nthat header behind my back.\n\n[...]\n> > +test_expect_success 'diff --shortstat output for binary file change' '\n> > +\techo \" 4 files changed, 2 insertions(+), 2 deletions(-)\" >expected &&\n> > +\tgit diff --shortstat >current &&\n> > +\ttest_i18ncmp expected current\n> > +'\n> > +\n> \n> It would also have been interesting if we can see the result for a\n> diff that involves _only_ binary files, no?\n\n  Seems like an interesting test to me. I will add it and send as v3\nin a moment.\n\n[...]\n\n  Alexander\n"}]}