Re: [PATCH v2 1/2] format-patch: have progress option while generating patches

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Perhaps we may want to replace the calls to progress_delay() with a
> call to a simpler wrapper that does not let the callers give their
> own delay threashold to simplify the API.

... which does not look too bad, but because it makes me wonder if
we even need to make the delay-threshold customizable per callers,
I'll wait further discussions before committing to the approach.

For example, I do not quite understand why 95 is a good value for
prune-packed while 0 is a good value for prune.  

The rename detection in diffcore-rename, delay in blame, and
checkout (via unpack-trees) all of which use 50-percent threshold
with 1 second delay, sort of make sense to me in that if we
completed 50 percent within 1 second, it is likely we will finish
all in 2 seconds (which is the norm for everybody else), perhaps as
a better version of 0-percent 2 seconds rule.

 builtin/blame.c        |  3 +--
 builtin/fsck.c         |  2 +-
 builtin/prune-packed.c |  4 ++--
 builtin/prune.c        |  2 +-
 builtin/rev-list.c     |  2 +-
 diffcore-rename.c      |  4 ++--
 progress.c             | 10 ++++++++--
 progress.h             |  4 ++--
 unpack-trees.c         |  3 +--
 9 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index bda1a78726..05115900f3 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -925,8 +925,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	sb.found_guilty_entry = &found_guilty_entry;
 	sb.found_guilty_entry_data = &pi;
 	if (show_progress)
-		pi.progress = start_progress_delay(_("Blaming lines"),
-						   sb.num_lines, 50, 1);
+		pi.progress = start_delayed_progress(_("Blaming lines"), sb.num_lines, 50);
 
 	assign_blame(&sb, opt);
 
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 99dea7adf6..6a26079ebc 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -188,7 +188,7 @@ static int traverse_reachable(void)
 	unsigned int nr = 0;
 	int result = 0;
 	if (show_progress)
-		progress = start_progress_delay(_("Checking connectivity"), 0, 0, 2);
+		progress = start_delayed_progress(_("Checking connectivity"), 0, 0);
 	while (pending.nr) {
 		struct object_array_entry *entry;
 		struct object *obj;
diff --git a/builtin/prune-packed.c b/builtin/prune-packed.c
index ac978ad401..4fc6b175de 100644
--- a/builtin/prune-packed.c
+++ b/builtin/prune-packed.c
@@ -37,8 +37,8 @@ static int prune_object(const struct object_id *oid, const char *path,
 void prune_packed_objects(int opts)
 {
 	if (opts & PRUNE_PACKED_VERBOSE)
-		progress = start_progress_delay(_("Removing duplicate objects"),
-			256, 95, 2);
+		progress = start_delayed_progress(_("Removing duplicate objects"),
+						  256, 95);
 
 	for_each_loose_file_in_objdir(get_object_directory(),
 				      prune_object, NULL, prune_subdir, &opts);
diff --git a/builtin/prune.c b/builtin/prune.c
index c378690545..f32adaa0e9 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -138,7 +138,7 @@ int cmd_prune(int argc, const char **argv, const char *prefix)
 	if (show_progress == -1)
 		show_progress = isatty(2);
 	if (show_progress)
-		progress = start_progress_delay(_("Checking connectivity"), 0, 0, 2);
+		progress = start_delayed_progress(_("Checking connectivity"), 0, 0);
 
 	mark_reachable_objects(&revs, 1, expire, progress);
 	stop_progress(&progress);
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 95d84d5cda..5cfc4b35f4 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -364,7 +364,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 		revs.limited = 1;
 
 	if (show_progress)
-		progress = start_progress_delay(show_progress, 0, 0, 2);
+		progress = start_delayed_progress(show_progress, 0, 0);
 
 	if (use_bitmap_index && !revs.prune) {
 		if (revs.count && !revs.left_right && !revs.cherry_mark) {
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 786f389498..0eafa43618 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -532,9 +532,9 @@ void diffcore_rename(struct diff_options *options)
 	}
 
 	if (options->show_rename_progress) {
-		progress = start_progress_delay(
+		progress = start_delayed_progress(
 				_("Performing inexact rename detection"),
-				rename_dst_nr * rename_src_nr, 50, 1);
+				rename_dst_nr * rename_src_nr, 50);
 	}
 
 	mx = xcalloc(st_mult(NUM_CANDIDATE_PER_DST, num_create), sizeof(*mx));
diff --git a/progress.c b/progress.c
index 73e36d4a42..eeebe15b6f 100644
--- a/progress.c
+++ b/progress.c
@@ -205,8 +205,8 @@ int display_progress(struct progress *progress, unsigned n)
 	return progress ? display(progress, n, NULL) : 0;
 }
 
-struct progress *start_progress_delay(const char *title, unsigned total,
-				       unsigned percent_treshold, unsigned delay)
+static struct progress *start_progress_delay(const char *title, unsigned total,
+					     unsigned percent_treshold, unsigned delay)
 {
 	struct progress *progress = malloc(sizeof(*progress));
 	if (!progress) {
@@ -227,6 +227,12 @@ struct progress *start_progress_delay(const char *title, unsigned total,
 	return progress;
 }
 
+struct progress *start_delayed_progress(const char *title,
+					unsigned total,	unsigned percent_treshold)
+{
+	return start_progress_delay(title, total, percent_treshold, 2);
+}
+
 struct progress *start_progress(const char *title, unsigned total)
 {
 	return start_progress_delay(title, total, 0, 0);
diff --git a/progress.h b/progress.h
index 611e4c4d42..c7fc431989 100644
--- a/progress.h
+++ b/progress.h
@@ -6,8 +6,8 @@ struct progress;
 void display_throughput(struct progress *progress, off_t total);
 int display_progress(struct progress *progress, unsigned n);
 struct progress *start_progress(const char *title, unsigned total);
-struct progress *start_progress_delay(const char *title, unsigned total,
-				       unsigned percent_treshold, unsigned delay);
+struct progress *start_delayed_progress(const char *title, unsigned total,
+					unsigned percent_treshold);
 void stop_progress(struct progress **progress);
 void stop_progress_msg(struct progress **progress, const char *msg);
 
diff --git a/unpack-trees.c b/unpack-trees.c
index dd535bc849..03d1b37578 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -343,8 +343,7 @@ static struct progress *get_progress(struct unpack_trees_options *o)
 			total++;
 	}
 
-	return start_progress_delay(_("Checking out files"),
-				    total, 50, 1);
+	return start_delayed_progress(_("Checking out files"), total, 50);
 }
 
 static int check_updates(struct unpack_trees_options *o)