| Message ID | 20190325103844.26749-4-szeder.dev@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Progress display fixes | expand |
On Mon, Mar 25, 2019 at 11:38:42AM +0100, SZEDER Gábor wrote: > 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. > > To prepare for those changes assemble the changing parts in a separate > strbuf before printing. Makes sense. Unlike the previous patch, we're already in the "slow path" of the function here, so we don't need to worry about adding an extra buffer (though do still think it's worth reusing the same buffer each time, as you do here). -Peff
On Tue, Mar 26, 2019 at 01:45:41AM -0400, Jeff King wrote: > On Mon, Mar 25, 2019 at 11:38:42AM +0100, SZEDER Gábor wrote: > > > 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. > > > > To prepare for those changes assemble the changing parts in a separate > > strbuf before printing. > > Makes sense. Unlike the previous patch, we're already in the "slow path" > of the function here, so we don't need to worry about adding an extra > buffer (though do still think it's worth reusing the same buffer each > time, as you do here). The commit message doesn't mention this, but the next patch needs the length of the previously displayed progress bar to properly clean up its remnants. Or the length of its changing parts anyway. So I could either add a 'prev_len' field to 'struct progress', or the whole strbuf. The strbuf containing the throughput is already stored in there and reused, so I just followed suit.
On Wed, Mar 27, 2019 at 11:24:51AM +0100, SZEDER Gábor wrote: > On Tue, Mar 26, 2019 at 01:45:41AM -0400, Jeff King wrote: > > On Mon, Mar 25, 2019 at 11:38:42AM +0100, SZEDER Gábor wrote: > > > > > 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. > > > > > > To prepare for those changes assemble the changing parts in a separate > > > strbuf before printing. > > > > Makes sense. Unlike the previous patch, we're already in the "slow path" > > of the function here, so we don't need to worry about adding an extra > > buffer (though do still think it's worth reusing the same buffer each > > time, as you do here). > > The commit message doesn't mention this, but the next patch needs the > length of the previously displayed progress bar to properly clean up > its remnants. Or the length of its changing parts anyway. So I could > either add a 'prev_len' field to 'struct progress', or the whole > strbuf. The strbuf containing the throughput is already stored in > there and reused, so I just followed suit. Ah, right, that makes perfect sense. It is doubly a good idea, then. :) -Peff
diff --git a/progress.c b/progress.c index b57c0dae16..390d2487ca 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,7 +81,9 @@ 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 update = 0; if (progress->delay && (!progress_update || --progress->delay)) return; @@ -93,25 +96,31 @@ static void display(struct progress *progress, uint64_t n, const char *done) } 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; - 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); + update = 1; } } else if (progress_update) { - fprintf(stderr, "%s: %"PRIuMAX"%s%s", - progress->title, (uintmax_t)n, tp, eol); + strbuf_reset(counters_sb); + strbuf_addf(counters_sb, "%"PRIuMAX"%s", (uintmax_t)n, tp); + update = 1; + } + + if (update) { + const char *eol = done ? done : " \r"; + + fprintf(stderr, "%s: %s%s", progress->title, counters_sb->buf, + eol); fflush(stderr); progress_update = 0; - return; } return; @@ -213,6 +222,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; } @@ -256,6 +266,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);
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. To prepare for those changes assemble the changing parts in a separate strbuf before printing. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> --- progress.c | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-)