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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 14, 2008, 23:04 UTC
Message-ID: <7viqu3ci5k.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <20080814223429.GC10544@machine.or.cz>

Petr Baudis <pasky@suse.cz> writes:

Show 9 quoted lines

> 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.