On Thu, Aug 14, 2008 at 06:18:27PM -0400, Marcus Griep wrote:
> Takes a strbuf as its first argument and appends the human-readable
> form of 'value', the second argument, to that buffer.
>
> Argument 3, 'maxlen', specifies the longest string that should
> be returned. That will make it easier for any pretty-ish formatting
> like `ls` and `du` use. A value of 0 is unlimited length.
Frankly, I doubt this has too much value, and it complicates the code _a
lot_. If you can't fit your stuff into pretty column, it's better to
just print whatever you have to and disrupt the columns instead of
_failing_, isn't it?
> 'scale' specifies a boundary, above which 'value' should be
> reduced, and below which it should be reported. Commonly this is
> 1000. If 0, then it will find a scale that best fits into 'maxlen'.
> If both 'maxlen' and 'scale' are 0, then scale will default to 1000.
>
> 'suffix' is appended onto every formatted string. This is often
> "", "B", "bps", "objects" etc.
>
> 'flags' provides the ability to switch between a binary (1024)
> and an si (1000) period (HR_USE_SI). Also, adding a space between
> number and unit (HR_SPACE).
>
> On success, returns 0. If maxlen is specified and there is not
> enough space given the scale or an inordinately large value, returns
> -n, where n is the amount of additional length necessary.
>
> e.g. strbuf_append_human_readable(sb, 1012, 0, 0, "bps", HR_SPACE)
> produces "0.9 Kbps".
Shouldn't pretty much all of this be documented in the code too?
> Also, add in test cases to ensure it produces the expected output
> and to demonstrate what different arguments do.
>
> Signed-off-by: Marcus Griep <marcus@xxxxxxxx>
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.
> +int strbuf_append_human_readable(struct strbuf *sb,
> + double val,
> + int maxlen, int scale,
> + const char *suffix,
> + int flags)
> +{
> + const int maxscale = 7;
> +
> + char *hr_prefixes[] = {
> + "", "K", "M", "G", "T", "P", "E", "Z", "Y", NULL
> + };
> + char **prefix = &hr_prefixes[0];
Whitespace damage? Also at a lot of other places in your patch.
> + int period = 1024;
> + int sign = val < 0 ? -1 : 1;
> + /* Baselen is (sign, if needed) (digit) (space, if needed)
> + (prefix) (suffix) */
> + int baselen = (val < 0 ? 1 : 0) + 1 + (flags & HR_SPACE ? 1 : 0)
> + + 1 + strlen(suffix);
> +
> + val *= sign;
> +
> + if (flags & HR_USE_SI) {
> + period = 1000;
> + hr_prefixes[1] = "k";
Hmmm. We could have
+ char *hr_prefixes[] = {
+ "", "Ki", "Mi", "Gi", "Ti", "Pi", "Ei", "Zi", "Yi", NULL
+ };
+ char *hr_si_prefixes[] = {
+ "", "k", "M", "G", "T", "P", "E", "Z", "Y", NULL
+ };
;-)
--
Petr "Pasky" Baudis
The next generation of interesting software will be done
on the Macintosh, not the IBM PC. -- Bill Gates
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
