[PATCH v3 0/4] Progress display fixes

From

SZEDER Gábor <szeder.dev@gmail.com>

Date

Apr 5, 2019, 00:45 UTC

Message-ID

<20190405004539.31467-1-szeder.dev@gmail.com>

In-Reply-To

<20190401115217.3423-1-szeder.dev@gmail.com>

Changes since v2, following Eric's and Peff's suggestions:

  - Remove return statements that just became unnecessary in patch
    1/4.

  - Use size_t helper variables to store intermediate results of
    calculations based on length of strings.

SZEDER Gábor (4):
  progress: make display_progress() return void
  progress: assemble percentage and counters in a strbuf before printing
  progress: clear previous progress update dynamically
  progress: break too long progress bar lines

 progress.c | 73 ++++++++++++++++++++++++++++++++++++++----------------
 progress.h |  2 +-
 2 files changed, 53 insertions(+), 22 deletions(-)

Interdiff:

diff --git a/progress.c b/progress.c
index e28ccdafd2..97e18671e5 100644
--- a/progress.c
+++ b/progress.c
@@ -115,32 +115,30 @@ static void display(struct progress *progress, uint64_t n, const char *done)
 	if (show_update) {
 		if (is_foreground_fd(fileno(stderr)) || done) {
 			const char *eol = done ? done : "\r";
-			int clear_len = counters_sb->len < last_count_len ?
+			size_t clear_len = counters_sb->len < last_count_len ?
 					last_count_len - counters_sb->len : 0;
-			int progress_line_len = progress->title_len +
+			size_t progress_line_len = progress->title_len +
 						counters_sb->len + 2;
 			int cols = term_columns();
 
 			if (progress->split) {
 				fprintf(stderr, "  %s%-*s", counters_sb->buf,
-					clear_len, eol);
+					(int) clear_len, eol);
 			} else if (!done && cols < progress_line_len) {
 				clear_len = progress->title_len + 1 < cols ?
 					    cols - progress->title_len - 1 : 0;
 				fprintf(stderr, "%s:%*s\n  %s%s",
-					progress->title, clear_len, "",
+					progress->title, (int) clear_len, "",
 					counters_sb->buf, eol);
 				progress->split = 1;
 			} else {
 				fprintf(stderr, "%s: %s%-*s", progress->title,
-					counters_sb->buf, clear_len, eol);
+					counters_sb->buf, (int) clear_len, eol);
 			}
 			fflush(stderr);
 		}
 		progress_update = 0;
 	}
-
-	return;
 }
 
 static void throughput_string(struct strbuf *buf, uint64_t total,
Range-diff:
1:  dea36bd2a7 ! 1:  cb68e5b0ec progress: make display_progress() return void
    @@ -18,7 +18,9 @@
         its introduction in cf84d51c43 (add throughput to progress display,
         2007-10-30).
     
    -    Let's make display_progress() return void, too.
    +    Let's make display_progress() return void, too.  While doing so
    +    several return statements in display() become unnecessary, remove
    +    them.
     
         Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
     
    @@ -45,7 +47,6 @@
      			}
      			progress_update = 0;
     -			return 1;
    -+			return;
      		}
      	} else if (progress_update) {
      		if (is_foreground_fd(fileno(stderr)) || done) {
    @@ -54,11 +55,9 @@
      		}
      		progress_update = 0;
     -		return 1;
    -+		return;
      	}
    - 
    +-
     -	return 0;
    -+	return;
      }
      
      static void throughput_string(struct strbuf *buf, uint64_t total,
2:  97de2a98a0 ! 2:  017d095142 progress: assemble percentage and counters in a strbuf before printing
    @@ -50,7 +50,6 @@
     -				fflush(stderr);
     -			}
     -			progress_update = 0;
    --			return;
     +
     +			strbuf_reset(counters_sb);
     +			strbuf_addf(counters_sb,
    @@ -76,10 +75,6 @@
      			fflush(stderr);
      		}
      		progress_update = 0;
    --		return;
    - 	}
    - 
    - 	return;
     @@
      	progress->delay = delay;
      	progress->throughput = NULL;
3:  edfe0157a7 ! 3:  c5a4def5ac progress: clear previous progress update dynamically
    @@ -48,10 +48,10 @@
     -			fprintf(stderr, "%s: %s%s", progress->title,
     -				counters_sb->buf, eol);
     +			const char *eol = done ? done : "\r";
    -+			int clear_len = counters_sb->len < last_count_len ?
    ++			size_t clear_len = counters_sb->len < last_count_len ?
     +					last_count_len - counters_sb->len : 0;
     +			fprintf(stderr, "%s: %s%-*s", progress->title,
    -+				counters_sb->buf, clear_len, eol);
    ++				counters_sb->buf, (int) clear_len, eol);
      			fflush(stderr);
      		}
      		progress_update = 0;
4:  d53de231ee ! 4:  2f44dff84e progress: break too long progress bar lines
    @@ -70,27 +70,27 @@
      static volatile sig_atomic_t progress_update;
     @@
      			const char *eol = done ? done : "\r";
    - 			int clear_len = counters_sb->len < last_count_len ?
    + 			size_t clear_len = counters_sb->len < last_count_len ?
      					last_count_len - counters_sb->len : 0;
     -			fprintf(stderr, "%s: %s%-*s", progress->title,
    --				counters_sb->buf, clear_len, eol);
    -+			int progress_line_len = progress->title_len +
    +-				counters_sb->buf, (int) clear_len, eol);
    ++			size_t progress_line_len = progress->title_len +
     +						counters_sb->len + 2;
     +			int cols = term_columns();
     +
     +			if (progress->split) {
     +				fprintf(stderr, "  %s%-*s", counters_sb->buf,
    -+					clear_len, eol);
    ++					(int) clear_len, eol);
     +			} else if (!done && cols < progress_line_len) {
     +				clear_len = progress->title_len + 1 < cols ?
     +					    cols - progress->title_len - 1 : 0;
     +				fprintf(stderr, "%s:%*s\n  %s%s",
    -+					progress->title, clear_len, "",
    ++					progress->title, (int) clear_len, "",
     +					counters_sb->buf, eol);
     +				progress->split = 1;
     +			} else {
     +				fprintf(stderr, "%s: %s%-*s", progress->title,
    -+					counters_sb->buf, clear_len, eol);
    ++					counters_sb->buf, (int) clear_len, eol);
     +			}
      			fflush(stderr);
      		}

-- 
2.21.0.539.g07239c3a71.dirty