Jeff Smith <whydoubt@xxxxxxxxx> 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 <whydoubt@xxxxxxxxx>
> ---
> 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 = π
> + 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.