Re: [PATCH 18/29] blame: move progess updates to a scoreboard callback

[Junio C Hamano]

Jeff Smith <whydo...@gmail.com> writes:

> Subject: Re: [PATCH 18/29] blame: move progess updates to a scoreboard 
> callback

s/progess/progress/ (I can do this on my end).

> Allow the interface user to decide how to handle a progress update.
>
> Signed-off-by: Jeff Smith <whydo...@gmail.com>
> ---
>  builtin/blame.c | 27 +--
>  1 file changed, 17 insertions(+), 10 deletions(-)
>
> -static void found_guilty_entry(struct blame_entry *ent,
> -struct progress_info *pi)
> +static void found_guilty_entry(struct blame_entry *ent, void *data)
>  {
> + struct progress_info *pi = (struct progress_info *)data;
> +
>   if (incremental) {
>   struct blame_origin *suspect = ent->suspect;
>  

This hunk is interesting.  The function not only does the "progress"
eye candy, but it actually handles the "--incremental" option.
Anybody who wants to do something similar using the libified blame
needs to implement it in their found_guilty callback like this one
does, which is probably a good division of labor between the blame
lib and the front-end (which builtin/blame.c is one instance of).

> @@ -1754,11 +1758,6 @@ static void assign_blame(struct blame_scoreboard *sb, 
> int opt)
>  {
>   struct rev_info *revs = sb->revs;
>   struct commit *commit = prio_queue_get(>commits);
> - struct progress_info pi = { NULL, 0 };
> -
> - if (show_progress)
> - pi.progress = start_progress_delay(_("Blaming lines"),
> -sb->num_lines, 50, 1);

And this piece (and matching "stop progress" at the end of the
function) is now the responsibility of the caller, which makes
sense.

>  
> + sb.found_guilty_entry = _guilty_entry;
> + sb.found_guilty_entry_data = 
> + if (show_progress)
> + pi.progress = start_progress_delay(_("Blaming lines"),
> +sb.num_lines, 50, 1);
> +
>   assign_blame(, opt);
>  
> + stop_progress();
> +
>   if (!incremental)
>   setup_pager();


Very nicely done so far.

[PATCH 18/29] blame: move progess updates to a scoreboard callback

[Jeff Smith]

Allow the interface user to decide how to handle a progress update.

Signed-off-by: Jeff Smith 
---
 builtin/blame.c | 27 +--
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 1b53325..d05907b 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -391,6 +391,9 @@ struct blame_scoreboard {
 
/* callbacks */
void(*on_sanity_fail)(struct blame_scoreboard *, int);
+   void(*found_guilty_entry)(struct blame_entry *, void *);
+
+   void *found_guilty_entry_data;
 };
 
 static void sanity_check_refcnt(struct blame_scoreboard *);
@@ -1729,9 +1732,10 @@ static int emit_one_suspect_detail(struct blame_origin 
*suspect, int repeat)
  * The blame_entry is found to be guilty for the range.
  * Show it in incremental output.
  */
-static void found_guilty_entry(struct blame_entry *ent,
-  struct progress_info *pi)
+static void found_guilty_entry(struct blame_entry *ent, void *data)
 {
+   struct progress_info *pi = (struct progress_info *)data;
+
if (incremental) {
struct blame_origin *suspect = ent->suspect;
 
@@ -1754,11 +1758,6 @@ static void assign_blame(struct blame_scoreboard *sb, 
int opt)
 {
struct rev_info *revs = sb->revs;
struct commit *commit = prio_queue_get(>commits);
-   struct progress_info pi = { NULL, 0 };
-
-   if (show_progress)
-   pi.progress = start_progress_delay(_("Blaming lines"),
-  sb->num_lines, 50, 1);
 
while (commit) {
struct blame_entry *ent;
@@ -1800,7 +1799,8 @@ static void assign_blame(struct blame_scoreboard *sb, int 
opt)
suspect->guilty = 1;
for (;;) {
struct blame_entry *next = ent->next;
-   found_guilty_entry(ent, );
+   if (sb->found_guilty_entry)
+   sb->found_guilty_entry(ent, 
sb->found_guilty_entry_data);
if (next) {
ent = next;
continue;
@@ -1816,8 +1816,6 @@ static void assign_blame(struct blame_scoreboard *sb, int 
opt)
if (sb->debug) /* sanity */
sanity_check_refcnt(sb);
}
-
-   stop_progress();
 }
 
 static const char *format_time(timestamp_t time, const char *tz_str,
@@ -2550,6 +2548,7 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
char *final_commit_name = NULL;
enum object_type type;
struct commit *final_commit = NULL;
+   struct progress_info pi = { NULL, 0 };
 
struct string_list range_list = STRING_LIST_INIT_NODUP;
int output_option = 0, opt = 0;
@@ -2905,8 +2904,16 @@ int cmd_blame(int argc, const char **argv, const char 
*prefix)
 
read_mailmap(, NULL);
 
+   sb.found_guilty_entry = _guilty_entry;
+   sb.found_guilty_entry_data = 
+   if (show_progress)
+   pi.progress = start_progress_delay(_("Blaming lines"),
+  sb.num_lines, 50, 1);
+
assign_blame(, opt);
 
+   stop_progress();
+
if (!incremental)
setup_pager();
 
-- 
2.9.3