Re: [PATCH] reflog expire: add progress output

[Duy Nguyen <pclouds@gmail.com>]

On Wed, Sep 19, 2018 at 07:22:44PM +0200, Ævar Arnfjörð Bjarmason wrote:
> >> @@ -225,14 +226,20 @@ static void mark_reachable(struct expire_reflog_policy_cb *cb)
> >>         struct commit_list *pending;
> >>         timestamp_t expire_limit = cb->mark_limit;
> >>         struct commit_list *leftover = NULL;
> >> +       struct progress *progress = NULL;
> >> +       int i = 0;
> >>
> >>         for (pending = cb->mark_list; pending; pending = pending->next)
> >>                 pending->item->object.flags &= ~REACHABLE;
> >>
> >>         pending = cb->mark_list;
> >> +       progress = start_delayed_progress(
> >> +               _("Marking unreachable commits in reflog for expiry"), 0);
> >
> > Maybe just "Searching expired reflog entries" or something like that.
> > It's not technically as accurate, but I think it's easier to
> > understand by by new people.
> 
> Or "Pruning reflog entries"? I was aiming for some combination of a)
> making it clear what we're doing (pruning stuff) b) that we're doing it
> on a subset of the counter of the (very large) values we're showing.
> 
> So the current one has "a" && "b", "Searching..." has neither, and
> "Pruning..." has "a" but not "b".
> 
> Maybe making a && b clear isn't that important, but I'm currently
> leaning towards keeping the current one because it's not *that* long and
> makes things clearer to the user.

OK

> >>         while (pending) {
> >>                 struct commit_list *parent;
> >>                 struct commit *commit = pop_commit(&pending);
> >> +
> >> +               display_progress(progress, ++i);
> >
> > maybe rename it to commit_count or something and leave "i" for
> > temporary/short lived usage.
> 
> Good point. Willdo.

Actually I'm still not sure if it's valuable to show the actual commit
count here. Some patch like this could show "activity" without the
number. But this is tangent.

-- 8< --
Subject: [PATCH] progress: add api for displaying a throbber

---
 progress.c | 29 +++++++++++++++++++++++++----
 progress.h |  1 +
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/progress.c b/progress.c
index 5a99c9fbf0..fada556bf0 100644
--- a/progress.c
+++ b/progress.c
@@ -36,6 +36,7 @@ struct progress {
 	unsigned delay;
 	struct throughput *throughput;
 	uint64_t start_ns;
+	int show_throbber;
 };
 
 static volatile sig_atomic_t progress_update;
@@ -81,11 +82,13 @@ static int is_foreground_fd(int fd)
 static int display(struct progress *progress, uint64_t n, const char *done)
 {
 	const char *eol, *tp;
+	static char throbber[] = "\\|/-";
 
 	if (progress->delay && (!progress_update || --progress->delay))
 		return 0;
 
-	progress->last_value = n;
+	if (!progress->show_throbber)
+		progress->last_value = n;
 	tp = (progress->throughput) ? progress->throughput->display.buf : "";
 	eol = done ? done : "   \r";
 	if (progress->total) {
@@ -104,8 +107,15 @@ static int display(struct progress *progress, uint64_t n, const char *done)
 		}
 	} else if (progress_update) {
 		if (is_foreground_fd(fileno(stderr)) || done) {
-			fprintf(stderr, "%s: %"PRIuMAX"%s%s",
-				progress->title, (uintmax_t)n, tp, eol);
+			if (progress->show_throbber) {
+				progress->last_value++;
+				fprintf(stderr, "%s: %c%s%s",
+					progress->title,
+					throbber[progress->last_value % 4],
+					tp, eol);
+			} else
+				fprintf(stderr, "%s: %"PRIuMAX"%s%s",
+					progress->title, (uintmax_t)n, tp, eol);
 			fflush(stderr);
 		}
 		progress_update = 0;
@@ -193,6 +203,14 @@ int display_progress(struct progress *progress, uint64_t n)
 	return progress ? display(progress, n, NULL) : 0;
 }
 
+void display_throbber(struct progress *progress)
+{
+	if (progress) {
+		progress->show_throbber = 1;
+		display(progress, 0, NULL);
+	}
+}
+
 static struct progress *start_progress_delay(const char *title, uint64_t total,
 					     unsigned delay)
 {
@@ -248,7 +266,10 @@ void stop_progress_msg(struct progress **p_progress, const char *msg)
 			throughput_string(&tp->display, tp->curr_total, rate);
 		}
 		progress_update = 1;
-		buf = xstrfmt(", %s.\n", msg);
+		if (progress->show_throbber)
+			buf = xstrfmt(" %s.\n", msg);
+		else
+			buf = xstrfmt(", %s.\n", msg);
 		display(progress, progress->last_value, buf);
 		free(buf);
 	}
diff --git a/progress.h b/progress.h
index 70a4d4a0d6..9d23ff242f 100644
--- a/progress.h
+++ b/progress.h
@@ -5,6 +5,7 @@ struct progress;
 
 void display_throughput(struct progress *progress, uint64_t total);
 int display_progress(struct progress *progress, uint64_t n);
+void display_throbber(struct progress *progress);
 struct progress *start_progress(const char *title, uint64_t total);
 struct progress *start_delayed_progress(const char *title, uint64_t total);
 void stop_progress(struct progress **progress);
-- 
2.19.0.rc0.337.ge906d732e7

-- 8< --