Improve VerbosePkgList by caching attributes then and cell's length with
the cell's label instead of recalculating.

Right align every cell that containing a file size, not just the last
collumn.

Simplify printf statements and the alignment application.
---
 src/pacman/util.c | 237 ++++++++++++++++++++++++++++--------------------------
 1 file changed, 123 insertions(+), 114 deletions(-)

diff --git a/src/pacman/util.c b/src/pacman/util.c
index bd09adc..acbf217 100644
--- a/src/pacman/util.c
+++ b/src/pacman/util.c
@@ -47,9 +47,17 @@
 #include "callback.h"
 
 
-struct table_row_t {
-       const char *label;
-       off_t size;
+struct table_cell_t {
+       char *label;
+       size_t len;
+       int mode;
+};
+
+enum {
+       CELL_NORMAL      = 0,
+       CELL_TITLE       = 1,
+       CELL_RIGHT_ALIGN = 1 << 2,
+       CELL_FREE        = 1 << 3
 };
 
 int trans_init(alpm_transflag_t flags, int check_valid)
@@ -424,6 +432,60 @@ static size_t string_length(const char *s)
        return len;
 }
 
+static void add_table_cell(alpm_list_t **row, char *label, int mode)
+{
+       struct table_cell_t *cell = malloc(sizeof(struct table_cell_t));
+
+       cell->label = label;
+       cell->mode = mode;
+       cell->len = string_length(label);
+
+       *row = alpm_list_add(*row, cell);
+}
+
+static void table_free_cell(void *ptr)
+{
+       struct table_cell_t *cell = ptr;
+
+       if(cell) {
+               if((cell->mode & CELL_FREE) && cell->label) {
+                       free(cell->label);
+               }
+               free(cell);
+       }
+}
+
+static void table_free(alpm_list_t *headers, alpm_list_t *rows)
+{
+       alpm_list_t *i;
+
+       alpm_list_free_inner(headers, table_free_cell);
+
+       for(i = rows; i; i = alpm_list_next(i)) {
+               if(i->data) {
+                       alpm_list_free_inner(i->data, table_free_cell);
+                       alpm_list_free(i->data);
+               }
+       }
+
+       alpm_list_free(headers);
+       alpm_list_free(rows);
+}
+
+static void add_transaction_sizes_row(alpm_list_t **table, char *label, int 
size)
+{
+       alpm_list_t *row = NULL;
+       char *str;
+       const char *units;
+       double s = humanize_size(size, 'M', 2, &units);
+       pm_asprintf(&str, "%.2f %s", s, units);
+
+       add_table_cell(&row, label, CELL_TITLE);
+       add_table_cell(&row, str, CELL_RIGHT_ALIGN | CELL_FREE);
+
+       *table = alpm_list_add(*table, row);
+}
+
 void string_display(const char *title, const char *string, unsigned short cols)
 {
        if(title) {
@@ -442,43 +504,30 @@ void string_display(const char *title, const char 
*string, unsigned short cols)
 static void table_print_line(const alpm_list_t *line, short col_padding,
                size_t colcount, size_t *widths, int *has_data)
 {
-       size_t i, lastcol = 0;
+       size_t i;
        int need_padding = 0;
        const alpm_list_t *curcell;
 
-       for(i = colcount; i > 0; i--) {
-               if(has_data[i - 1]) {
-                       lastcol = i - 1;
-                       break;
-               }
-       }
-
        for(i = 0, curcell = line; curcell && i < colcount;
                        i++, curcell = alpm_list_next(curcell)) {
-               const char *value;
+               const struct table_cell_t *cell = curcell->data;
+               const char *str = (cell->label ? cell->label : "");
                int cell_padding;
 
                if(!has_data[i]) {
                        continue;
                }
 
-               value = curcell->data;
-               if(!value) {
-                       value = "";
-               }
-               /* silly printf requires padding size to be an int */
-               cell_padding = (int)widths[i] - (int)string_length(value);
-               if(cell_padding < 0) {
-                       cell_padding = 0;
-               }
+               cell_padding = (cell->mode & CELL_RIGHT_ALIGN ? (int)widths[i] 
: -(int)widths[i]);
+
                if(need_padding) {
                        printf("%*s", col_padding, "");
                }
-               /* left-align all but the last column */
-               if(i != lastcol) {
-                       printf("%s%*s", value, cell_padding, "");
+
+               if(cell->mode & CELL_TITLE) {
+                       printf("%s%*s%s", config->colstr.title, cell_padding, 
str, config->colstr.nocolor);
                } else {
-                       printf("%*s%s", cell_padding, "", value);
+                       printf("%*s", cell_padding, str);
                }
                need_padding = 1;
        }
@@ -487,7 +536,6 @@ static void table_print_line(const alpm_list_t *line, short 
col_padding,
 }
 
 
-
 /**
  * Find the max string width of each column. Also determines whether values
  * exist in the column and sets the value in has_data accordingly.
@@ -522,7 +570,8 @@ static size_t table_calc_widths(const alpm_list_t *header,
        }
        /* header determines column count and initial values of longest_strs */
        for(i = header, curcol = 0; i; i = alpm_list_next(i), curcol++) {
-               colwidths[curcol] = string_length(i->data);
+               const struct table_cell_t *row = i->data;
+               colwidths[curcol] = row->len;
                /* note: header does not determine whether column has data */
        }
 
@@ -531,8 +580,8 @@ static size_t table_calc_widths(const alpm_list_t *header,
                /* grab first column of each row and iterate through columns */
                const alpm_list_t *j = i->data;
                for(curcol = 0; j; j = alpm_list_next(j), curcol++) {
-                       const char *str = j->data;
-                       size_t str_len = string_length(str);
+                       const struct table_cell_t *row = j->data;
+                       size_t str_len = row ? row->len : 0;
 
                        if(str_len > colwidths[curcol]) {
                                colwidths[curcol] = str_len;
@@ -571,20 +620,24 @@ static size_t table_calc_widths(const alpm_list_t *header,
  * @param cols the number of columns available in the terminal
  * @return -1 if not enough terminal cols available, else 0
  */
-static int table_display(const char *title, const alpm_list_t *header,
+static int table_display(const alpm_list_t *header,
                const alpm_list_t *rows, unsigned short cols)
 {
        const unsigned short padding = 2;
-       const alpm_list_t *i;
+       const alpm_list_t *i, *first;
        size_t *widths = NULL, totalcols, totalwidth;
        int *has_data = NULL;
 
-       if(rows == NULL || header == NULL) {
+       if(rows == NULL) {
                return 0;
        }
 
-       totalcols = alpm_list_count(header);
-       totalwidth = table_calc_widths(header, rows, padding, totalcols,
+       /* we want the first row. if no headers are provided, use the first
+        * entry of the rows array. */
+       first = header ? header : rows->data;
+
+       totalcols = alpm_list_count(first);
+       totalwidth = table_calc_widths(first, rows, padding, totalcols,
                        &widths, &has_data);
        /* return -1 if terminal is not wide enough */
        if(totalwidth > cols) {
@@ -596,13 +649,11 @@ static int table_display(const char *title, const 
alpm_list_t *header,
                return -1;
        }
 
-       if(title != NULL) {
-               printf("%s\n\n", title);
+       if (header) {
+               table_print_line(header, padding, totalcols, widths, has_data);
+               printf("\n");
        }
 
-       table_print_line(header, padding, totalcols, widths, has_data);
-       printf("\n");
-
        for(i = rows; i; i = alpm_list_next(i)) {
                table_print_line(i->data, padding, totalcols, widths, has_data);
        }
@@ -759,23 +810,20 @@ void signature_display(const char *title, alpm_siglist_t 
*siglist,
 }
 
 /* creates a header row for use with table_display */
-static alpm_list_t *create_verbose_header(void)
+static alpm_list_t *create_verbose_header(size_t count)
 {
-       alpm_list_t *res = NULL;
-       char *str;
+       alpm_list_t *ret = NULL;
+
+       char *header;
+       pm_asprintf(&header, "%s (%zd)", _("Package"), count);
+
+       add_table_cell(&ret, header, CELL_TITLE | CELL_FREE);
+       add_table_cell(&ret, _("Old Version"), CELL_TITLE);
+       add_table_cell(&ret, _("New Version"), CELL_TITLE);
+       add_table_cell(&ret, _("Net Change"), CELL_TITLE);
+       add_table_cell(&ret, _("Download Size"), CELL_TITLE);
 
-       str = _("Name");
-       res = alpm_list_add(res, str);
-       str = _("Old Version");
-       res = alpm_list_add(res, str);
-       str = _("New Version");
-       res = alpm_list_add(res, str);
-       str = _("Net Change");
-       res = alpm_list_add(res, str);
-       str = _("Download Size");
-       res = alpm_list_add(res, str);
-
-       return res;
+       return ret;
 }
 
 /* returns package info as list of strings */
@@ -798,23 +846,23 @@ static alpm_list_t *create_verbose_row(pm_target_t 
*target)
        } else {
                pm_asprintf(&str, "%s", alpm_pkg_get_name(target->remove));
        }
-       ret = alpm_list_add(ret, str);
+       add_table_cell(&ret, str, CELL_NORMAL);
 
        /* old and new versions */
        pm_asprintf(&str, "%s",
                        target->remove != NULL ? 
alpm_pkg_get_version(target->remove) : "");
-       ret = alpm_list_add(ret, str);
+       add_table_cell(&ret, str, CELL_NORMAL);
 
        pm_asprintf(&str, "%s",
                        target->install != NULL ? 
alpm_pkg_get_version(target->install) : "");
-       ret = alpm_list_add(ret, str);
+       add_table_cell(&ret, str, CELL_NORMAL);
 
        /* and size */
        size -= target->remove ? alpm_pkg_get_isize(target->remove) : 0;
        size += target->install ? alpm_pkg_get_isize(target->install) : 0;
        human_size = humanize_size(size, 'M', 2, &label);
        pm_asprintf(&str, "%.2f %s", human_size, label);
-       ret = alpm_list_add(ret, str);
+       add_table_cell(&ret, str, CELL_RIGHT_ALIGN);
 
        size = target->install ? alpm_pkg_download_size(target->install) : 0;
        if(size != 0) {
@@ -823,55 +871,18 @@ static alpm_list_t *create_verbose_row(pm_target_t 
*target)
        } else {
                str = NULL;
        }
-       ret = alpm_list_add(ret, str);
+       add_table_cell(&ret, str, CELL_RIGHT_ALIGN);
 
        return ret;
 }
 
-static void add_transaction_sizes_row(alpm_list_t **table, const char *label, 
off_t size)
-{
-       struct table_row_t *row = malloc(sizeof(struct table_row_t));
-
-       row->label = label;
-       row->size = size;
-
-       *table = alpm_list_add(*table, row);
-}
-
-static void display_transaction_sizes(alpm_list_t *table)
-{
-       alpm_list_t *i;
-       int max_len = 0;
-
-       for(i = table; i; i = alpm_list_next(i)) {
-               struct table_row_t *row = i->data;
-               int len = string_length(row->label);
-
-               if(len > max_len) {
-                       max_len = len;
-               }
-       }
-
-       max_len += 2;
-
-       for(i = table; i; i = alpm_list_next(i)) {
-               struct table_row_t *row = i->data;
-               const char *units;
-               const colstr_t *colstr = &config->colstr;
-               double s = humanize_size(row->size, 'M', 2, &units);
-
-               printf("%s%-*s%s %.2f %s\n", colstr->title, max_len, row->label,
-                               colstr->nocolor, s, units);
-       }
-}
-
 /* prepare a list of pkgs to display */
 static void _display_targets(alpm_list_t *targets, int verbose)
 {
        char *str;
        off_t isize = 0, rsize = 0, dlsize = 0;
        unsigned short cols;
-       alpm_list_t *i, *rows = NULL, *names = NULL, *table = NULL;
+       alpm_list_t *i, *names = NULL, *header = NULL, *rows = NULL;
 
        if(!targets) {
                return;
@@ -895,7 +906,10 @@ static void _display_targets(alpm_list_t *targets, int 
verbose)
        for(i = targets; i; i = alpm_list_next(i)) {
                pm_target_t *target = i->data;
 
-               rows = alpm_list_add(rows, create_verbose_row(target));
+               if(verbose) {
+                       rows = alpm_list_add(rows, create_verbose_row(target));
+               }
+
                if(target->install) {
                        pm_asprintf(&str, "%s-%s", 
alpm_pkg_get_name(target->install),
                                        alpm_pkg_get_version(target->install));
@@ -910,48 +924,43 @@ static void _display_targets(alpm_list_t *targets, int 
verbose)
        }
 
        /* print to screen */
-       pm_asprintf(&str, "%s (%zd):", _("Packages"), alpm_list_count(targets));
+       pm_asprintf(&str, "%s (%zd)", _("Packages"), alpm_list_count(targets));
        printf("\n");
 
        cols = getcols(fileno(stdout));
        if(verbose) {
-               alpm_list_t *header = create_verbose_header();
-               if(table_display(str, header, rows, cols) != 0) {
+               header = create_verbose_header(alpm_list_count(targets));
+               if(table_display(header, rows, cols) != 0) {
                        /* fallback to list display if table wouldn't fit */
                        list_display(str, names, cols);
                }
-               alpm_list_free(header);
        } else {
                list_display(str, names, cols);
        }
        printf("\n");
 
-       /* rows is a list of lists of strings, free inner lists here */
-       for(i = rows; i; i = alpm_list_next(i)) {
-               alpm_list_t *lp = i->data;
-               FREELIST(lp);
-       }
-       alpm_list_free(rows);
+       table_free(header, rows);
        FREELIST(names);
        free(str);
+       rows = NULL;
 
        if(dlsize > 0 || config->op_s_downloadonly) {
-               add_transaction_sizes_row(&table, _("Total Download Size:"), 
dlsize);
+               add_transaction_sizes_row(&rows, _("Total Download Size:"), 
dlsize);
        }
        if(!config->op_s_downloadonly) {
                if(isize > 0) {
-                       add_transaction_sizes_row(&table, _("Total Installed 
Size:"), isize);
+                       add_transaction_sizes_row(&rows, _("Total Installed 
Size:"), isize);
                }
                if(rsize > 0 && isize == 0) {
-                       add_transaction_sizes_row(&table, _("Total Removed 
Size:"), rsize);
+                       add_transaction_sizes_row(&rows, _("Total Removed 
Size:"), rsize);
                }
                /* only show this net value if different from raw installed 
size */
                if(isize > 0 && rsize > 0) {
-                       add_transaction_sizes_row(&table, _("Net Upgrade 
Size:"), isize - rsize);
+                       add_transaction_sizes_row(&rows, _("Net Upgrade 
Size:"), isize - rsize);
                }
        }
-       display_transaction_sizes(table);
-       FREELIST(table);
+       table_display(NULL, rows, cols);
+       table_free(NULL, rows);
 }
 
 static int target_cmp(const void *p1, const void *p2)
-- 
1.8.3


Reply via email to