Antoine Pelisse writes:
> Currently, humanization of downloaded size is done in the same function
> as text formatting. This is an issue if anyone else wants to use this.
>
> Separate text formatting from size simplification and make the function
> public so that it can easily be used by other clients.
>
> We now can use humanize() for both downloaded size and download speed
> calculation. One of the drawbacks is that speed will no look like this
> when download is stalled: "0 bytes/s" instead of "0 KiB/s".
>
> Signed-off-by: Antoine Pelisse
> ---
Sounds good, but I think this helper function should live in
strbuf.[ch], where many other text/string helpers that take a strbuf
as their first parameter live.
> progress.c | 60
> ++--
> progress.h |2 ++
> 2 files changed, 36 insertions(+), 26 deletions(-)
>
> diff --git a/progress.c b/progress.c
> index 3971f49..76c1e42 100644
> --- a/progress.c
> +++ b/progress.c
> @@ -8,8 +8,11 @@
> * published by the Free Software Foundation.
> */
>
> +#include
> +
Please do not do this.
If you somehow need to have , a suitable place should be
found in git-compat-util.h; various platforms seem to have quirks
with the ordering of system header files, and git-compat-util.h is
meant to encapsulate them.
In fact, git-compat-util.h should already include it.
> #include "git-compat-util.h"
> #include "progress.h"
> +#include "strbuf.h"
>
> #define TP_IDX_MAX 8
>
> @@ -112,34 +115,33 @@ static int display(struct progress *progress, unsigned
> n, const char *done)
> return 0;
> }
>
> -static void throughput_string(struct throughput *tp, off_t total,
> - unsigned int rate)
> +void humanize(struct strbuf *buf, off_t bytes)
> {
> - int l = sizeof(tp->display);
> - if (total > 1 << 30) {
> - l -= snprintf(tp->display, l, ", %u.%2.2u GiB",
> - (int)(total >> 30),
> - (int)(total & ((1 << 30) - 1)) / 10737419);
> - } else if (total > 1 << 20) {
> - int x = total + 5243; /* for rounding */
> - l -= snprintf(tp->display, l, ", %u.%2.2u MiB",
> - x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
> - } else if (total > 1 << 10) {
> - int x = total + 5; /* for rounding */
> - l -= snprintf(tp->display, l, ", %u.%2.2u KiB",
> - x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
> + if (bytes > 1 << 30) {
> + strbuf_addf(buf, "%u.%2.2u GiB",
> + (int)(bytes >> 30),
> + (int)(bytes & ((1 << 30) - 1)) / 10737419);
> + } else if (bytes > 1 << 20) {
> + int x = bytes + 5243; /* for rounding */
> + strbuf_addf(buf, "%u.%2.2u MiB",
> + x >> 20, ((x & ((1 << 20) - 1)) * 100) >> 20);
> + } else if (bytes > 1 << 10) {
> + int x = bytes + 5; /* for rounding */
> + strbuf_addf(buf, "%u.%2.2u KiB",
> + x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
> } else {
> - l -= snprintf(tp->display, l, ", %u bytes", (int)total);
> + strbuf_addf(buf, "%u bytes", (int)bytes);
> }
> +}
>
> - if (rate > 1 << 10) {
> - int x = rate + 5; /* for rounding */
> - snprintf(tp->display + sizeof(tp->display) - l, l,
> - " | %u.%2.2u MiB/s",
> - x >> 10, ((x & ((1 << 10) - 1)) * 100) >> 10);
> - } else if (rate)
> - snprintf(tp->display + sizeof(tp->display) - l, l,
> - " | %u KiB/s", rate);
> +static void throughput_string(struct strbuf *buf, off_t total,
> + unsigned int rate)
> +{
> + strbuf_addstr(buf, ", ");
> + humanize(buf, total);
> + strbuf_addstr(buf, " | ");
> + humanize(buf, rate * 1024);
> + strbuf_addstr(buf, "/s");
> }
>
> void display_throughput(struct progress *progress, off_t total)
> @@ -183,6 +185,7 @@ void display_throughput(struct progress *progress, off_t
> total)
> misecs += (int)(tv.tv_usec - tp->prev_tv.tv_usec) / 977;
>
> if (misecs > 512) {
> + struct strbuf buf = STRBUF_INIT;
> unsigned int count, rate;
>
> count = total - tp->prev_total;
> @@ -197,7 +200,9 @@ void display_throughput(struct progress *progress, off_t
> total)
> tp->last_misecs[tp->idx] = misecs;
> tp->idx = (tp->idx + 1) % TP_IDX_MAX;
>
> - throughput_string(tp, total, rate);
> + throughput_string(&buf, total, rate);
> + strncpy(tp->display, buf.buf, sizeof(tp->display));
> + strbuf_release(&buf);
> if (progress->last_value != -1 && progress_update)
> display(progress, progress->last_value, NULL);
> }