[pacman-dev] [PATCH] Verbose package list now show if packages are explicit or dependencies
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(-) 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); 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"); + 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); + rows = NULL; if(dlsize > 0 || config->op_s_downloadonly) { -- 1.9.2
Hello,
I'm reopening this discussion. A few time ago, I proposed to "grey"
explicit package name. Finally it was judged as confusing.
Instead I propose to display only the package type during verbose
package listing. To achieve this, I splitted the packages table into
two tables, the one for explicit packages, and the other for
dependencies, examples:
$ pacman -S epiphany
[sudo] password for guillaume:
resolving dependencies...
looking for conflicting packages...
Package (1) New Version Net Change Download Size
extra/epiphany 3.12.0-1 6.89 MiB 1.48 MiB
Dependency (3) New Version Net Change Download Size
extra/gcr 3.12.0-1 5.42 MiB 0.77 MiB
extra/libwnck3 3.4.7-1 2.85 MiB
extra/libxres 1.0.7-1 0.02 MiB
Total Download Size: 2.25 MiB
Total Installed Size: 15.17 MiB
$ pacman -Rs gitg
checking dependencies...
Package (1) Old Version Net Change
gitg 0.3.2-1 -1.41 MiB
Dependency (8) Old Version Net Change
geoclue 0.12.99-1 -1.02 MiB
gtksourceview3 3.12.1-1 -6.78 MiB
...
Feedback needed ;)
--
G
On Sat, May 10, 2014 at 4:48 PM, Guillaume Bouchard
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(-)
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);
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"); + 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); + rows = NULL;
if(dlsize > 0 || config->op_s_downloadonly) { -- 1.9.2
On 11/05/14 00:54, Guillaume Bouchard wrote:
Hello,
I'm reopening this discussion. A few time ago, I proposed to "grey" explicit package name. Finally it was judged as confusing.
Instead I propose to display only the package type during verbose package listing. To achieve this, I splitted the packages table into two tables, the one for explicit packages, and the other for dependencies, examples:
$ pacman -S epiphany [sudo] password for guillaume: resolving dependencies... looking for conflicting packages...
Package (1) New Version Net Change Download Size
extra/epiphany 3.12.0-1 6.89 MiB 1.48 MiB
Dependency (3) New Version Net Change Download Size
extra/gcr 3.12.0-1 5.42 MiB 0.77 MiB extra/libwnck3 3.4.7-1 2.85 MiB extra/libxres 1.0.7-1 0.02 MiB
Total Download Size: 2.25 MiB Total Installed Size: 15.17 MiB
$ pacman -Rs gitg checking dependencies...
Package (1) Old Version Net Change
gitg 0.3.2-1 -1.41 MiB
Dependency (8) Old Version Net Change
geoclue 0.12.99-1 -1.02 MiB gtksourceview3 3.12.1-1 -6.78 MiB ...
Feedback needed ;)
Anybody want to comment on this? I do not use the verbose table so have no opinion on whether this information is useful and if it is clear. Allan
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
On 07/06/14 23:03, Andrew Gregory wrote:
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
None, or both. I don't like the half-way here.
-- 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.
I'm not a fan of two tables either. And there is constantly a request to highlight new dependencies so we need a third... I do not think there is a good answer to this problem. Allan
participants (3)
-
Allan McRae
-
Andrew Gregory
-
Guillaume Bouchard