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(&sb->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 = &found_guilty_entry;
> +     sb.found_guilty_entry_data = &pi;
> +     if (show_progress)
> +             pi.progress = start_progress_delay(_("Blaming lines"),
> +                                                sb.num_lines, 50, 1);
> +
>       assign_blame(&sb, opt);
>  
> +     stop_progress(&pi.progress);
> +
>       if (!incremental)
>               setup_pager();


Very nicely done so far.