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

28 Feb 2011

On 02/25/2011 03:57 PM, Dan McGee wrote:
...
On Mon, Feb 21, 2011 at 1:02 PM, Jakob Gruber  wrote:
...
Row handling is moved to its own function in preparation for verbose
package lists.
Signed-off-by: Jakob Gruber
---
  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

    