Re: [PATCH 2] count-objects: add human-readable size option

[Junio C Hamano <gitster@pobox.com>]

"Shawn O. Pearce" <spearce@spearce.org> writes:

> Marcus Griep <marcus@griep.us> wrote:
>> diff --git a/Documentation/git-count-objects.txt b/Documentation/git-count-objects.txt
>> index 75a8da1..291bc5e 100644
>> --- a/Documentation/git-count-objects.txt
>> +++ b/Documentation/git-count-objects.txt
>> +++ b/builtin-count-objects.c
> ...
>> +void human_readable_size(char *buf, int buf_size, double size /* in bytes */)
>
> Hmm.  This probably should be static.  Or if it really is meant
> to be a utility for use elsewhere in Git, moved to someplace where
> string handling is done.  Its not strbuf related, but maybe strbuf.c
> is a better location for this sort of library function.

Yes, with customizable precision (so that the caller can control "1.6k" vs
"1.62k"), and perhaps cutomizable unit (so that you can use this for
"3.6kB" and "2.6Mbps"), this kind of thing is a good candidate to be a
library function in strbuf.c.

>> +{
>> +	char human_readable_prefixes[10] = "BKMGTPEZY";

This enumerates suffix if I am not mistaken.  Do you have to say "10"
here, or does the compiler counts them for you?

>> +	for (; i < 8 && size >= 1000 ; ++i, size = size / 1024)
>> +		;

I do not think you would need to use the magic number "8" here.

I have this suspicion that the caller, if this is made into a generic
library, would want to pass in a list of units, not magnitude suffixes,
like this:

    extern int human_readable(struct strbuf *,
    			      double value, int precision,
                              const char **unit);

    static const char **size_unit = {
    	"byte", "KB", "MB", "GB", NULL,
    };
    static const char **throughput_unit = {
    	"bps", "Kbps", "Mbps", "Gbps", NULL,
    };

    human_readble(&sb, (double) bytes_transferred, 0, size_unit);
    human_readble(&sb, (double) throughput, 2, throughput_unit);