On 05/10/14 at 04:48pm, Guillaume Bouchard wrote:
> When verbose package is activated, the table is displayed two times,
> once for explicit packages and a second time for dependencies.
>
> This helps understanding the upgrade process. You can focus on on
> explicit package and gives less attention to dependencies.
>
> Design choices:
>
> -- If a table does not fit in terminal width, it fallbacks to traditional
> compact display, but this fallback can happen on none, one or both
> tables
>
> -- The header name stay short, "Package" and "Dependancies". This is to
> avoid too long column name, but i think it is enough explicit as is.
> ---
> src/pacman/util.c | 59
> +++++++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 46 insertions(+), 13 deletions(-)
I'm not a fan of having two independent tables, especially when one
might be printed as a table and the other a list, but I do like the
idea of making it easy to pick out the packages I actually care about.
The patch needs a few fixes, but I'll give the overall change a +1
unless somebody has a better way to provide the information.
> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index d42e27b..864b7a3 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -774,12 +774,12 @@ 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(size_t count)
> +static alpm_list_t *create_verbose_header(size_t count, int is_explicit)
> {
> alpm_list_t *ret = NULL;
>
> char *header;
> - pm_asprintf(&header, "%s (%zd)", _("Package"), count);
> + pm_asprintf(&header, "%s (%zd)", (is_explicit ? _("Package") :
> _("Dependency")), count);
"Package" and "Dependency" are a little ambiguous. Dependency could
refer to either packages installed as dependencies or packages
currently required by other packages. If we can't find a clearer way
to describe what each list is, I think a note should be added to the
pacman.conf man page to clarify.
> add_table_cell(&ret, header, CELL_TITLE | CELL_FREE);
> add_table_cell(&ret, _("Old Version"), CELL_TITLE);
> @@ -846,7 +846,11 @@ 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, *names = NULL, *header = NULL, *rows = NULL;
> + alpm_list_t *i,
> + *names = NULL, *names_explicit = NULL,
> *names_deps = NULL,
> + *header_explicit = NULL, *header_deps = NULL,
> + *rows = NULL, *explicit = NULL, *deps = NULL;
> + size_t count_explicit = 0, count_deps = 0;
>
> if(!targets) {
> return;
> @@ -870,10 +874,6 @@ 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;
>
> - 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));
> @@ -885,27 +885,60 @@ static void _display_targets(alpm_list_t *targets, int
> verbose)
> alpm_pkg_get_version(target->remove),
> _("removal"));
> }
> names = alpm_list_add(names, str);
> +
> + if(verbose) {
> + // Check if package is installed as a explicit or as a
> dependency
> + // and store it in associated lists
> + if(alpm_pkg_get_reason(target->remove ? target->remove
> : target->install) == ALPM_PKG_REASON_DEPEND) {
> + deps = alpm_list_add(deps,
> create_verbose_row(target));
> + names_explicit =
> alpm_list_add(names_explicit, str);
> + count_deps += 1;
> + } else {
> + explicit = alpm_list_add(explicit,
> create_verbose_row(target));
> + names_deps = alpm_list_add(names_deps,
> str);
> + count_explicit += 1;
> + }
> + }
> +
> }
>
> /* print to screen */
> - pm_asprintf(&str, "%s (%zd)", _("Packages"), alpm_list_count(targets));
> printf("\n");
>
> cols = getcols(fileno(stdout));
> if(verbose) {
> - header = create_verbose_header(alpm_list_count(targets));
> - if(table_display(header, rows, cols) != 0) {
> + header_explicit = create_verbose_header(count_explicit, 1);
> + if(table_display(header_explicit, explicit, cols) != 0) {
> + /* fallback to list display if table wouldn't fit */
> + pm_asprintf(&str, "%s (%zd)", _("Packages"),
> count_explicit);
> + list_display(str, names_explicit, cols);
> + free(str);
> + }
> + printf("\n");
The trailing newline for each table should only be printed if a table
or list is printed, otherwise you end up with extra newlines when
a section is empty.
> + header_deps = create_verbose_header(count_deps, 0);
> + if(table_display(header_deps, deps, cols) != 0) {
> /* fallback to list display if table wouldn't fit */
> - list_display(str, names, cols);
> + pm_asprintf(&str, "%s (%zd)", _("Dependencies"),
> count_deps);
> + list_display(str, names_deps, cols);
> + free(str);
> }
> +
> } else {
> + pm_asprintf(&str, "%s (%zd)", _("Packages"),
> alpm_list_count(targets));
> list_display(str, names, cols);
> + free(str);
> }
> printf("\n");
>
> - table_free(header, rows);
> + table_free(header_explicit, explicit);
> + table_free(header_deps, deps);
> +
> FREELIST(names);
> - free(str);
> + /* The content of names_explicit and names_deps has allready been
> free'ed
> + * FREELIST(names), so we only free the containers */
> + free(names_explicit);
> + free(names_deps);
These need to be alpm_list_free.
> +
> rows = NULL;
>
> if(dlsize > 0 || config->op_s_downloadonly) {
> --
> 1.9.2
