Re: [PATCH] progress: pay attention to (customized) delay time

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

Johannes Sixt <j6t@kdbg.org> writes:

> Subject: [PATCH] progress: pay attention to (customized) delay time
> ...
> until zero is reached. Due to the frequency of the calls, this happens
> without an observable delay in practice, so that the effective delay is
> always just one second.
> ...
> Since we have not had any complaints that the delay of one second is
> too short nor that GIT_PROGRESS_DELAY is ignored, people seem to be
> comfortable with the status quo. Therefore, set the default to 1 to
> keep the current behavior.

OK.  This is documenting the established behaviour, which makes
sense.

>  	struct strbuf *counters_sb = &progress->counters_sb;
>  	int show_update = 0;
> +	sig_atomic_t update = progress_update;

It is somewhat misleading to use sig_atomic_t for "update", which is
never updated via the signal handler.  It confused me a bit during
my initial reading.  If it were

	int update = !!progress_update;

it would have made it more obvious what is going on, at least to me.
In any case, I think it is an excellent idea to clear the global one
first ...

>  	int last_count_len = counters_sb->len;
>  
> -	if (progress->delay && (!progress_update || --progress->delay))
> +	progress_update = 0;

..., while remembering the fact that progress_update was originally
set or unset, and consistently use the latter in the remainder of
the function, like ...

> +	if (progress->delay && (!update || --progress->delay))
>  		return;

... this one and everywhere below (omitted from quote).

Thanks.