Re: [RFC Redux] strbuf: Add method to convert byte-size to human readable form

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

Marcus Griep <marcus@griep.us> writes:

>  This is a redux of a prior patch as part of a series on count-objects
>  but is now split off and submitted on its own as an RFC for a library
>  function to be added to strbuf.

Ok, so I looked at the patch again.

> diff --git a/.gitignore b/.gitignore
> index bbaf9de..251537b 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -147,6 +147,7 @@ test-date
>  test-delta
>  test-dump-cache-tree
>  test-genrandom
> +test-human-read
>  test-match-trees
>  test-parse-options
>  test-path-utils

Is it just me or should the test called "test-human-readable"?

> diff --git a/strbuf.c b/strbuf.c
> index 720737d..d9888fb 100644
> --- a/strbuf.c
> +++ b/strbuf.c
> @@ -308,3 +308,95 @@ int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)
> ...
> +{
> +	const int maxscale = 7;

This is unused as far as I can tell.

> +	strbuf_addf(sb, "%f", sign * val);
> +
> +	if (maxlen) {
> +		int signlen = sign == -1 ? 1 : 0;
> +		maxlen -= (sb->buf[maxlen-1+signlen] == '.' ? 1 : 0);
> +		if (maxlen <= 0) {
> +			strbuf_setlen(sb, 0);
> +			retval = maxlen - 1;
> +		} else {
> +			strbuf_setlen(sb, maxlen + signlen);
> +		}
> +	}

This means you print to the buffer and then _truncate_ down to precision,
doesn't it?  Shouldn't you be rounding (possibly up if it is above the
midway point)?

For example, if I hand 1638 to you, you would give 1.599Ki back to me, but
if I give you only 4 digits to work with, you do not want to say 1.59Ki;
instead you would rather say 1.60Ki, right?

You would need to compute the number of digits you would want to see
upfront and format the value using "%.*f" with appropriate precision.

> diff --git a/test-human-read.c b/test-human-read.c
> new file mode 100644
> index 0000000..7890922
> --- /dev/null
> +++ b/test-human-read.c
> @@ -0,0 +1,23 @@
> +#include "builtin.h"
> +#include "strbuf.h"
> +
> +int main(int argc, char **argv) {
> +	if (argc != 6) {
> +		exit(-1);
> +	}
> +
> +	struct strbuf sb;

Decl after statement.

> +	strbuf_init(&sb, 0);
> +
> +	int retval = strbuf_append_human_readable(&sb,
> +		atof(argv[1]), atoi(argv[2]), atoi(argv[3]), atoi(argv[4]));
> +
> +	int failed = strcmp(sb.buf, argv[5]);
> +
> +	fprintf( stderr, failed ? "Failure" : "Success" );
> +	fprintf( stderr, ": Act '%s'; Exp '%s'\n", sb.buf, argv[5] );
> +	fprintf( stderr, "Return Value: %d\n", retval );

Style.

> +
> +	if(failed) return -1;

Style.

> +	return retval;
> +}
> -- 
> 1.6.0.1.451.gc8d31