[v2,2/4] progress: assemble percentage and counters in a strbuf before printing

Commit Message

SZEDER Gábor April 1, 2019, 11:52 a.m. UTC

The following patches in this series want to handle the progress bar's
title and changing parts (i.e. the counter and the optional percentage
and throughput combined) differently, and need to know the length
of the changing parts of the previously displayed progress bar.

To prepare for those changes assemble the changing parts in a separate
strbuf kept in 'struct progress' before printing.

Signed-off-by: SZEDER Gábor <[e-mail]>
---
 progress.c | 37 +++++++++++++++++++++++--------------
 1 file changed, 23 insertions(+), 14 deletions(-)

Comments

Eric Sunshine April 2, 2019, 5:45 a.m. UTC | #1

On Mon, Apr 1, 2019 at 7:52 AM SZEDER Gábor <[e-mail]> wrote:
> [...]
> To prepare for those changes assemble the changing parts in a separate
> strbuf kept in 'struct progress' before printing.
>
> Signed-off-by: SZEDER Gábor <[e-mail]>
> ---
> diff --git a/progress.c b/progress.c
> @@ -80,36 +81,42 @@ static int is_foreground_fd(int fd)
>  static void display(struct progress *progress, uint64_t n, const char *done)
>  {
>         if (progress->total) {
>                 if (percent != progress->last_percent || progress_update) {
>                         [...]
> -                       progress_update = 0;
> -                       return;
>                 }
> +       if (show_update) {
>                 [...]
>                 progress_update = 0;
> -               return;
>         }

Removal of these two 'returns' is unrelated to the change made by this
patch and should have been done by 1/4.

>         return;

Likewise, this final 'return' doesn't need to be here and should go away in 1/4.

Eric Sunshine April 2, 2019, 5:50 a.m. UTC | #2

On Tue, Apr 2, 2019 at 1:45 AM Eric Sunshine <[e-mail]> wrote:
> On Mon, Apr 1, 2019 at 7:52 AM SZEDER Gábor <[e-mail]> wrote:
> > +       if (show_update) {
> >                 [...]
> >                 progress_update = 0;
> > -               return;
> >         }
>
> Removal of these two 'returns' is unrelated to the change made by this
> patch and should have been done by 1/4.
>
> >         return;
>
> Likewise, this final 'return' doesn't need to be here and should go away in 1/4.

I forgot to mention that these are micro-nits, not necessarily worth a re-roll.

Patch

diff --git a/progress.c b/progress.c
index 02a20e7d58..842db14b72 100644
--- a/progress.c
+++ b/progress.c
@@ -36,6 +36,7 @@  struct progress {
 	unsigned delay;
 	struct throughput *throughput;
 	uint64_t start_ns;
+	struct strbuf counters_sb;
 };
 
 static volatile sig_atomic_t progress_update;
@@ -80,36 +81,42 @@  static int is_foreground_fd(int fd)
 
 static void display(struct progress *progress, uint64_t n, const char *done)
 {
-	const char *eol, *tp;
+	const char *tp;
+	struct strbuf *counters_sb = &progress->counters_sb;
+	int show_update = 0;
 
 	if (progress->delay && (!progress_update || --progress->delay))
 		return;
 
 	progress->last_value = n;
 	tp = (progress->throughput) ? progress->throughput->display.buf : "";
-	eol = done ? done : "   \r";
 	if (progress->total) {
 		unsigned percent = n * 100 / progress->total;
 		if (percent != progress->last_percent || progress_update) {
 			progress->last_percent = percent;
-			if (is_foreground_fd(fileno(stderr)) || done) {
-				fprintf(stderr, "%s: %3u%% (%"PRIuMAX"/%"PRIuMAX")%s%s",
-					progress->title, percent,
-					(uintmax_t)n, (uintmax_t)progress->total,
-					tp, eol);
-				fflush(stderr);
-			}
-			progress_update = 0;
-			return;
+
+			strbuf_reset(counters_sb);
+			strbuf_addf(counters_sb,
+				    "%3u%% (%"PRIuMAX"/%"PRIuMAX")%s", percent,
+				    (uintmax_t)n, (uintmax_t)progress->total,
+				    tp);
+			show_update = 1;
 		}
 	} else if (progress_update) {
+		strbuf_reset(counters_sb);
+		strbuf_addf(counters_sb, "%"PRIuMAX"%s", (uintmax_t)n, tp);
+		show_update = 1;
+	}
+
+	if (show_update) {
 		if (is_foreground_fd(fileno(stderr)) || done) {
-			fprintf(stderr, "%s: %"PRIuMAX"%s%s",
-				progress->title, (uintmax_t)n, tp, eol);
+			const char *eol = done ? done : "   \r";
+
+			fprintf(stderr, "%s: %s%s", progress->title,
+				counters_sb->buf, eol);
 			fflush(stderr);
 		}
 		progress_update = 0;
-		return;
 	}
 
 	return;
@@ -211,6 +218,7 @@  static struct progress *start_progress_delay(const char *title, uint64_t total,
 	progress->delay = delay;
 	progress->throughput = NULL;
 	progress->start_ns = getnanotime();
+	strbuf_init(&progress->counters_sb, 0);
 	set_progress_signal();
 	return progress;
 }
@@ -254,6 +262,7 @@  void stop_progress_msg(struct progress **p_progress, const char *msg)
 		free(buf);
 	}
 	clear_progress_signal();
+	strbuf_release(&progress->counters_sb);
 	if (progress->throughput)
 		strbuf_release(&progress->throughput->display);
 	free(progress->throughput);