Re: [PATCH v2 2/3] strbuf: Add method to convert byte-size to human readable form

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

Petr Baudis <pasky@suse.cz> writes:

> My point still stands - in case of binary units, we should always
> consistently use the i suffix. So having an example in the commit
> message that advertises "bps" is simply wrong when it should read "iB/s"
> (like it does with the current progress.c code).
>
> I may sound boring, but it seems to me that you're still ignoring my
> point quitly without proper counter-argumentation and I think it's an
> important want, and since it's so hard to keep things consistent across
> the wide Git codebase, we should do all we can to keep it.

I pretty much agree with everything you said in this thread.  In addition,
I wonder if we would want to be able to say:

	960 bps
        0.9 KiB/s
	2.3 MiB/s

IOW, I do not think it is a good idea to have the list of "prefixes" in
this function and force callers to _append_ unit.  You might be better off
by making the interface to the function to pass something like this:

	struct human_unit {
		char *unitname;
                unsigned long valuescale;
	} bps_to_human[] = {
        	{ "bps", 1 },
                { "KiB/s", 1024 },
                { "MiB/s", 1024 * 1024 },
                { NULL, 0 },
	};

and perhaps give canned set of unit list for sizes and throughputs as
convenience.

By doing so, you could even do this:

	struct human_unit bits_to_human[] = {
        	{ "bits", 1 },
                { "bytes", 8 },
                { "Kbytes", 8 * 1024 },
                { "Mbytes", 8 * 1024 * 1024 },
                { NULL, 0 },
	};

I also am not particularly happy about using "double" in this API.  Most
of the callers that gather stats in the rest of the codebase count in
(long) integers as far as I can tell, and it may be conceptually cleaner
to keep the use of double as an internal implementation issue of this
particular function.