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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 25, 2025, 17:00 UTC
Message-ID: <xmqq349fs5ee.fsf@gitster.g>
In-Reply-To: <2d56de10-f829-4bc8-9c76-76eab6b137ae@kdbg.org>

Johannes Sixt <j6t@kdbg.org> writes:

Show 10 quoted lines

> 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.