[pacman-dev] [PATCH 4/6] Refactor display_targets for readability

[Jakob Gruber]

On 02/25/2011 03:57 PM, Dan McGee wrote:
> On Mon, Feb 21, 2011 at 1:02 PM, Jakob Gruber<jakob.gruber at gmail.com>  wrote:
>> Row handling is moved to its own function in preparation for verbose
>> package lists.
>>
>> Signed-off-by: Jakob Gruber<jakob.gruber at gmail.com>
>> ---
>>   src/pacman/util.c |   53 ++++++++++++++++++++++++++++-------------------------
>>   1 files changed, 28 insertions(+), 25 deletions(-)
>>
>> diff --git a/src/pacman/util.c b/src/pacman/util.c
>> index 8903d61..85d2d8c 100644
>> --- a/src/pacman/util.c
>> +++ b/src/pacman/util.c
>> @@ -498,10 +498,26 @@ void list_display_linebreak(const char *title, const alpm_list_t *list)
>>                 }
>>         }
>>   }
>> +
>> +/* returns package info as a string */
>> +static char *create_list_element(pmpkg_t *pkg)
>> +{
>> +       char *ret, *size;
>> +       const char *pkgstr = config->showsize ? "%s-%s [%s]" : "%s-%s";
>> +
>> +       size = size_to_human_string_mb(alpm_pkg_get_size(pkg));
>> +       pm_asprintf(&ret, pkgstr, alpm_pkg_get_name(pkg), alpm_pkg_get_version(pkg),
>> +                       size);
>> +       free(size);
>> +
>> +       return ret;
> return()
Done, sorry bout that.

>> +}
>> +
>>   /* prepare a list of pkgs to display */
>>   void display_targets(const alpm_list_t *pkgs, int install)
>>   {
>>         char *str, *size;
>> +       const char *title;
>>         const alpm_list_t *i;
>>         off_t isize = 0, dlsize = 0;
>>         alpm_list_t *targets = NULL;
>> @@ -510,34 +526,24 @@ void display_targets(const alpm_list_t *pkgs, int install)
>>                 return;
>>         }
>>
>> -       printf("\n");
>> +       title = install ? _("Targets (%d):") : _("Remove (%d):");
>> +
>>         for(i = pkgs; i; i = alpm_list_next(i)) {
>>                 pmpkg_t *pkg = alpm_list_getdata(i);
>>
>> -               if(install) {
>> -                       dlsize += alpm_pkg_download_size(pkg);
>> -               }
>> +               dlsize += alpm_pkg_download_size(pkg);
> So we're now adding this up for both install and remove? We should
> probably keep it conditional.
>
Done.

>>                 isize += alpm_pkg_get_isize(pkg);
>>
>> -               /* print the package size with the output if ShowSize option set */
>> -               if(config->showsize) {
>> -                       size = size_to_human_string_mb(alpm_pkg_get_size(pkg));
>> -                       pm_asprintf(&str, "%s-%s [%s]", alpm_pkg_get_name(pkg),
>> -                                       alpm_pkg_get_version(pkg), size);
>> -                       free(size);
>> -               } else {
>> -                       pm_asprintf(&str, "%s-%s", alpm_pkg_get_name(pkg),
>> -                                       alpm_pkg_get_version(pkg));
>> -               }
>> -               targets = alpm_list_add(targets, str);
>> +               targets = alpm_list_add(targets, create_list_element(pkg));
>>         }
>>
>> -       if(install) {
>> -               pm_asprintf(&str, _("Targets (%d):"), alpm_list_count(targets));
>> -               list_display(str, targets);
>> -               free(str);
>> -               printf("\n");
>> +       pm_asprintf(&str, title, alpm_list_count(targets));
>> +
>> +       printf("\n");
>> +       list_display(str, targets);
>> +       printf("\n");
>>
>> +       if(install) {
>>                 size = size_to_human_string_mb(dlsize);
>>                 printf(_("Total Download Size:    %s\n"), size);
>>                 free(size);
>> @@ -547,16 +553,13 @@ void display_targets(const alpm_list_t *pkgs, int install)
>>                         free(size);
>>                 }
>>         } else {
>> -               pm_asprintf(&str, _("Remove (%d):"), alpm_list_count(targets));
>> -               list_display(str, targets);
>> -               free(str);
>> -               printf("\n");
>> -
>>                 size = size_to_human_string_mb(isize);
>>                 printf(_("Total Removed Size:   %s\n"), size);
>>                 free(size);
>>         }
>>
>> +out:
>> +       free(str);
>>         FREELIST(targets);
>>   }
>>
>> --
>> 1.7.4.1
>>
>>
>>