Subject: [PATCH v3 0/4] Progress display fixes

From: SZEDER Gábor <[email-masked]>
Date: Fri, 5 Apr 2019 02:45:35 +0200

This patch series fixes two progress display issues by breaking
progress bars longer than the width of the terminal and by properly
cleaning up the previously shown progress bar.

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 <[email-masked]>
     
    @@ -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