Re: [PATCH v2 1/7] progress: add function to set total

From: Jeff King <peff@peff.net>
Date: Feb 21, 2025, 07:43 UTC
Message-ID: <20250221074308.GB1988395@coredump.intra.peff.net>
In-Reply-To: <20250219-toon-bundleuri-progress-v2-1-a84e7ffa921a@iotcl.com>

On Wed, Feb 19, 2025 at 03:30:19PM +0100, Toon Claes wrote:

Show 6 quoted lines

> We're about to add the use of progress through curl. Although, curl
> doesn't know the total at the start of the download, but might receive
> this information in the Content-Length header when the download starts.
> 
> To allow users set the total size after calling start_progress(), add a
> function progress_set_total().

Makes sense.

Show 5 quoted lines

> +void progress_set_total(struct progress *progress, uint64_t total)
> +{
> +	if (progress)
> +		progress->total = total;
> +}

I wondered if we'd need to do any other computation here (that would have been done in start_progress() if we specified the total then). But it looks like we don't look at progress->total until we're ready to display something.

Would we want to call display_progress() or similar here, to update the original view like:

  Working hard: 1<CR>

to:

  Working hard: 33% (1/3)<CR>

immediately, rather than waiting for more progress to be made? I guess it probably doesn't matter that much in practice as we'd remain stale for a brief period in most cases (particularly for transfers where we're updating based on bytes received, which is a pretty small unit of work).

Plus I think it may be awkward, because we don't know whether to call display_progress() or display_throughput().

-Peff