{"thread":{"id":"33350","subject":"[PATCH] count-objects: output \"KiB\" instead of \"kilobytes\"","startedAt":"2013-04-02T11:43:30Z","lastAt":"2013-04-10T20:12:00Z","messageCount":21,"participants":["Mihai Capotă","Junio C Hamano","Antoine Pelisse","Eric Sunshine","Jonathan Nieder"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"212874","messageId":"1364903010-644-1-git-send-email-mihai@mihaic.ro","threadId":"33350","inReplyTo":null,"subject":"[PATCH] count-objects: output \"KiB\" instead of \"kilobytes\"","fromName":"Mihai Capotă","fromEmail":"mihai@mihaic.ro","sentAt":"2013-04-02T11:43:30Z","receivedAt":"2013-04-02T11:43:30Z","isPatch":true,"sender":{"key":"mihai@mihaic.ro","avatar":"https://avatars.githubusercontent.com/u/165546?v=4"},"body":"The code uses division by 1024. Also, the manual uses \"KiB\".\n\nSigned-off-by: Mihai Capotă <mihai@mihaic.ro>\n---\n builtin/count-objects.c |    2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/count-objects.c b/builtin/count-objects.c\nindex 9afaa88..ecc13b0 100644\n--- a/builtin/count-objects.c\n+++ b/builtin/count-objects.c\n@@ -124,7 +124,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t\tprintf(\"garbage: %lu\\n\", garbage);\n \t}\n \telse\n-\t\tprintf(\"%lu objects, %lu kilobytes\\n\",\n+\t\tprintf(\"%lu objects, %lu KiB\\n\",\n \t\t       loose, (unsigned long) (loose_size / 1024));\n \treturn 0;\n }\n-- \n1.7.9.5\n"},{"id":"212923","messageId":"7vhajoesp9.fsf@alter.siamese.dyndns.org","threadId":"33350","inReplyTo":"1364903010-644-1-git-send-email-mihai@mihaic.ro","subject":"Re: [PATCH] count-objects: output \"KiB\" instead of \"kilobytes\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-02T17:41:06Z","receivedAt":"2013-04-02T17:41:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mihai Capotă <mihai@mihaic.ro> writes:\n\n> The code uses division by 1024. Also, the manual uses \"KiB\".\n>\n> Signed-off-by: Mihai Capotă <mihai@mihaic.ro>\n> ---\n>  builtin/count-objects.c |    2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/count-objects.c b/builtin/count-objects.c\n> index 9afaa88..ecc13b0 100644\n> --- a/builtin/count-objects.c\n> +++ b/builtin/count-objects.c\n> @@ -124,7 +124,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n>  \t\tprintf(\"garbage: %lu\\n\", garbage);\n>  \t}\n>  \telse\n> -\t\tprintf(\"%lu objects, %lu kilobytes\\n\",\n> +\t\tprintf(\"%lu objects, %lu KiB\\n\",\n>  \t\t       loose, (unsigned long) (loose_size / 1024));\n>  \treturn 0;\n>  }\n\nI guess nobody reads this in scripts, so it should be OK.\n\nWill queue. Thanks.\n"},{"id":"212999","messageId":"7vip44a8xl.fsf@alter.siamese.dyndns.org","threadId":"33350","inReplyTo":"1364903010-644-1-git-send-email-mihai@mihaic.ro","subject":"Re: [PATCH] count-objects: output \"KiB\" instead of \"kilobytes\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-02T22:01:42Z","receivedAt":"2013-04-02T22:01:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mihai Capotă <mihai@mihaic.ro> writes:\n\n> The code uses division by 1024. Also, the manual uses \"KiB\".\n>\n> Signed-off-by: Mihai Capotă <mihai@mihaic.ro>\n> ---\n>  builtin/count-objects.c |    2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/count-objects.c b/builtin/count-objects.c\n> index 9afaa88..ecc13b0 100644\n> --- a/builtin/count-objects.c\n> +++ b/builtin/count-objects.c\n> @@ -124,7 +124,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n>  \t\tprintf(\"garbage: %lu\\n\", garbage);\n>  \t}\n>  \telse\n> -\t\tprintf(\"%lu objects, %lu kilobytes\\n\",\n> +\t\tprintf(\"%lu objects, %lu KiB\\n\",\n>  \t\t       loose, (unsigned long) (loose_size / 1024));\n>  \treturn 0;\n>  }\n\nThis breaks existing tests (5301, 7408 and 5700); I noticed it too\nlate and wasted 20 minutes, having to re-run today's integration\ncycle.\n\nNext time, please run the testsuite before sending a patch.\n"},{"id":"213024","messageId":"CADyhzG1srEqiDdo8bAB+Hw=DaRB2vkwOoCHzYtpiuUiZHEo4LQ@mail.gmail.com","threadId":"33350","inReplyTo":"7vip44a8xl.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] count-objects: output \"KiB\" instead of \"kilobytes\"","fromName":"Mihai Capotă","fromEmail":"mihai@mihaic.ro","sentAt":"2013-04-03T06:27:55Z","receivedAt":"2013-04-03T06:27:55Z","isPatch":true,"sender":{"key":"mihai@mihaic.ro","avatar":"https://avatars.githubusercontent.com/u/165546?v=4"},"body":"I'm really sorry about that. I'll make sure to run the tests before\nsending patches in the future.\n\nOn Wed, Apr 3, 2013 at 12:01 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Mihai Capotă <mihai@mihaic.ro> writes:\n>\n>> The code uses division by 1024. Also, the manual uses \"KiB\".\n>>\n>> Signed-off-by: Mihai Capotă <mihai@mihaic.ro>\n>> ---\n>>  builtin/count-objects.c |    2 +-\n>>  1 file changed, 1 insertion(+), 1 deletion(-)\n>>\n>> diff --git a/builtin/count-objects.c b/builtin/count-objects.c\n>> index 9afaa88..ecc13b0 100644\n>> --- a/builtin/count-objects.c\n>> +++ b/builtin/count-objects.c\n>> @@ -124,7 +124,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n>>               printf(\"garbage: %lu\\n\", garbage);\n>>       }\n>>       else\n>> -             printf(\"%lu objects, %lu kilobytes\\n\",\n>> +             printf(\"%lu objects, %lu KiB\\n\",\n>>                      loose, (unsigned long) (loose_size / 1024));\n>>       return 0;\n>>  }\n>\n> This breaks existing tests (5301, 7408 and 5700); I noticed it too\n> late and wasted 20 minutes, having to re-run today's integration\n> cycle.\n>\n> Next time, please run the testsuite before sending a patch.\n"},{"id":"213031","messageId":"1364993331-20199-1-git-send-email-mihai@mihaic.ro","threadId":"33350","inReplyTo":"7vip44a8xl.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] count-objects: output \"KiB\" instead of \"kilobytes\"","fromName":"Mihai Capotă","fromEmail":"mihai@mihaic.ro","sentAt":"2013-04-03T12:48:51Z","receivedAt":"2013-04-03T12:48:51Z","isPatch":true,"sender":{"key":"mihai@mihaic.ro","avatar":"https://avatars.githubusercontent.com/u/165546?v=4"},"body":"The code uses division by 1024. The master branch count-objects manual also\nuses \"KiB\".\n\nAlso updated the code that reads count-objects output (t5301, t5700, t7408, and\ngit-cvsimport) and the Git User's Manual.\n\nSigned-off-by: Mihai Capotă <mihai@mihaic.ro>\n---\n Documentation/user-manual.txt  |    4 ++--\n builtin/count-objects.c        |    2 +-\n git-cvsimport.perl             |    8 ++++----\n t/t5301-sliding-window.sh      |    4 ++--\n t/t5700-clone-reference.sh     |    4 ++--\n t/t7408-submodule-reference.sh |    4 ++--\n 6 files changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt\nindex e831cc2..b61a09c 100644\n--- a/Documentation/user-manual.txt\n+++ b/Documentation/user-manual.txt\n@@ -3175,7 +3175,7 @@ lot of objects.  Try this on an old project:\n \n ------------------------------------------------\n $ git count-objects\n-6930 objects, 47620 kilobytes\n+6930 objects, 47620 KiB\n ------------------------------------------------\n \n The first number is the number of objects which are kept in\n@@ -3215,7 +3215,7 @@ You can verify that the loose objects are gone by looking at the\n \n ------------------------------------------------\n $ git count-objects\n-0 objects, 0 kilobytes\n+0 objects, 0 KiB\n ------------------------------------------------\n \n Although the object files are gone, any commands that refer to those\ndiff --git a/builtin/count-objects.c b/builtin/count-objects.c\nindex 9afaa88..ecc13b0 100644\n--- a/builtin/count-objects.c\n+++ b/builtin/count-objects.c\n@@ -124,7 +124,7 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t\tprintf(\"garbage: %lu\\n\", garbage);\n \t}\n \telse\n-\t\tprintf(\"%lu objects, %lu kilobytes\\n\",\n+\t\tprintf(\"%lu objects, %lu KiB\\n\",\n \t\t       loose, (unsigned long) (loose_size / 1024));\n \treturn 0;\n }\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 73d367c..de44e33 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -1126,12 +1126,12 @@ unless ($opt_P) {\n }\n \n # The heuristic of repacking every 1024 commits can leave a\n-# lot of unpacked data.  If there is more than 1MB worth of\n+# lot of unpacked data.  If there is more than 1MiB worth of\n # not-packed objects, repack once more.\n my $line = `git count-objects`;\n-if ($line =~ /^(\\d+) objects, (\\d+) kilobytes$/) {\n-  my ($n_objects, $kb) = ($1, $2);\n-  1024 < $kb\n+if ($line =~ /^(\\d+) objects, (\\d+) KiB$/) {\n+  my ($n_objects, $kib) = ($1, $2);\n+  1024 < $kib\n     and system(qw(git repack -a -d));\n }\n \ndiff --git a/t/t5301-sliding-window.sh b/t/t5301-sliding-window.sh\nindex 2fc5af6..37931d2 100755\n--- a/t/t5301-sliding-window.sh\n+++ b/t/t5301-sliding-window.sh\n@@ -20,7 +20,7 @@ test_expect_success \\\n      commit1=`git commit-tree $tree </dev/null` &&\n      git update-ref HEAD $commit1 &&\n      git repack -a -d &&\n-     test \"`git count-objects`\" = \"0 objects, 0 kilobytes\" &&\n+     test \"`git count-objects`\" = \"0 objects, 0 KiB\" &&\n      pack1=`ls .git/objects/pack/*.pack` &&\n      test -f \"$pack1\"'\n \n@@ -46,7 +46,7 @@ test_expect_success \\\n      commit2=`git commit-tree $tree -p $commit1 </dev/null` &&\n      git update-ref HEAD $commit2 &&\n      git repack -a -d &&\n-     test \"`git count-objects`\" = \"0 objects, 0 kilobytes\" &&\n+     test \"`git count-objects`\" = \"0 objects, 0 KiB\" &&\n      pack2=`ls .git/objects/pack/*.pack` &&\n      test -f \"$pack2\" &&\n      test \"$pack1\" \\!= \"$pack2\"'\ndiff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh\nindex c47d450..e5cfd6a 100755\n--- a/t/t5700-clone-reference.sh\n+++ b/t/t5700-clone-reference.sh\n@@ -46,7 +46,7 @@ cd \"$base_dir\"\n \n test_expect_success 'that reference gets used' \\\n 'cd C &&\n-echo \"0 objects, 0 kilobytes\" > expected &&\n+echo \"0 objects, 0 KiB\" > expected &&\n git count-objects > current &&\n test_cmp expected current'\n \n@@ -73,7 +73,7 @@ test_expect_success 'pulling from reference' \\\n cd \"$base_dir\"\n \n test_expect_success 'that reference gets used' \\\n-'cd D && echo \"0 objects, 0 kilobytes\" > expected &&\n+'cd D && echo \"0 objects, 0 KiB\" > expected &&\n git count-objects > current &&\n test_cmp expected current'\n \ndiff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh\nindex b770b2f..aeface6 100755\n--- a/t/t7408-submodule-reference.sh\n+++ b/t/t7408-submodule-reference.sh\n@@ -49,7 +49,7 @@ cd \"$base_dir\"\n \n test_expect_success 'that reference gets used with add' \\\n 'cd super/sub &&\n-echo \"0 objects, 0 kilobytes\" > expected &&\n+echo \"0 objects, 0 KiB\" > expected &&\n git count-objects > current &&\n diff expected current'\n \n@@ -72,7 +72,7 @@ cd \"$base_dir\"\n \n test_expect_success 'that reference gets used with update' \\\n 'cd super-clone/sub &&\n-echo \"0 objects, 0 kilobytes\" > expected &&\n+echo \"0 objects, 0 KiB\" > expected &&\n git count-objects > current &&\n diff expected current'\n \n-- \n1.7.9.5\n"},{"id":"213036","messageId":"7vd2ub7k7c.fsf@alter.siamese.dyndns.org","threadId":"33350","inReplyTo":"1364993331-20199-1-git-send-email-mihai@mihaic.ro","subject":"Re: [PATCH v2] count-objects: output \"KiB\" instead of \"kilobytes\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-03T14:38:47Z","receivedAt":"2013-04-03T14:38:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mihai Capotă <mihai@mihaic.ro> writes:\n\n> The code uses division by 1024. The master branch count-objects manual also\n> uses \"KiB\".\n>\n> Also updated the code that reads count-objects output (t5301, t5700, t7408, and\n> git-cvsimport) and the Git User's Manual.\n>\n> Signed-off-by: Mihai Capotă <mihai@mihaic.ro>\n> ---\n>  Documentation/user-manual.txt  |    4 ++--\n>  builtin/count-objects.c        |    2 +-\n>  git-cvsimport.perl             |    8 ++++----\n>  t/t5301-sliding-window.sh      |    4 ++--\n>  t/t5700-clone-reference.sh     |    4 ++--\n>  t/t7408-submodule-reference.sh |    4 ++--\n>  6 files changed, 13 insertions(+), 13 deletions(-)\n>\n> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt\n> index e831cc2..b61a09c 100644\n> --- a/Documentation/user-manual.txt\n> +++ b/Documentation/user-manual.txt\n> @@ -3175,7 +3175,7 @@ lot of objects.  Try this on an old project:\n>  \n>  ------------------------------------------------\n>  $ git count-objects\n> -6930 objects, 47620 kilobytes\n> +6930 objects, 47620 KiB\n>  ------------------------------------------------\n>  \n>  The first number is the number of objects which are kept in\n> @@ -3215,7 +3215,7 @@ You can verify that the loose objects are gone by looking at the\n>  \n>  ------------------------------------------------\n>  $ git count-objects\n> -0 objects, 0 kilobytes\n> +0 objects, 0 KiB\n>  ------------------------------------------------\n>  \n>  Although the object files are gone, any commands that refer to those\n\nIt is good to see the patch being thorough, adjusting even\ndocumentation.\n\n> diff --git a/git-cvsimport.perl b/git-cvsimport.perl\n> index 73d367c..de44e33 100755\n> --- a/git-cvsimport.perl\n> +++ b/git-cvsimport.perl\n> @@ -1126,12 +1126,12 @@ unless ($opt_P) {\n>  }\n>  \n>  # The heuristic of repacking every 1024 commits can leave a\n> -# lot of unpacked data.  If there is more than 1MB worth of\n> +# lot of unpacked data.  If there is more than 1MiB worth of\n>  # not-packed objects, repack once more.\n>  my $line = `git count-objects`;\n> -if ($line =~ /^(\\d+) objects, (\\d+) kilobytes$/) {\n> -  my ($n_objects, $kb) = ($1, $2);\n> -  1024 < $kb\n> +if ($line =~ /^(\\d+) objects, (\\d+) KiB$/) {\n> +  my ($n_objects, $kib) = ($1, $2);\n> +  1024 < $kib\n>      and system(qw(git repack -a -d));\n>  }\n\nThis hunk makes me wonder if this s/kilobytes/kib/ is a good idea in\nthe first place.  This in-tree user was lucky enough to have been\ncaught and adjusted, but we don't know how many out-of-tree scripts\nare broken the same way and in need of a similar treatment.\n"},{"id":"293757","messageId":"CADyhzG3HJhrXJAoTfyHUsg=8ZmUUwUgrNfUiLHF0Ws=gSERAqw@mail.gmail.com","threadId":"33350","inReplyTo":"7vd2ub7k7c.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] count-objects: output \"KiB\" instead of \"kilobytes\"","fromName":"Mihai Capotă","fromEmail":"mihai@mihaic.ro","sentAt":"2013-04-04T13:18:25Z","receivedAt":"2013-04-04T13:18:25Z","isPatch":true,"sender":{"key":"mihai@mihaic.ro","avatar":"https://avatars.githubusercontent.com/u/165546?v=4"},"body":"On Wed, Apr 3, 2013 at 4:38 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Mihai Capotă <mihai@mihaic.ro> writes:\n>> diff --git a/git-cvsimport.perl b/git-cvsimport.perl\n>> index 73d367c..de44e33 100755\n>> --- a/git-cvsimport.perl\n>> +++ b/git-cvsimport.perl\n>> @@ -1126,12 +1126,12 @@ unless ($opt_P) {\n>>  }\n>>\n>>  # The heuristic of repacking every 1024 commits can leave a\n>> -# lot of unpacked data.  If there is more than 1MB worth of\n>> +# lot of unpacked data.  If there is more than 1MiB worth of\n>>  # not-packed objects, repack once more.\n>>  my $line = `git count-objects`;\n>> -if ($line =~ /^(\\d+) objects, (\\d+) kilobytes$/) {\n>> -  my ($n_objects, $kb) = ($1, $2);\n>> -  1024 < $kb\n>> +if ($line =~ /^(\\d+) objects, (\\d+) KiB$/) {\n>> +  my ($n_objects, $kib) = ($1, $2);\n>> +  1024 < $kib\n>>      and system(qw(git repack -a -d));\n>>  }\n>\n> This hunk makes me wonder if this s/kilobytes/kib/ is a good idea in\n> the first place.  This in-tree user was lucky enough to have been\n> caught and adjusted, but we don't know how many out-of-tree scripts\n> are broken the same way and in need of a similar treatment.\n\nThe git manual contains an explicit warning about the output of a\nporcelain command changing: \"The interface to Porcelain commands on\nthe other hand are subject to change in order to improve the end user\nexperience.\"\n\n"},{"id":"213126","messageId":"7vvc82jm77.fsf@alter.siamese.dyndns.org","threadId":"33350","inReplyTo":"CADyhzG3HJhrXJAoTfyHUsg=8ZmUUwUgrNfUiLHF0Ws=gSERAqw@mail.gmail.com","subject":"Re: [PATCH v2] count-objects: output \"KiB\" instead of \"kilobytes\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-04T16:27:08Z","receivedAt":"2013-04-04T16:27:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Mihai Capotă <mihai@mihaic.ro> writes:\n\n> The git manual contains an explicit warning about the output of a\n> porcelain command changing: \"The interface to Porcelain commands on\n> the other hand are subject to change in order to improve the end user\n> experience.\"\n\nYeah, I know that, as I wrote it ;-)\n\nAside from count-object being not exactly a Porcelain, the statement\ndoes not give us a blank check to make random changes as we see fit.\nThere needs to be a clear improvement.\n\nI am just having a hard time weighing the benefit of using more\naccurate kibibytes over kilobytes and the possible downside of\nbreaking other peoples' tools.\n\nPerhaps it would be alright if the change was accompanied by a\nwarning in the Release Notes to say something like:\n\n        If you have scripts that decide when to run \"git repack\" by\n\tparsing the output from \"git count-objects\", this release\n\tmay break them.  Sorry about that.  One of the scripts\n\tshipped by git-core itself also had to be adjusted.  The\n\tcommand reports the total diskspace used to store loose\n\tobjects in kibibytes, but it was labelled as \"kilobytes\".\n\tThe number now is shown with \"KiB\", e.g. \"6750 objects,\n\t50928 KiB\".\n\n\tYou may want to consider updating such scripts to always\n\tcall \"git gc --auto\" to let it decide when to repack for\n\tyou.\n\nAlso, I suspect that for the purpose of this exact output field,\nnobody cares the difference between kibibytes and kilobytes.\nDepending on the system, we add up either st.st_blocks or st.st_size\nand the result is not that exact as \"how much diskspace is\nconsumed\".\n"},{"id":"213262","messageId":"CADyhzG04H-9w51YsUruW58LHyPxFCLhEcd7Za2CQhDgxR4fKBw@mail.gmail.com","threadId":"33350","inReplyTo":"7vvc82jm77.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] count-objects: output \"KiB\" instead of \"kilobytes\"","fromName":"Mihai Capotă","fromEmail":"mihai@mihaic.ro","sentAt":"2013-04-05T09:38:52Z","receivedAt":"2013-04-05T09:38:52Z","isPatch":true,"sender":{"key":"mihai@mihaic.ro","avatar":"https://avatars.githubusercontent.com/u/165546?v=4"},"body":"On Thu, Apr 4, 2013 at 6:27 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Mihai Capotă <mihai@mihaic.ro> writes:\n>\n>> The git manual contains an explicit warning about the output of a\n>> porcelain command changing: \"The interface to Porcelain commands on\n>> the other hand are subject to change in order to improve the end user\n>> experience.\"\n>\n> Yeah, I know that, as I wrote it ;-)\n>\n> Aside from count-object being not exactly a Porcelain, the statement\n> does not give us a blank check to make random changes as we see fit.\n> There needs to be a clear improvement.\n>\n> I am just having a hard time weighing the benefit of using more\n> accurate kibibytes over kilobytes and the possible downside of\n> breaking other peoples' tools.\n>\n> Perhaps it would be alright if the change was accompanied by a\n> warning in the Release Notes to say something like:\n>\n>         If you have scripts that decide when to run \"git repack\" by\n>         parsing the output from \"git count-objects\", this release\n>         may break them.  Sorry about that.  One of the scripts\n>         shipped by git-core itself also had to be adjusted.  The\n>         command reports the total diskspace used to store loose\n>         objects in kibibytes, but it was labelled as \"kilobytes\".\n>         The number now is shown with \"KiB\", e.g. \"6750 objects,\n>         50928 KiB\".\n>\n>         You may want to consider updating such scripts to always\n>         call \"git gc --auto\" to let it decide when to repack for\n>         you.\n>\n> Also, I suspect that for the purpose of this exact output field,\n> nobody cares the difference between kibibytes and kilobytes.\n> Depending on the system, we add up either st.st_blocks or st.st_size\n> and the result is not that exact as \"how much diskspace is\n> consumed\".\n\nI agree completely. I think the release notes warning is a good plan.\nJust in case you decide against it, I'm also sending a completely\ndifferent patch to document the issue.\n\nMihai\n"},{"id":"213256","messageId":"1365154763-9875-1-git-send-email-mihai@mihaic.ro","threadId":"33350","inReplyTo":"7vvc82jm77.fsf@alter.siamese.dyndns.org","subject":"[PATCH] count-objects doc: document use of kibibytes","fromName":"Mihai Capotă","fromEmail":"mihai@mihaic.ro","sentAt":"2013-04-05T09:39:23Z","receivedAt":"2013-04-05T09:39:23Z","isPatch":true,"sender":{"key":"mihai@mihaic.ro","avatar":"https://avatars.githubusercontent.com/u/165546?v=4"},"body":"Document the use of kibibytes, not kilobytes, in the output of count-objects\nand the reason for not correcting the output.\n\nAlso, make cvsimport comment and variable name reflect unit actually used.\n\nSigned-off-by: Mihai Capotă <mihai@mihaic.ro>\n---\n Documentation/git-count-objects.txt |    7 +++++++\n git-cvsimport.perl                  |    6 +++---\n 2 files changed, 10 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt\nindex 23c80ce..d562dad 100644\n--- a/Documentation/git-count-objects.txt\n+++ b/Documentation/git-count-objects.txt\n@@ -26,6 +26,13 @@ OPTIONS\n \tand number of objects that can be removed by running\n \t`git prune-packed`.\n \n+\n+BUGS\n+----\n+Consumed space is actually expressed in kibibytes, not kilobytes as stated in\n+the output. The output is kept as it is for compatibility reasons.\n+\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/git-cvsimport.perl b/git-cvsimport.perl\nindex 73d367c..6803f04 100755\n--- a/git-cvsimport.perl\n+++ b/git-cvsimport.perl\n@@ -1126,12 +1126,12 @@ unless ($opt_P) {\n }\n \n # The heuristic of repacking every 1024 commits can leave a\n-# lot of unpacked data.  If there is more than 1MB worth of\n+# lot of unpacked data.  If there is more than 1MiB worth of\n # not-packed objects, repack once more.\n my $line = `git count-objects`;\n if ($line =~ /^(\\d+) objects, (\\d+) kilobytes$/) {\n-  my ($n_objects, $kb) = ($1, $2);\n-  1024 < $kb\n+  my ($n_objects, $kib) = ($1, $2);\n+  1024 < $kib\n     and system(qw(git repack -a -d));\n }\n \n-- \n1.7.9.5\n"},{"id":"213272","messageId":"CALWbr2wgJmY86Fic-eE9AbtP=HMPddTO=LDp5RGYmt6_kFawpg@mail.gmail.com","threadId":"33350","inReplyTo":"7vvc82jm77.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v2] count-objects: output \"KiB\" instead of \"kilobytes\"","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-04-05T20:31:47Z","receivedAt":"2013-04-05T20:31:47Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Should we use that opportunity to implement an option like -h (for\nhumanize) similar to what ls(1), df(1), du(1) does ? Of course \"-h\" is\nalready used for help, so we could use -H or any other sensible\nchoice.\nIt can become tough to read the size when it gets big enough.\n\nOn Thu, Apr 4, 2013 at 6:27 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Mihai Capotă <mihai@mihaic.ro> writes:\n>\n>> The git manual contains an explicit warning about the output of a\n>> porcelain command changing: \"The interface to Porcelain commands on\n>> the other hand are subject to change in order to improve the end user\n>> experience.\"\n>\n> Yeah, I know that, as I wrote it ;-)\n>\n> Aside from count-object being not exactly a Porcelain, the statement\n> does not give us a blank check to make random changes as we see fit.\n> There needs to be a clear improvement.\n>\n> I am just having a hard time weighing the benefit of using more\n> accurate kibibytes over kilobytes and the possible downside of\n> breaking other peoples' tools.\n>\n> Perhaps it would be alright if the change was accompanied by a\n> warning in the Release Notes to say something like:\n>\n>         If you have scripts that decide when to run \"git repack\" by\n>         parsing the output from \"git count-objects\", this release\n>         may break them.  Sorry about that.  One of the scripts\n>         shipped by git-core itself also had to be adjusted.  The\n>         command reports the total diskspace used to store loose\n>         objects in kibibytes, but it was labelled as \"kilobytes\".\n>         The number now is shown with \"KiB\", e.g. \"6750 objects,\n>         50928 KiB\".\n>\n>         You may want to consider updating such scripts to always\n>         call \"git gc --auto\" to let it decide when to repack for\n>         you.\n>\n> Also, I suspect that for the purpose of this exact output field,\n> nobody cares the difference between kibibytes and kilobytes.\n> Depending on the system, we add up either st.st_blocks or st.st_size\n> and the result is not that exact as \"how much diskspace is\n> consumed\".\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n"},{"id":"213570","messageId":"1365445101-10425-1-git-send-email-apelisse@gmail.com","threadId":"33350","inReplyTo":"CALWbr2wgJmY86Fic-eE9AbtP=HMPddTO=LDp5RGYmt6_kFawpg@mail.gmail.com","subject":"[PATCH 1/2] progress: create public humanize() to show sizes","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-04-08T18:18:20Z","receivedAt":"2013-04-08T18:18:20Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Currently, humanization of downloaded size is done in the same function\nas text formatting. This is an issue if anyone else wants to use this.\n\nSeparate text formatting from size simplification and make the function\npublic so that it can easily be used by other clients.\n\nWe now can use humanize() for both downloaded size and download speed\ncalculation. One of the drawbacks is that speed will no look like this\nwhen download is stalled: \"0 bytes/s\" instead of \"0 KiB/s\".\n\nSigned-off-by: Antoine Pelisse <apelisse@gmail.com>\n---\n progress.c |   60 ++++++++++++++++++++++++++++++++++--------------------------\n progress.h |    2 ++\n 2 files changed, 36 insertions(+), 26 deletions(-)\n\ndiff --git a/progress.c b/progress.c\nindex 3971f49..76c1e42 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -8,8 +8,11 @@\n  * published by the Free Software Foundation.\n  */\n \n+#include <string.h>\n+\n #include \"git-compat-util.h\"\n #include \"progress.h\"\n+#include \"strbuf.h\"\n \n #define TP_IDX_MAX      8\n \n@@ -112,34 +115,33 @@ static int display(struct progress *progress, unsigned n, const char *done)\n \treturn 0;\n }\n \n-static void throughput_string(struct throughput *tp, off_t total,\n-\t\t\t      unsigned int rate)\n+void humanize(struct strbuf *buf, off_t bytes)\n {\n-\tint l = sizeof(tp->display);\n-\tif (total > 1 << 30) {\n-\t\tl -= snprintf(tp->display, l, \", %u.%2.2u GiB\",\n-\t\t\t      (int)(total >> 30),\n-\t\t\t      (int)(total & ((1 << 30) - 1)) / 10737419);\n-\t} else if (total > 1 << 20) {\n-\t\tint x = total + 5243;  /* for rounding */\n-\t\tl -= snprintf(tp->display, l, \", %u.%2.2u MiB\",\n-\t\t\t      x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);\n-\t} else if (total > 1 << 10) {\n-\t\tint x = total + 5;  /* for rounding */\n-\t\tl -= snprintf(tp->display, l, \", %u.%2.2u KiB\",\n-\t\t\t      x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);\n+\tif (bytes > 1 << 30) {\n+\t\tstrbuf_addf(buf, \"%u.%2.2u GiB\",\n+\t\t\t    (int)(bytes >> 30),\n+\t\t\t    (int)(bytes & ((1 << 30) - 1)) / 10737419);\n+\t} else if (bytes > 1 << 20) {\n+\t\tint x = bytes + 5243;  /* for rounding */\n+\t\tstrbuf_addf(buf, \"%u.%2.2u MiB\",\n+\t\t\t    x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);\n+\t} else if (bytes > 1 << 10) {\n+\t\tint x = bytes + 5;  /* for rounding */\n+\t\tstrbuf_addf(buf, \"%u.%2.2u KiB\",\n+\t\t\t    x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);\n \t} else {\n-\t\tl -= snprintf(tp->display, l, \", %u bytes\", (int)total);\n+\t\tstrbuf_addf(buf, \"%u bytes\", (int)bytes);\n \t}\n+}\n \n-\tif (rate > 1 << 10) {\n-\t\tint x = rate + 5;  /* for rounding */\n-\t\tsnprintf(tp->display + sizeof(tp->display) - l, l,\n-\t\t\t \" | %u.%2.2u MiB/s\",\n-\t\t\t x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);\n-\t} else if (rate)\n-\t\tsnprintf(tp->display + sizeof(tp->display) - l, l,\n-\t\t\t \" | %u KiB/s\", rate);\n+static void throughput_string(struct strbuf *buf, off_t total,\n+\t\t\t      unsigned int rate)\n+{\n+\tstrbuf_addstr(buf, \", \");\n+\thumanize(buf, total);\n+\tstrbuf_addstr(buf, \" | \");\n+\thumanize(buf, rate * 1024);\n+\tstrbuf_addstr(buf, \"/s\");\n }\n \n void display_throughput(struct progress *progress, off_t total)\n@@ -183,6 +185,7 @@ void display_throughput(struct progress *progress, off_t total)\n \tmisecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;\n \n \tif (misecs > 512) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n \t\tunsigned int count, rate;\n \n \t\tcount = total - tp->prev_total;\n@@ -197,7 +200,9 @@ void display_throughput(struct progress *progress, off_t total)\n \t\ttp->last_misecs[tp->idx] = misecs;\n \t\ttp->idx = (tp->idx + 1) % TP_IDX_MAX;\n \n-\t\tthroughput_string(tp, total, rate);\n+\t\tthroughput_string(&buf, total, rate);\n+\t\tstrncpy(tp->display, buf.buf, sizeof(tp->display));\n+\t\tstrbuf_release(&buf);\n \t\tif (progress->last_value != -1 && progress_update)\n \t\t\tdisplay(progress, progress->last_value, NULL);\n \t}\n@@ -253,9 +258,12 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)\n \n \t\tbufp = (len < sizeof(buf)) ? buf : xmalloc(len + 1);\n \t\tif (tp) {\n+\t\t\tstruct strbuf strbuf = STRBUF_INIT;\n \t\t\tunsigned int rate = !tp->avg_misecs ? 0 :\n \t\t\t\t\ttp->avg_bytes / tp->avg_misecs;\n-\t\t\tthroughput_string(tp, tp->curr_total, rate);\n+\t\t\tthroughput_string(&strbuf, tp->curr_total, rate);\n+\t\t\tstrncpy(tp->display, strbuf.buf, sizeof(tp->display));\n+\t\t\tstrbuf_release(&strbuf);\n \t\t}\n \t\tprogress_update = 1;\n \t\tsprintf(bufp, \", %s.\\n\", msg);\ndiff --git a/progress.h b/progress.h\nindex 611e4c4..0e70f55 100644\n--- a/progress.h\n+++ b/progress.h\n@@ -2,7 +2,9 @@\n #define PROGRESS_H\n \n struct progress;\n+struct strbuf;\n \n+void humanize(struct strbuf *buf, off_t bytes);\n void display_throughput(struct progress *progress, off_t total);\n int display_progress(struct progress *progress, unsigned n);\n struct progress *start_progress(const char *title, unsigned total);\n-- \n1.7.9.5\n"},{"id":"213571","messageId":"1365445101-10425-2-git-send-email-apelisse@gmail.com","threadId":"33350","inReplyTo":"1365445101-10425-1-git-send-email-apelisse@gmail.com","subject":"[PATCH 2/2] count-objects: add -H option to humanize sizes","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-04-08T18:18:21Z","receivedAt":"2013-04-08T18:18:21Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Use the new humanize() function to print loose objects size, pack size,\nand garbage size in verbose mode, or loose objects size in regular mode.\nThis patch doesn't change the way anything is displayed when the option\nis not used.\n\nAlso update the documentation.\n\nSigned-off-by: Antoine Pelisse <apelisse@gmail.com>\n---\n Documentation/git-count-objects.txt |   14 ++++++++---\n builtin/count-objects.c             |   47 +++++++++++++++++++++++++++++------\n 2 files changed, 49 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt\nindex da6e72e..b300e84 100644\n--- a/Documentation/git-count-objects.txt\n+++ b/Documentation/git-count-objects.txt\n@@ -8,7 +8,7 @@ git-count-objects - Count unpacked number of objects and their disk consumption\n SYNOPSIS\n --------\n [verse]\n-'git count-objects' [-v]\n+'git count-objects' [-v] [-H | --human-readable]\n \n DESCRIPTION\n -----------\n@@ -24,11 +24,11 @@ OPTIONS\n +\n count: the number of loose objects\n +\n-size: disk space consumed by loose objects, in KiB\n+size: disk space consumed by loose objects, in KiB (unless -H is specified)\n +\n in-pack: the number of in-pack objects\n +\n-size-pack: disk space consumed by the packs, in KiB\n+size-pack: disk space consumed by the packs, in KiB (unless -H is specified)\n +\n prune-packable: the number of loose objects that are also present in\n the packs. These objects could be pruned using `git prune-packed`.\n@@ -36,7 +36,13 @@ the packs. These objects could be pruned using `git prune-packed`.\n garbage: the number of files in object database that are not valid\n loose objects nor valid packs\n +\n-size-garbage: disk space consumed by garbage files, in KiB\n+size-garbage: disk space consumed by garbage files, in KiB (unless -H is\n+specified)\n+\n+-H::\n+--human-readable::\n+\n+Print sizes in human readable format\n \n GIT\n ---\ndiff --git a/builtin/count-objects.c b/builtin/count-objects.c\nindex 0343e35..9836f6a 100644\n--- a/builtin/count-objects.c\n+++ b/builtin/count-objects.c\n@@ -8,6 +8,7 @@\n #include \"dir.h\"\n #include \"builtin.h\"\n #include \"parse-options.h\"\n+#include \"progress.h\"\n \n static unsigned long garbage;\n static off_t size_garbage;\n@@ -79,13 +80,13 @@ static void count_objects(DIR *d, char *path, int len, int verbose,\n }\n \n static char const * const count_objects_usage[] = {\n-\tN_(\"git count-objects [-v]\"),\n+\tN_(\"git count-objects [-v] [-H | --human-readable]\"),\n \tNULL\n };\n \n int cmd_count_objects(int argc, const char **argv, const char *prefix)\n {\n-\tint i, verbose = 0;\n+\tint i, verbose = 0, human_readable = 0;\n \tconst char *objdir = get_object_directory();\n \tint len = strlen(objdir);\n \tchar *path = xmalloc(len + 50);\n@@ -93,6 +94,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \toff_t loose_size = 0;\n \tstruct option opts[] = {\n \t\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n+\t\tOPT_BOOLEAN('H', \"human-readable\", &human_readable,\n+\t\t\t    N_(\"print sizes in human readable format\")),\n \t\tOPT_END(),\n \t};\n \n@@ -119,6 +122,9 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t\tstruct packed_git *p;\n \t\tunsigned long num_pack = 0;\n \t\toff_t size_pack = 0;\n+\t\tstruct strbuf loose_buf = STRBUF_INIT;\n+\t\tstruct strbuf pack_buf = STRBUF_INIT;\n+\t\tstruct strbuf garbage_buf = STRBUF_INIT;\n \t\tif (!packed_git)\n \t\t\tprepare_packed_git();\n \t\tfor (p = packed_git; p; p = p->next) {\n@@ -130,17 +136,42 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t\t\tsize_pack += p->pack_size + p->index_size;\n \t\t\tnum_pack++;\n \t\t}\n+\n+\t\tif (human_readable) {\n+\t\t\thumanize(&loose_buf, loose_size);\n+\t\t\thumanize(&pack_buf, size_pack);\n+\t\t\thumanize(&garbage_buf, size_garbage);\n+\t\t}\n+\t\telse {\n+\t\t\tstrbuf_addf(&loose_buf, \"%lu\",\n+\t\t\t\t    (unsigned long)(loose_size / 1024));\n+\t\t\tstrbuf_addf(&pack_buf, \"%lu\",\n+\t\t\t\t    (unsigned long)(size_pack / 1024));\n+\t\t\tstrbuf_addf(&garbage_buf, \"%lu\",\n+\t\t\t\t    (unsigned long)(size_garbage / 1024));\n+\t\t}\n+\n \t\tprintf(\"count: %lu\\n\", loose);\n-\t\tprintf(\"size: %lu\\n\", (unsigned long) (loose_size / 1024));\n+\t\tprintf(\"size: %s\\n\", loose_buf.buf);\n \t\tprintf(\"in-pack: %lu\\n\", packed);\n \t\tprintf(\"packs: %lu\\n\", num_pack);\n-\t\tprintf(\"size-pack: %lu\\n\", (unsigned long) (size_pack / 1024));\n+\t\tprintf(\"size-pack: %s\\n\", pack_buf.buf);\n \t\tprintf(\"prune-packable: %lu\\n\", packed_loose);\n \t\tprintf(\"garbage: %lu\\n\", garbage);\n-\t\tprintf(\"size-garbage: %lu\\n\", (unsigned long) (size_garbage / 1024));\n+\t\tprintf(\"size-garbage: %s\\n\", garbage_buf.buf);\n+\t\tstrbuf_release(&loose_buf);\n+\t\tstrbuf_release(&pack_buf);\n+\t\tstrbuf_release(&garbage_buf);\n+\t}\n+\telse {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tif (human_readable)\n+\t\t\thumanize(&buf, loose_size);\n+\t\telse\n+\t\t\tstrbuf_addf(&buf, \"%lu KiB\",\n+\t\t\t\t    (unsigned long)(loose_size / 1024));\n+\t\tprintf(\"%lu objects, %s\\n\", loose, buf.buf);\n+\t\tstrbuf_release(&buf);\n \t}\n-\telse\n-\t\tprintf(\"%lu objects, %lu KiB\\n\",\n-\t\t       loose, (unsigned long) (loose_size / 1024));\n \treturn 0;\n }\n-- \n1.7.9.5\n"},{"id":"213615","messageId":"7vli8svgyo.fsf@alter.siamese.dyndns.org","threadId":"33350","inReplyTo":"1365445101-10425-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH 1/2] progress: create public humanize() to show sizes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-08T21:40:47Z","receivedAt":"2013-04-08T21:40:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse <apelisse@gmail.com> writes:\n\n> Currently, humanization of downloaded size is done in the same function\n> as text formatting. This is an issue if anyone else wants to use this.\n>\n> Separate text formatting from size simplification and make the function\n> public so that it can easily be used by other clients.\n>\n> We now can use humanize() for both downloaded size and download speed\n> calculation. One of the drawbacks is that speed will no look like this\n> when download is stalled: \"0 bytes/s\" instead of \"0 KiB/s\".\n>\n> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>\n> ---\n\nSounds good, but I think this helper function should live in\nstrbuf.[ch], where many other text/string helpers that take a strbuf\nas their first parameter live.\n\n>  progress.c |   60 ++++++++++++++++++++++++++++++++++--------------------------\n>  progress.h |    2 ++\n>  2 files changed, 36 insertions(+), 26 deletions(-)\n>\n> diff --git a/progress.c b/progress.c\n> index 3971f49..76c1e42 100644\n> --- a/progress.c\n> +++ b/progress.c\n> @@ -8,8 +8,11 @@\n>   * published by the Free Software Foundation.\n>   */\n>  \n> +#include <string.h>\n> +\n\nPlease do not do this.\n\nIf you somehow need to have <string.h>, a suitable place should be\nfound in git-compat-util.h; various platforms seem to have quirks\nwith the ordering of system header files, and git-compat-util.h is\nmeant to encapsulate them.\n\nIn fact, git-compat-util.h should already include it.\n\n>  #include \"git-compat-util.h\"\n>  #include \"progress.h\"\n> +#include \"strbuf.h\"\n>  \n>  #define TP_IDX_MAX      8\n>  \n> @@ -112,34 +115,33 @@ static int display(struct progress *progress, unsigned n, const char *done)\n>  \treturn 0;\n>  }\n>  \n> -static void throughput_string(struct throughput *tp, off_t total,\n> -\t\t\t      unsigned int rate)\n> +void humanize(struct strbuf *buf, off_t bytes)\n>  {\n> -\tint l = sizeof(tp->display);\n> -\tif (total > 1 << 30) {\n> -\t\tl -= snprintf(tp->display, l, \", %u.%2.2u GiB\",\n> -\t\t\t      (int)(total >> 30),\n> -\t\t\t      (int)(total & ((1 << 30) - 1)) / 10737419);\n> -\t} else if (total > 1 << 20) {\n> -\t\tint x = total + 5243;  /* for rounding */\n> -\t\tl -= snprintf(tp->display, l, \", %u.%2.2u MiB\",\n> -\t\t\t      x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);\n> -\t} else if (total > 1 << 10) {\n> -\t\tint x = total + 5;  /* for rounding */\n> -\t\tl -= snprintf(tp->display, l, \", %u.%2.2u KiB\",\n> -\t\t\t      x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);\n> +\tif (bytes > 1 << 30) {\n> +\t\tstrbuf_addf(buf, \"%u.%2.2u GiB\",\n> +\t\t\t    (int)(bytes >> 30),\n> +\t\t\t    (int)(bytes & ((1 << 30) - 1)) / 10737419);\n> +\t} else if (bytes > 1 << 20) {\n> +\t\tint x = bytes + 5243;  /* for rounding */\n> +\t\tstrbuf_addf(buf, \"%u.%2.2u MiB\",\n> +\t\t\t    x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);\n> +\t} else if (bytes > 1 << 10) {\n> +\t\tint x = bytes + 5;  /* for rounding */\n> +\t\tstrbuf_addf(buf, \"%u.%2.2u KiB\",\n> +\t\t\t    x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);\n>  \t} else {\n> -\t\tl -= snprintf(tp->display, l, \", %u bytes\", (int)total);\n> +\t\tstrbuf_addf(buf, \"%u bytes\", (int)bytes);\n>  \t}\n> +}\n>  \n> -\tif (rate > 1 << 10) {\n> -\t\tint x = rate + 5;  /* for rounding */\n> -\t\tsnprintf(tp->display + sizeof(tp->display) - l, l,\n> -\t\t\t \" | %u.%2.2u MiB/s\",\n> -\t\t\t x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);\n> -\t} else if (rate)\n> -\t\tsnprintf(tp->display + sizeof(tp->display) - l, l,\n> -\t\t\t \" | %u KiB/s\", rate);\n> +static void throughput_string(struct strbuf *buf, off_t total,\n> +\t\t\t      unsigned int rate)\n> +{\n> +\tstrbuf_addstr(buf, \", \");\n> +\thumanize(buf, total);\n> +\tstrbuf_addstr(buf, \" | \");\n> +\thumanize(buf, rate * 1024);\n> +\tstrbuf_addstr(buf, \"/s\");\n>  }\n>  \n>  void display_throughput(struct progress *progress, off_t total)\n> @@ -183,6 +185,7 @@ void display_throughput(struct progress *progress, off_t total)\n>  \tmisecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;\n>  \n>  \tif (misecs > 512) {\n> +\t\tstruct strbuf buf = STRBUF_INIT;\n>  \t\tunsigned int count, rate;\n>  \n>  \t\tcount = total - tp->prev_total;\n> @@ -197,7 +200,9 @@ void display_throughput(struct progress *progress, off_t total)\n>  \t\ttp->last_misecs[tp->idx] = misecs;\n>  \t\ttp->idx = (tp->idx + 1) % TP_IDX_MAX;\n>  \n> -\t\tthroughput_string(tp, total, rate);\n> +\t\tthroughput_string(&buf, total, rate);\n> +\t\tstrncpy(tp->display, buf.buf, sizeof(tp->display));\n> +\t\tstrbuf_release(&buf);\n>  \t\tif (progress->last_value != -1 && progress_update)\n>  \t\t\tdisplay(progress, progress->last_value, NULL);\n>  \t}\n> @@ -253,9 +258,12 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)\n>  \n>  \t\tbufp = (len < sizeof(buf)) ? buf : xmalloc(len + 1);\n>  \t\tif (tp) {\n> +\t\t\tstruct strbuf strbuf = STRBUF_INIT;\n>  \t\t\tunsigned int rate = !tp->avg_misecs ? 0 :\n>  \t\t\t\t\ttp->avg_bytes / tp->avg_misecs;\n> -\t\t\tthroughput_string(tp, tp->curr_total, rate);\n> +\t\t\tthroughput_string(&strbuf, tp->curr_total, rate);\n> +\t\t\tstrncpy(tp->display, strbuf.buf, sizeof(tp->display));\n> +\t\t\tstrbuf_release(&strbuf);\n>  \t\t}\n>  \t\tprogress_update = 1;\n>  \t\tsprintf(bufp, \", %s.\\n\", msg);\n> diff --git a/progress.h b/progress.h\n> index 611e4c4..0e70f55 100644\n> --- a/progress.h\n> +++ b/progress.h\n> @@ -2,7 +2,9 @@\n>  #define PROGRESS_H\n>  \n>  struct progress;\n> +struct strbuf;\n>  \n> +void humanize(struct strbuf *buf, off_t bytes);\n>  void display_throughput(struct progress *progress, off_t total);\n>  int display_progress(struct progress *progress, unsigned n);\n>  struct progress *start_progress(const char *title, unsigned total);\n"},{"id":"213618","messageId":"CAPig+cSBf1g=cRxBNFA=k_bA1JxUfom5C2gNSjvK=+4BK7D7iw@mail.gmail.com","threadId":"33350","inReplyTo":"1365445101-10425-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH 1/2] progress: create public humanize() to show sizes","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2013-04-08T21:55:58Z","receivedAt":"2013-04-08T21:55:58Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Apr 8, 2013 at 2:18 PM, Antoine Pelisse <apelisse@gmail.com> wrote:\n> Currently, humanization of downloaded size is done in the same function\n> as text formatting. This is an issue if anyone else wants to use this.\n>\n> Separate text formatting from size simplification and make the function\n> public so that it can easily be used by other clients.\n>\n> We now can use humanize() for both downloaded size and download speed\n> calculation. One of the drawbacks is that speed will no look like this\n\ns/no/now/\n\n> when download is stalled: \"0 bytes/s\" instead of \"0 KiB/s\".\n>\n> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>\n"},{"id":"213807","messageId":"1365620604-17851-1-git-send-email-apelisse@gmail.com","threadId":"33350","inReplyTo":"7vli8svgyo.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/2] strbuf: create strbuf_humanize() to show byte sizes","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-04-10T19:03:23Z","receivedAt":"2013-04-10T19:03:23Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Currently, humanization of downloaded size is done in the same\nfunction as text formatting in 'process.c'. This is an issue if anyone\nelse wants to use this.\n\nSeparate text formatting from size simplification and make the function\npublic in strbuf so that it can easily be used by other clients.\n\nWe now can use strbuf_humanize() for both downloaded size and download\nspeed calculation. One of the drawbacks is that speed will now look like\nthis when download is stalled: \"0 bytes/s\" instead of \"0 KiB/s\".\n\nSigned-off-by: Antoine Pelisse <apelisse@gmail.com>\n---\n Documentation/technical/api-strbuf.txt |    5 ++++\n progress.c                             |   43 +++++++++++---------------------\n strbuf.c                               |   19 ++++++++++++++\n strbuf.h                               |    1 +\n 4 files changed, 40 insertions(+), 28 deletions(-)\n\ndiff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt\nindex 2c59cb2..7b6ecda 100644\n--- a/Documentation/technical/api-strbuf.txt\n+++ b/Documentation/technical/api-strbuf.txt\n@@ -230,6 +230,11 @@ which can be used by the programmer of the callback as she sees fit.\n \tdestination. This is useful for literal data to be fed to either\n \tstrbuf_expand or to the *printf family of functions.\n \n+`strbuf_humanize`::\n+\n+\tAppend the given byte size as a human-readable string (i.e. 12.23 KiB,\n+\t3.50 MiB).\n+\n `strbuf_addf`::\n \n \tAdd a formatted string to the buffer.\ndiff --git a/progress.c b/progress.c\nindex 3971f49..8e09058 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -10,6 +10,7 @@\n \n #include \"git-compat-util.h\"\n #include \"progress.h\"\n+#include \"strbuf.h\"\n \n #define TP_IDX_MAX      8\n \n@@ -112,34 +113,14 @@ static int display(struct progress *progress, unsigned n, const char *done)\n \treturn 0;\n }\n \n-static void throughput_string(struct throughput *tp, off_t total,\n+static void throughput_string(struct strbuf *buf, off_t total,\n \t\t\t      unsigned int rate)\n {\n-\tint l = sizeof(tp->display);\n-\tif (total > 1 << 30) {\n-\t\tl -= snprintf(tp->display, l, \", %u.%2.2u GiB\",\n-\t\t\t      (int)(total >> 30),\n-\t\t\t      (int)(total & ((1 << 30) - 1)) / 10737419);\n-\t} else if (total > 1 << 20) {\n-\t\tint x = total + 5243;  /* for rounding */\n-\t\tl -= snprintf(tp->display, l, \", %u.%2.2u MiB\",\n-\t\t\t      x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);\n-\t} else if (total > 1 << 10) {\n-\t\tint x = total + 5;  /* for rounding */\n-\t\tl -= snprintf(tp->display, l, \", %u.%2.2u KiB\",\n-\t\t\t      x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);\n-\t} else {\n-\t\tl -= snprintf(tp->display, l, \", %u bytes\", (int)total);\n-\t}\n-\n-\tif (rate > 1 << 10) {\n-\t\tint x = rate + 5;  /* for rounding */\n-\t\tsnprintf(tp->display + sizeof(tp->display) - l, l,\n-\t\t\t \" | %u.%2.2u MiB/s\",\n-\t\t\t x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);\n-\t} else if (rate)\n-\t\tsnprintf(tp->display + sizeof(tp->display) - l, l,\n-\t\t\t \" | %u KiB/s\", rate);\n+\tstrbuf_addstr(buf, \", \");\n+\tstrbuf_humanize(buf, total);\n+\tstrbuf_addstr(buf, \" | \");\n+\tstrbuf_humanize(buf, rate * 1024);\n+\tstrbuf_addstr(buf, \"/s\");\n }\n \n void display_throughput(struct progress *progress, off_t total)\n@@ -183,6 +164,7 @@ void display_throughput(struct progress *progress, off_t total)\n \tmisecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;\n \n \tif (misecs > 512) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n \t\tunsigned int count, rate;\n \n \t\tcount = total - tp->prev_total;\n@@ -197,7 +179,9 @@ void display_throughput(struct progress *progress, off_t total)\n \t\ttp->last_misecs[tp->idx] = misecs;\n \t\ttp->idx = (tp->idx + 1) % TP_IDX_MAX;\n \n-\t\tthroughput_string(tp, total, rate);\n+\t\tthroughput_string(&buf, total, rate);\n+\t\tstrncpy(tp->display, buf.buf, sizeof(tp->display));\n+\t\tstrbuf_release(&buf);\n \t\tif (progress->last_value != -1 && progress_update)\n \t\t\tdisplay(progress, progress->last_value, NULL);\n \t}\n@@ -253,9 +237,12 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)\n \n \t\tbufp = (len < sizeof(buf)) ? buf : xmalloc(len + 1);\n \t\tif (tp) {\n+\t\t\tstruct strbuf strbuf = STRBUF_INIT;\n \t\t\tunsigned int rate = !tp->avg_misecs ? 0 :\n \t\t\t\t\ttp->avg_bytes / tp->avg_misecs;\n-\t\t\tthroughput_string(tp, tp->curr_total, rate);\n+\t\t\tthroughput_string(&strbuf, tp->curr_total, rate);\n+\t\t\tstrncpy(tp->display, strbuf.buf, sizeof(tp->display));\n+\t\t\tstrbuf_release(&strbuf);\n \t\t}\n \t\tprogress_update = 1;\n \t\tsprintf(bufp, \", %s.\\n\", msg);\ndiff --git a/strbuf.c b/strbuf.c\nindex 48e9abb..8a50e66 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -528,6 +528,25 @@ void strbuf_addstr_urlencode(struct strbuf *sb, const char *s,\n \tstrbuf_add_urlencode(sb, s, strlen(s), reserved);\n }\n \n+void strbuf_humanize(struct strbuf *buf, off_t bytes)\n+{\n+\tif (bytes > 1 << 30) {\n+\t\tstrbuf_addf(buf, \"%u.%2.2u GiB\",\n+\t\t\t    (int)(bytes >> 30),\n+\t\t\t    (int)(bytes & ((1 << 30) - 1)) / 10737419);\n+\t} else if (bytes > 1 << 20) {\n+\t\tint x = bytes + 5243;  /* for rounding */\n+\t\tstrbuf_addf(buf, \"%u.%2.2u MiB\",\n+\t\t\t    x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);\n+\t} else if (bytes > 1 << 10) {\n+\t\tint x = bytes + 5;  /* for rounding */\n+\t\tstrbuf_addf(buf, \"%u.%2.2u KiB\",\n+\t\t\t    x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);\n+\t} else {\n+\t\tstrbuf_addf(buf, \"%u bytes\", (int)bytes);\n+\t}\n+}\n+\n int printf_ln(const char *fmt, ...)\n {\n \tint ret;\ndiff --git a/strbuf.h b/strbuf.h\nindex 958822c..317c5a8 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -170,6 +170,7 @@ extern int strbuf_check_branch_ref(struct strbuf *sb, const char *name);\n \n extern void strbuf_addstr_urlencode(struct strbuf *, const char *,\n \t\t\t\t    int reserved);\n+extern void strbuf_humanize(struct strbuf *buf, off_t bytes);\n \n __attribute__((format (printf,1,2)))\n extern int printf_ln(const char *fmt, ...);\n-- \n1.7.9.5\n"},{"id":"213808","messageId":"1365620604-17851-2-git-send-email-apelisse@gmail.com","threadId":"33350","inReplyTo":"1365620604-17851-1-git-send-email-apelisse@gmail.com","subject":"[PATCH 2/2] count-objects: add -H option to humanize sizes","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-04-10T19:03:24Z","receivedAt":"2013-04-10T19:03:24Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"Use the new humanize() function to print loose objects size, pack size,\nand garbage size in verbose mode, or loose objects size in regular mode.\nThis patch doesn't change the way anything is displayed when the option\nis not used.\n\nAlso update the documentation.\n\nSigned-off-by: Antoine Pelisse <apelisse@gmail.com>\n---\n Documentation/git-count-objects.txt |   14 ++++++++---\n builtin/count-objects.c             |   46 +++++++++++++++++++++++++++++------\n 2 files changed, 48 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt\nindex da6e72e..b300e84 100644\n--- a/Documentation/git-count-objects.txt\n+++ b/Documentation/git-count-objects.txt\n@@ -8,7 +8,7 @@ git-count-objects - Count unpacked number of objects and their disk consumption\n SYNOPSIS\n --------\n [verse]\n-'git count-objects' [-v]\n+'git count-objects' [-v] [-H | --human-readable]\n \n DESCRIPTION\n -----------\n@@ -24,11 +24,11 @@ OPTIONS\n +\n count: the number of loose objects\n +\n-size: disk space consumed by loose objects, in KiB\n+size: disk space consumed by loose objects, in KiB (unless -H is specified)\n +\n in-pack: the number of in-pack objects\n +\n-size-pack: disk space consumed by the packs, in KiB\n+size-pack: disk space consumed by the packs, in KiB (unless -H is specified)\n +\n prune-packable: the number of loose objects that are also present in\n the packs. These objects could be pruned using `git prune-packed`.\n@@ -36,7 +36,13 @@ the packs. These objects could be pruned using `git prune-packed`.\n garbage: the number of files in object database that are not valid\n loose objects nor valid packs\n +\n-size-garbage: disk space consumed by garbage files, in KiB\n+size-garbage: disk space consumed by garbage files, in KiB (unless -H is\n+specified)\n+\n+-H::\n+--human-readable::\n+\n+Print sizes in human readable format\n \n GIT\n ---\ndiff --git a/builtin/count-objects.c b/builtin/count-objects.c\nindex 0343e35..935ad9e 100644\n--- a/builtin/count-objects.c\n+++ b/builtin/count-objects.c\n@@ -79,13 +79,13 @@ static void count_objects(DIR *d, char *path, int len, int verbose,\n }\n \n static char const * const count_objects_usage[] = {\n-\tN_(\"git count-objects [-v]\"),\n+\tN_(\"git count-objects [-v] [-H | --human-readable]\"),\n \tNULL\n };\n \n int cmd_count_objects(int argc, const char **argv, const char *prefix)\n {\n-\tint i, verbose = 0;\n+\tint i, verbose = 0, human_readable = 0;\n \tconst char *objdir = get_object_directory();\n \tint len = strlen(objdir);\n \tchar *path = xmalloc(len + 50);\n@@ -93,6 +93,8 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \toff_t loose_size = 0;\n \tstruct option opts[] = {\n \t\tOPT__VERBOSE(&verbose, N_(\"be verbose\")),\n+\t\tOPT_BOOLEAN('H', \"human-readable\", &human_readable,\n+\t\t\t    N_(\"print sizes in human readable format\")),\n \t\tOPT_END(),\n \t};\n \n@@ -119,6 +121,9 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t\tstruct packed_git *p;\n \t\tunsigned long num_pack = 0;\n \t\toff_t size_pack = 0;\n+\t\tstruct strbuf loose_buf = STRBUF_INIT;\n+\t\tstruct strbuf pack_buf = STRBUF_INIT;\n+\t\tstruct strbuf garbage_buf = STRBUF_INIT;\n \t\tif (!packed_git)\n \t\t\tprepare_packed_git();\n \t\tfor (p = packed_git; p; p = p->next) {\n@@ -130,17 +135,42 @@ int cmd_count_objects(int argc, const char **argv, const char *prefix)\n \t\t\tsize_pack += p->pack_size + p->index_size;\n \t\t\tnum_pack++;\n \t\t}\n+\n+\t\tif (human_readable) {\n+\t\t\tstrbuf_humanize(&loose_buf, loose_size);\n+\t\t\tstrbuf_humanize(&pack_buf, size_pack);\n+\t\t\tstrbuf_humanize(&garbage_buf, size_garbage);\n+\t\t}\n+\t\telse {\n+\t\t\tstrbuf_addf(&loose_buf, \"%lu\",\n+\t\t\t\t    (unsigned long)(loose_size / 1024));\n+\t\t\tstrbuf_addf(&pack_buf, \"%lu\",\n+\t\t\t\t    (unsigned long)(size_pack / 1024));\n+\t\t\tstrbuf_addf(&garbage_buf, \"%lu\",\n+\t\t\t\t    (unsigned long)(size_garbage / 1024));\n+\t\t}\n+\n \t\tprintf(\"count: %lu\\n\", loose);\n-\t\tprintf(\"size: %lu\\n\", (unsigned long) (loose_size / 1024));\n+\t\tprintf(\"size: %s\\n\", loose_buf.buf);\n \t\tprintf(\"in-pack: %lu\\n\", packed);\n \t\tprintf(\"packs: %lu\\n\", num_pack);\n-\t\tprintf(\"size-pack: %lu\\n\", (unsigned long) (size_pack / 1024));\n+\t\tprintf(\"size-pack: %s\\n\", pack_buf.buf);\n \t\tprintf(\"prune-packable: %lu\\n\", packed_loose);\n \t\tprintf(\"garbage: %lu\\n\", garbage);\n-\t\tprintf(\"size-garbage: %lu\\n\", (unsigned long) (size_garbage / 1024));\n+\t\tprintf(\"size-garbage: %s\\n\", garbage_buf.buf);\n+\t\tstrbuf_release(&loose_buf);\n+\t\tstrbuf_release(&pack_buf);\n+\t\tstrbuf_release(&garbage_buf);\n+\t}\n+\telse {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tif (human_readable)\n+\t\t\tstrbuf_humanize(&buf, loose_size);\n+\t\telse\n+\t\t\tstrbuf_addf(&buf, \"%lu KiB\",\n+\t\t\t\t    (unsigned long)(loose_size / 1024));\n+\t\tprintf(\"%lu objects, %s\\n\", loose, buf.buf);\n+\t\tstrbuf_release(&buf);\n \t}\n-\telse\n-\t\tprintf(\"%lu objects, %lu KiB\\n\",\n-\t\t       loose, (unsigned long) (loose_size / 1024));\n \treturn 0;\n }\n-- \n1.7.9.5\n"},{"id":"213818","messageId":"20130410194307.GA27070@google.com","threadId":"33350","inReplyTo":"1365620604-17851-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH 1/2] strbuf: create strbuf_humanize() to show byte sizes","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-04-10T19:43:07Z","receivedAt":"2013-04-10T19:43:07Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Antoine Pelisse wrote:\n\n> Separate text formatting from size simplification and make the function\n> public in strbuf so that it can easily be used by other clients.\n>\n> We now can use strbuf_humanize() for both downloaded size and download\n> speed calculation.\n\nSounds like a good thing to do.\n\n>                    One of the drawbacks is that speed will now look like\n> this when download is stalled: \"0 bytes/s\" instead of \"0 KiB/s\".\n\nAt first glance that is neither obviously a benefit nor obviously a\ndrawback.  Can you spell this out more?\n\n> --- a/Documentation/technical/api-strbuf.txt\n> +++ b/Documentation/technical/api-strbuf.txt\n> @@ -230,6 +230,11 @@ which can be used by the programmer of the callback as she sees fit.\n>  \tdestination. This is useful for literal data to be fed to either\n>  \tstrbuf_expand or to the *printf family of functions.\n>  \n> +`strbuf_humanize`::\n> +\n> +\tAppend the given byte size as a human-readable string (i.e. 12.23 KiB,\n> +\t3.50 MiB).\n\nBased on the function name alone, it is not easy to guess what it will\ndo (e.g., maybe it will paraphrase 3 to \"three\" and 10000000 to\n\"enormous\").  How about something like strbuf_filesize?\n\nIf I understand the code correctly, this jumps units each time it\nexceeds 1.0 of the next unit (bytes, KiB, MiB, GiB), which sounds like\na fine behavior.\n\nHope that helps,\nJonathan\n"},{"id":"213822","messageId":"7vr4iikvkd.fsf@alter.siamese.dyndns.org","threadId":"33350","inReplyTo":"1365620604-17851-1-git-send-email-apelisse@gmail.com","subject":"Re: [PATCH 1/2] strbuf: create strbuf_humanize() to show byte sizes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-04-10T19:57:38Z","receivedAt":"2013-04-10T19:57:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Antoine Pelisse <apelisse@gmail.com> writes:\n\n> Currently, humanization of downloaded size is done in the same\n> function as text formatting in 'process.c'. This is an issue if anyone\n> else wants to use this.\n>\n> Separate text formatting from size simplification and make the function\n> public in strbuf so that it can easily be used by other clients.\n>\n> We now can use strbuf_humanize() for both downloaded size and download\n> speed calculation. One of the drawbacks is that speed will now look like\n> this when download is stalled: \"0 bytes/s\" instead of \"0 KiB/s\".\n\nPersonally, I do not think the \"drawback\" is so big an issue.  If\nthe caller really cares, we could always add another parameter to\nthis formatter to tell it the minimum unit we care about (e.g. pass\n1024 to say \"Don't bother showing scale lower than kibi\").\n\nThis is a bit late response, but if we ever want to count something\nin a dimention other than \"bytes\", like time (e.g. \"kiloseconds\") or\nnumber of commits (e.g. \"centicommits\"), etc., we cannot reuse this\nformatter very easily.  We may want to have \"byte\" somewhere in its\nname for now to make sure the callers understand its limitation.\n\nI'll tentatively rename it to \"strbuf_humanize_bytes()\" while queuing.\n\nThanks.\n\n> Signed-off-by: Antoine Pelisse <apelisse@gmail.com>\n> ---\n>  Documentation/technical/api-strbuf.txt |    5 ++++\n>  progress.c                             |   43 +++++++++++---------------------\n>  strbuf.c                               |   19 ++++++++++++++\n>  strbuf.h                               |    1 +\n>  4 files changed, 40 insertions(+), 28 deletions(-)\n>\n> diff --git a/Documentation/technical/api-strbuf.txt b/Documentation/technical/api-strbuf.txt\n> index 2c59cb2..7b6ecda 100644\n> --- a/Documentation/technical/api-strbuf.txt\n> +++ b/Documentation/technical/api-strbuf.txt\n> @@ -230,6 +230,11 @@ which can be used by the programmer of the callback as she sees fit.\n>  \tdestination. This is useful for literal data to be fed to either\n>  \tstrbuf_expand or to the *printf family of functions.\n>  \n> +`strbuf_humanize`::\n> +\n> +\tAppend the given byte size as a human-readable string (i.e. 12.23 KiB,\n> +\t3.50 MiB).\n> +\n>  `strbuf_addf`::\n>  \n>  \tAdd a formatted string to the buffer.\n> diff --git a/progress.c b/progress.c\n> index 3971f49..8e09058 100644\n> --- a/progress.c\n> +++ b/progress.c\n> @@ -10,6 +10,7 @@\n>  \n>  #include \"git-compat-util.h\"\n>  #include \"progress.h\"\n> +#include \"strbuf.h\"\n>  \n>  #define TP_IDX_MAX      8\n>  \n> @@ -112,34 +113,14 @@ static int display(struct progress *progress, unsigned n, const char *done)\n>  \treturn 0;\n>  }\n>  \n> -static void throughput_string(struct throughput *tp, off_t total,\n> +static void throughput_string(struct strbuf *buf, off_t total,\n>  \t\t\t      unsigned int rate)\n>  {\n> -\tint l = sizeof(tp->display);\n> -\tif (total > 1 << 30) {\n> -\t\tl -= snprintf(tp->display, l, \", %u.%2.2u GiB\",\n> -\t\t\t      (int)(total >> 30),\n> -\t\t\t      (int)(total & ((1 << 30) - 1)) / 10737419);\n> -\t} else if (total > 1 << 20) {\n> -\t\tint x = total + 5243;  /* for rounding */\n> -\t\tl -= snprintf(tp->display, l, \", %u.%2.2u MiB\",\n> -\t\t\t      x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);\n> -\t} else if (total > 1 << 10) {\n> -\t\tint x = total + 5;  /* for rounding */\n> -\t\tl -= snprintf(tp->display, l, \", %u.%2.2u KiB\",\n> -\t\t\t      x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);\n> -\t} else {\n> -\t\tl -= snprintf(tp->display, l, \", %u bytes\", (int)total);\n> -\t}\n> -\n> -\tif (rate > 1 << 10) {\n> -\t\tint x = rate + 5;  /* for rounding */\n> -\t\tsnprintf(tp->display + sizeof(tp->display) - l, l,\n> -\t\t\t \" | %u.%2.2u MiB/s\",\n> -\t\t\t x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);\n> -\t} else if (rate)\n> -\t\tsnprintf(tp->display + sizeof(tp->display) - l, l,\n> -\t\t\t \" | %u KiB/s\", rate);\n> +\tstrbuf_addstr(buf, \", \");\n> +\tstrbuf_humanize(buf, total);\n> +\tstrbuf_addstr(buf, \" | \");\n> +\tstrbuf_humanize(buf, rate * 1024);\n> +\tstrbuf_addstr(buf, \"/s\");\n>  }\n>  \n>  void display_throughput(struct progress *progress, off_t total)\n> @@ -183,6 +164,7 @@ void display_throughput(struct progress *progress, off_t total)\n>  \tmisecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;\n>  \n>  \tif (misecs > 512) {\n> +\t\tstruct strbuf buf = STRBUF_INIT;\n>  \t\tunsigned int count, rate;\n>  \n>  \t\tcount = total - tp->prev_total;\n> @@ -197,7 +179,9 @@ void display_throughput(struct progress *progress, off_t total)\n>  \t\ttp->last_misecs[tp->idx] = misecs;\n>  \t\ttp->idx = (tp->idx + 1) % TP_IDX_MAX;\n>  \n> -\t\tthroughput_string(tp, total, rate);\n> +\t\tthroughput_string(&buf, total, rate);\n> +\t\tstrncpy(tp->display, buf.buf, sizeof(tp->display));\n> +\t\tstrbuf_release(&buf);\n>  \t\tif (progress->last_value != -1 && progress_update)\n>  \t\t\tdisplay(progress, progress->last_value, NULL);\n>  \t}\n> @@ -253,9 +237,12 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)\n>  \n>  \t\tbufp = (len < sizeof(buf)) ? buf : xmalloc(len + 1);\n>  \t\tif (tp) {\n> +\t\t\tstruct strbuf strbuf = STRBUF_INIT;\n>  \t\t\tunsigned int rate = !tp->avg_misecs ? 0 :\n>  \t\t\t\t\ttp->avg_bytes / tp->avg_misecs;\n> -\t\t\tthroughput_string(tp, tp->curr_total, rate);\n> +\t\t\tthroughput_string(&strbuf, tp->curr_total, rate);\n> +\t\t\tstrncpy(tp->display, strbuf.buf, sizeof(tp->display));\n> +\t\t\tstrbuf_release(&strbuf);\n>  \t\t}\n>  \t\tprogress_update = 1;\n>  \t\tsprintf(bufp, \", %s.\\n\", msg);\n> diff --git a/strbuf.c b/strbuf.c\n> index 48e9abb..8a50e66 100644\n> --- a/strbuf.c\n> +++ b/strbuf.c\n> @@ -528,6 +528,25 @@ void strbuf_addstr_urlencode(struct strbuf *sb, const char *s,\n>  \tstrbuf_add_urlencode(sb, s, strlen(s), reserved);\n>  }\n>  \n> +void strbuf_humanize(struct strbuf *buf, off_t bytes)\n> +{\n> +\tif (bytes > 1 << 30) {\n> +\t\tstrbuf_addf(buf, \"%u.%2.2u GiB\",\n> +\t\t\t    (int)(bytes >> 30),\n> +\t\t\t    (int)(bytes & ((1 << 30) - 1)) / 10737419);\n> +\t} else if (bytes > 1 << 20) {\n> +\t\tint x = bytes + 5243;  /* for rounding */\n> +\t\tstrbuf_addf(buf, \"%u.%2.2u MiB\",\n> +\t\t\t    x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);\n> +\t} else if (bytes > 1 << 10) {\n> +\t\tint x = bytes + 5;  /* for rounding */\n> +\t\tstrbuf_addf(buf, \"%u.%2.2u KiB\",\n> +\t\t\t    x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);\n> +\t} else {\n> +\t\tstrbuf_addf(buf, \"%u bytes\", (int)bytes);\n> +\t}\n> +}\n> +\n>  int printf_ln(const char *fmt, ...)\n>  {\n>  \tint ret;\n> diff --git a/strbuf.h b/strbuf.h\n> index 958822c..317c5a8 100644\n> --- a/strbuf.h\n> +++ b/strbuf.h\n> @@ -170,6 +170,7 @@ extern int strbuf_check_branch_ref(struct strbuf *sb, const char *name);\n>  \n>  extern void strbuf_addstr_urlencode(struct strbuf *, const char *,\n>  \t\t\t\t    int reserved);\n> +extern void strbuf_humanize(struct strbuf *buf, off_t bytes);\n>  \n>  __attribute__((format (printf,1,2)))\n>  extern int printf_ln(const char *fmt, ...);\n"},{"id":"213823","messageId":"CALWbr2zciCO2Gzr_Hkg3oftYLtDkrPFrazP4HgRyPv=vYH5sXg@mail.gmail.com","threadId":"33350","inReplyTo":"20130410194307.GA27070@google.com","subject":"Re: [PATCH 1/2] strbuf: create strbuf_humanize() to show byte sizes","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-04-10T20:00:25Z","receivedAt":"2013-04-10T20:00:25Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Wed, Apr 10, 2013 at 9:43 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Antoine Pelisse wrote:\n>>                    One of the drawbacks is that speed will now look like\n>> this when download is stalled: \"0 bytes/s\" instead of \"0 KiB/s\".\n>\n> At first glance that is neither obviously a benefit nor obviously a\n> drawback.  Can you spell this out more?\n\nThe drawback to me is that it changes the user experience with no reason.\nBut that's really a minor change, I agree. (maybe I should have put it\nas a comment/question after ---)\n\n>> --- a/Documentation/technical/api-strbuf.txt\n>> +++ b/Documentation/technical/api-strbuf.txt\n>> @@ -230,6 +230,11 @@ which can be used by the programmer of the callback as she sees fit.\n>>       destination. This is useful for literal data to be fed to either\n>>       strbuf_expand or to the *printf family of functions.\n>>\n>> +`strbuf_humanize`::\n>> +\n>> +     Append the given byte size as a human-readable string (i.e. 12.23 KiB,\n>> +     3.50 MiB).\n>\n> Based on the function name alone, it is not easy to guess what it will\n> do (e.g., maybe it will paraphrase 3 to \"three\" and 10000000 to\n> \"enormous\").  How about something like strbuf_filesize?\n\nI think the suggestion from Junio makes more sense, as it can be used\nfor download speed.\n\n> If I understand the code correctly, this jumps units each time it\n> exceeds 1.0 of the next unit (bytes, KiB, MiB, GiB), which sounds like\n> a fine behavior.\n\nThe code has simply been extracted from the former function and kept unmodified.\n\nThanks for the help !\nAntoine,\n"},{"id":"213830","messageId":"CALWbr2w=q=BkMOeqmSAbi50vNbup+e9GF0gdxNH9-vpyyND5Vw@mail.gmail.com","threadId":"33350","inReplyTo":"7vr4iikvkd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/2] strbuf: create strbuf_humanize() to show byte sizes","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-04-10T20:12:00Z","receivedAt":"2013-04-10T20:12:00Z","isPatch":true,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Wed, Apr 10, 2013 at 9:57 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Antoine Pelisse <apelisse@gmail.com> writes:\n>\n>> Currently, humanization of downloaded size is done in the same\n>> function as text formatting in 'process.c'. This is an issue if anyone\n>> else wants to use this.\n>>\n>> Separate text formatting from size simplification and make the function\n>> public in strbuf so that it can easily be used by other clients.\n>>\n>> We now can use strbuf_humanize() for both downloaded size and download\n>> speed calculation. One of the drawbacks is that speed will now look like\n>> this when download is stalled: \"0 bytes/s\" instead of \"0 KiB/s\".\n>\n> Personally, I do not think the \"drawback\" is so big an issue.  If\n> the caller really cares, we could always add another parameter to\n> this formatter to tell it the minimum unit we care about (e.g. pass\n> 1024 to say \"Don't bother showing scale lower than kibi\").\n\nI thought about that, but decided it was not worth it (at least for the moment)\n\n> This is a bit late response, but if we ever want to count something\n> in a dimention other than \"bytes\", like time (e.g. \"kiloseconds\") or\n> number of commits (e.g. \"centicommits\"), etc., we cannot reuse this\n> formatter very easily.  We may want to have \"byte\" somewhere in its\n> name for now to make sure the callers understand its limitation.\n\nI'm not in a hurry.\nBut it look tough to make it generic: one is binary, another is\nsexagesimal, and the last is decimal\n\n> I'll tentatively rename it to \"strbuf_humanize_bytes()\" while queuing.\n\nI like the idea,\nThanks,\n"}]}