Re: [PATCH v2 2/4] progress: fix progress meters when dealing with lots of work

Thanks for the reviews!

On Mon, Nov 13, 2017 at 1:33 PM, Jonathan Tan [off-list ref] wrote:

On Mon, 13 Nov 2017 12:15:58 -0800
Elijah Newren [off-list ref] wrote:

quoted

-static int display(struct progress *progress, unsigned n, const char *done)
+static int display(struct progress *progress, uint64_t n, const char *done)
 {
      const char *eol, *tp;

@@ -106,7 +106,7 @@ static int display(struct progress *progress, unsigned n, const char *done)
              if (percent != progress->last_percent || progress_update) {
                      progress->last_percent = percent;
                      if (is_foreground_fd(fileno(stderr)) || done) {
-                             fprintf(stderr, "%s: %3u%% (%u/%u)%s%s",
+                             fprintf(stderr, "%s: %3u%% (%"PRIuMAX"/%"PRIuMAX")%s%s",
                                      progress->title, percent, n,
                                      progress->total, tp, eol);

I think it would be better to cast the appropriate arguments to
uintmax_t - searching through the Git code shows that we do that in
several situations. Same for the rest of the diff.

Interesting.  My first inclination was to ask why not just change the
variables to be of type uintmax_t instead of uint64_t (since we're
already changing their types already), and then get rid of the cast.
But I went digging through the source code based on your comment.
Almost all the existing examples in the codebase were off_t and size_t
values; there was only one case with uint64_t...but that one case led
me to commit 5be507fc95 (Use PRIuMAX instead of 'unsigned long long'
in show-index 2007-10-21), and that commit does suggest doing exactly
as you say here.

I'll fix it up.