From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Thu, 19 Apr 2007 09:03:06 GMT
Subject: Re: [PATCH] common progress display support
Message-ID: <Pine.LNX.4.64.0704191053590.8822@racer.site>
In-Reply-To: <alpine.LFD.0.98.0704181422050.4504@xanadu.home>

Hi,

On Wed, 18 Apr 2007, Nicolas Pitre wrote:

> Instead of having this code duplicated in multiple places, let's have a 
> common interface for progress display.

I like that!

> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index 19fae4c..150f56c 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> @@ -588,8 +588,10 @@ static off_t write_pack_file(void)
>  		f = sha1fd(fd, pack_tmp_name);
>  	}
>  
> -	if (do_progress)
> +	if (do_progress) {
>  		fprintf(stderr, "Writing %u objects.\n", nr_result);
> +		start_progress(&progress_state, "", nr_result);
> +	}

It seems like it would be a good idea to change the signature of 
start_progress() to take an optional message, like this:

	start_progress(&progress_state, "Writing %u object.s\n", 
		"", nr_result);

It would make the added/deleted ratio even more impressive.

I only skimmed the patch, but like all of your patches, it looks good to 
me.

Ciao,
Dscho