[pacman-dev] [PATCH 04/16] remove format from statistic messages

[Allan McRae]

On 02/03/13 07:32, Simon Gomizelj wrote:
> Remove the format component of the "Total Download Size" and related
> messages. The heading will be colourized, the size won't.
> 
> However since the length of these messages can vary by language, we need
> a pretty printer to format them nicely.
> 
> Signed-off-by: Simon Gomizelj <simongmzlj at gmail.com>
> ---
>  src/pacman/util.c | 51 +++++++++++++++++++++++++++++++++++++++++----------
>  1 file changed, 41 insertions(+), 10 deletions(-)

I'm going to ask for quite some changes here, but they should all be easy...

> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index 29b9f38..45ae983 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -47,6 +47,11 @@
>  #include "callback.h"
>  
>  
> +struct table_row_t {
> +	const char *label;
> +	int size;
> +};
> +
>  int trans_init(alpm_transflag_t flags, int check_valid)
>  {
>  	int ret;
> @@ -824,15 +829,35 @@ static alpm_list_t *create_verbose_row(pm_target_t *target)
>  	return ret;
>  }
>  
> +static void _print_table(struct table_row_t *rows, int spacing)

Kill the underscore at the start of the name.  We don't do that... (see
below).  Also, lets just give this a descriptive name.
display_transaction_sizes().

Why do we need the space parameter?  This function is used in a single
place...  Delete it.

> +{
> +	struct table_row_t *row;
> +	int max_len = 0;
> +
> +	/* max length line */
> +	for(row = rows; row && row->label; ++row) {
> +		int len = string_length(row->label);
> +		if(len > max_len)
> +			max_len = len;
> +	}
> +
> +	max_len += spacing;
> +
> +	for(row = rows; row && row->label; ++row) {
> +		const char *units;
> +		double s = humanize_size(row->size, 'M', 2, &units);
> +		printf("%-*s %.2f %s\n", max_len, row->label, s, units);
> +	}
> +}
> +
>  /* prepare a list of pkgs to display */
>  static void _display_targets(alpm_list_t *targets, int verbose)

(not for this patch) This underscore needs to die too... but we already
have display_targets.  Descriptive name needed...  Sigh!

>  {
>  	char *str;
> -	const char *label;
> -	double size;
>  	off_t isize = 0, rsize = 0, dlsize = 0;
>  	unsigned short cols;
>  	alpm_list_t *i, *rows = NULL, *names = NULL;
> +	struct table_row_t tbl_rows[5], *row = tbl_rows;

Not a fan of this at all...  An array that is actually of variable
length and at most the fifth element is used to confirm there is no more
elements. Yuck!

Just use an alpm_list_t here.

>  	if(!targets) {
>  		return;
> @@ -897,24 +922,30 @@ static void _display_targets(alpm_list_t *targets, int verbose)
>  	free(str);
>  
>  	if(dlsize > 0 || config->op_s_downloadonly) {
> -		size = humanize_size(dlsize, 'M', 2, &label);
> -		printf(_("Total Download Size:    %.2f %s\n"), size, label);
> +		row->label = _("Total Download Size:");

Can we add the ":" at the end rather than have it "translated"?

> +		row->size = dlsize;
> +		++row;
>  	}
>  	if(!config->op_s_downloadonly) {
>  		if(isize > 0) {
> -			size = humanize_size(isize, 'M', 2, &label);
> -			printf(_("Total Installed Size:   %.2f %s\n"), size, label);
> +			row->label = _("Total Installed Size:");
> +			row->size = isize;
> +			++row;
>  		}
>  		if(rsize > 0 && isize == 0) {
> -			size = humanize_size(rsize, 'M', 2, &label);
> -			printf(_("Total Removed Size:     %.2f %s\n"), size, label);
> +			row->label = _("Total Removed Size:");
> +			row->size = rsize;
> +			++row;
>  		}
>  		/* only show this net value if different from raw installed size */
>  		if(isize > 0 && rsize > 0) {
> -			size = humanize_size(isize - rsize, 'M', 2, &label);
> -			printf(_("Net Upgrade Size:       %.2f %s\n"), size, label);
> +			row->label = _("Net Upgrade Size:");
> +			row->size = isize - rsize;
> +			++row;
>  		}
>  	}
> +	row->label = NULL;
> +	_print_table(tbl_rows, 2);
>  }
>  
>  static int target_cmp(const void *p1, const void *p2)
>