Jeff King [off-list ref] writes:
a little more readable, or even:
stop_progress_msg(&progress, p->next ? NULL : "done\n");
Yeah, this one looks neat.
But I almost wonder if it is worth factoring out the concept of a
"progress group", where you would call it like:
progress = progress_group_start("Checking objects in pack", nr_packs);
for (p = packed_git; p; p = p->next) {
progress_group_next(progress, p->num_objects);
for (j = 0; j < num; j++) {
fsck_sha1(p, j);
display_progress(progress, j+1);
}
}
stop_progress(&progress);
Hmm, once you do this kind of thing I would expect two (or more) progress
bars to be shown, something like:
$ git fsck --progress
checking packs: 3 of 7 (42% done)
checking objects in pack: 12405 of 332340 (4% done)
On Thu, Nov 03, 2011 at 12:43:59PM -0700, Junio C Hamano wrote:
quoted
But I almost wonder if it is worth factoring out the concept of a
"progress group", where you would call it like:
progress = progress_group_start("Checking objects in pack", nr_packs);
for (p = packed_git; p; p = p->next) {
progress_group_next(progress, p->num_objects);
for (j = 0; j < num; j++) {
fsck_sha1(p, j);
display_progress(progress, j+1);
}
}
stop_progress(&progress);
Hmm, once you do this kind of thing I would expect two (or more) progress
bars to be shown, something like:
$ git fsck --progress
checking packs: 3 of 7 (42% done)
checking objects in pack: 12405 of 332340 (4% done)
I don't think we can do multiple lines portably, though, because the
progress code just uses "\r" to return to the beginning of the line.
However, I do think it's a nice design even if the output is the same
right now, because the calling code is specifying more clearly to the
progress code what it actually means. Which means it is easier to
tweak the progress code later to make a prettier representation of that
meaning.
One downside (as with all abstractions :) ), is that it's hard to
deviate from the defaults. With the above, how would you specify if it
was a group of throughput measurements, not counts? Or that you wanted
to use start_progress_delay instead of start_progress on each one?
Or hey, if you really want to get crazy, why can't we check each pack in
parallel on a different CPU, and have all of the progress meters
displayed and moving simultaneously? :)
The last one is obviously a bit tongue in cheek. But it does raise an
interesting point: is seeing the per-pack meter actually useful (whether
parallel or not)? The user just wants to know that progress is being
made, and when it is done. And maybe that argues for just summing the
size of each pack and showing a single progress meter per task.
-Peff