Antoine Pelisse <apelisse@xxxxxxxxx> 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 <apelisse@xxxxxxxxx>
> ---
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 <string.h>
> +
Please do not do this.
If you somehow need to have <string.h>, 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);
> }
> @@ -253,9 +258,12 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
>
> bufp = (len < sizeof(buf)) ? buf : xmalloc(len + 1);
> if (tp) {
> + struct strbuf strbuf = STRBUF_INIT;
> unsigned int rate = !tp->avg_misecs ? 0 :
> tp->avg_bytes / tp->avg_misecs;
> - throughput_string(tp, tp->curr_total, rate);
> + throughput_string(&strbuf, tp->curr_total, rate);
> + strncpy(tp->display, strbuf.buf, sizeof(tp->display));
> + strbuf_release(&strbuf);
> }
> progress_update = 1;
> sprintf(bufp, ", %s.\n", msg);
> diff --git a/progress.h b/progress.h
> index 611e4c4..0e70f55 100644
> --- a/progress.h
> +++ b/progress.h
> @@ -2,7 +2,9 @@
> #define PROGRESS_H
>
> struct progress;
> +struct strbuf;
>
> +void humanize(struct strbuf *buf, off_t bytes);
> void display_throughput(struct progress *progress, off_t total);
> int display_progress(struct progress *progress, unsigned n);
> struct progress *start_progress(const char *title, unsigned total);
--
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