[pacman-dev] [PATCH 5/6] Table formatted output functions

[Jakob Gruber]

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
>>
>>
>>