From: Jonathan Nieder <jrnieder@gmail.com>
Date: Wed, 10 Apr 2013 19:43:07 GMT
Subject: Re: [PATCH 1/2] strbuf: create strbuf_humanize() to show byte sizes
Message-ID: <20130410194307.GA27070@google.com>
In-Reply-To: <1365620604-17851-1-git-send-email-apelisse@gmail.com>

Antoine Pelisse wrote:

> Separate text formatting from size simplification and make the function
> public in strbuf so that it can easily be used by other clients.
>
> We now can use strbuf_humanize() for both downloaded size and download
> speed calculation.

Sounds like a good thing to do.

>                    One of the drawbacks is that speed will now look like
> this when download is stalled: "0 bytes/s" instead of "0 KiB/s".

At first glance that is neither obviously a benefit nor obviously a
drawback.  Can you spell this out more?

> --- a/Documentation/technical/api-strbuf.txt
> +++ b/Documentation/technical/api-strbuf.txt
> @@ -230,6 +230,11 @@ which can be used by the programmer of the callback as she sees fit.
>  	destination. This is useful for literal data to be fed to either
>  	strbuf_expand or to the *printf family of functions.
>  
> +`strbuf_humanize`::
> +
> +	Append the given byte size as a human-readable string (i.e. 12.23 KiB,
> +	3.50 MiB).

Based on the function name alone, it is not easy to guess what it will
do (e.g., maybe it will paraphrase 3 to "three" and 10000000 to
"enormous").  How about something like strbuf_filesize?

If I understand the code correctly, this jumps units each time it
exceeds 1.0 of the next unit (bytes, KiB, MiB, GiB), which sounds like
a fine behavior.

Hope that helps,
Jonathan