On 02/25/2011 04:21 PM, Dan McGee wrote:
I had a real hard time following all this, but that might just be the morning brain not working well. Your reuse of curformat->data as first a immutable string holder and later a alloced format string holder is particularly hard to wrap the brain around until you see it. I'd highly recommend not mixing the two for clarity.
I made a few changes that should hopefully make this part clearer. table_display now also takes a list of column headers, and table_create_format uses them to determine the column count before looking for the longest strings and creating the formats. This gets rid of adding the NULL in display_targets (NULL was equal to an empty line after the header) and the NULL in table_create_format (which was an initial value which meant "assign the first string we get to because there aren't any initial values to run strlen() on yet").
input: a list of rows, each with a list of cols (cells)? output: a list of formats for each col? Yeah, exactly.
+ + /* return NULL if terminal is not wide enough */ + if(neededcols> getcols()) { + FREELIST(format); + return NULL; + } Hmm. Is this likely to happen? Seems odd to then not have anything printed at all. I think we should truncate strings as necessary, just like we do in the callback functions.
It shouldn't happen (often) for regular usage. On an 80 column terminal, the current upgrade targets (which all have average length names and versions) takes up about 2/3 of the width. I added an error message at this spot, but I'm not sure about truncating strings? Currently, the error is passed up through table_display, display_targets will unset config->verbosepkglists and run itself again with the default list display as fallback. Is that behavior acceptable?
+ + return format; +} + +/** Displays the list in table format + * + * @param title the tables title + * @param rows the rows to display as a list of lists of strings. the outer + * list represents the rows, the inner list the cells (= columns) + * + * @return -1 if not enough terminal cols available, else 0 + */ +int table_display(const char *title, const alpm_list_t *rows) +{ + alpm_list_t *i, *formats; + + if(rows == NULL) + return 0; + + formats = table_create_format(rows); + if(formats == NULL) { + return -1; + } + + if(title != NULL) { + printf("%s\n\n", title); + } + + for(i = alpm_list_first(rows); i; i = alpm_list_next(i)) { + table_print_line(alpm_list_getdata(i), formats); + } + + FREELIST(formats); + return 0; +} + void list_display(const char *title, const alpm_list_t *list) { const alpm_list_t *i; diff --git a/src/pacman/util.h b/src/pacman/util.h index ed03c13..494becf 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -56,6 +56,7 @@ char *size_to_human_string_mb(off_t bytes); char *size_to_human_string_kb(off_t bytes); char *size_to_human_string_short(off_t bytes); void string_display(const char *title, const char *string); +int table_display(const char *title, const alpm_list_t *rows); void list_display(const char *title, const alpm_list_t *list); void list_display_linebreak(const char *title, const alpm_list_t *list); void display_targets(const alpm_list_t *pkgs, int install); -- 1.7.4.1