On 04/17/20 at 06:57pm, Carson Black wrote:
> This patch splits the monolithic 'Packages (count):' output on transactions
> into
> multiple package outputs per category of action: 'Installing (count):',
> 'Upgrading
> (count):', and 'Removing (count):'.
>
> Signed-off-by: Carson Black <uhh...@gmail.com>
> ---
> src/pacman/util.c | 79 ++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 65 insertions(+), 14 deletions(-)
>
> diff --git a/src/pacman/util.c b/src/pacman/util.c
> index 6693ec75..3bec6774 100644
> --- a/src/pacman/util.c
> +++ b/src/pacman/util.c
> @@ -898,7 +898,10 @@ 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;
> + char *install_header;
> + char *upgrade_header;
> + char *remove_header;
> + alpm_list_t *i, *install = NULL, *upgrade = NULL, *remove = NULL,
> *header = NULL, *rows = NULL;
>
> if(!targets) {
> return;
> @@ -918,6 +921,10 @@ static void _display_targets(alpm_list_t *targets, int
> verbose)
> }
> }
>
> + uint install_targets = 0;
> + uint upgrade_targets = 0;
> + uint remove_targets = 0;
No need for these, just use alpm_list_count as needed.
> /* form data for both verbose and non-verbose display */
> for(i = targets; i; i = alpm_list_next(i)) {
> pm_target_t *target = i->data;
> @@ -926,38 +933,82 @@ static void _display_targets(alpm_list_t *targets, int
> verbose)
> rows = alpm_list_add(rows, create_verbose_row(target));
> }
>
> - if(target->install) {
> + if(target->install && target->remove) {
> pm_asprintf(&str, "%s%s-%s%s",
> alpm_pkg_get_name(target->install), config->colstr.faint,
> - alpm_pkg_get_version(target->install),
> config->colstr.nocolor);
> - } else if(isize == 0) {
> - pm_asprintf(&str, "%s%s-%s%s",
> alpm_pkg_get_name(target->remove), config->colstr.faint,
> - alpm_pkg_get_version(target->remove),
> config->colstr.nocolor);
> +
> alpm_pkg_get_version(target->install), config->colstr.nocolor);
Remove the extra indentation.
> + } else if(target->install) {
> + pm_asprintf(&str, "%s%s-%s%s",
> alpm_pkg_get_name(target->install), config->colstr.faint,
> +
> alpm_pkg_get_version(target->install), config->colstr.nocolor);
> } else {
> - pm_asprintf(&str, "%s%s-%s %s[%s]%s",
> alpm_pkg_get_name(target->remove), config->colstr.faint,
> - alpm_pkg_get_version(target->remove),
> config->colstr.nocolor, _("removal"), config->colstr.nocolor);
> + pm_asprintf(&str, "%s%s-%s%s",
> alpm_pkg_get_name(target->remove), config->colstr.faint,
> +
> alpm_pkg_get_version(target->remove), config->colstr.nocolor);
> + }
> +
> + if (target->install && target->remove) {
> + upgrade = alpm_list_add(upgrade, str);
> + upgrade_targets++;
> + } else if (target->install) {
> + install = alpm_list_add(install, str);
> + install_targets++;
> + } else if (target->remove) {
> + remove = alpm_list_add(remove, str);
> + remove_targets++;
This just duplicates the conditional above, combine the two.
> }
> - names = alpm_list_add(names, str);
> }
>
> /* print to screen */
> - pm_asprintf(&str, "%s (%zu)", _("Packages"), alpm_list_count(targets));
> - printf("\n");
> + if (install_targets) {
> + pm_asprintf(&install_header, _("Installing (%u):"),
> install_targets);
> + }
> + if (remove_targets) {
> + pm_asprintf(&remove_header, _("Removing (%u):"),
> remove_targets);
> + }
> + if (upgrade_targets) {
> + pm_asprintf(&upgrade_header, _("Upgrading (%u):"),
> upgrade_targets);
> + }
I don't see any point in pre-generating all three headers, just use
str and generate and free them as they're used.
> cols = getcols();
> if(verbose) {
> header = create_verbose_header(alpm_list_count(targets));
> if(table_display(header, rows, cols) != 0) {
> /* fallback to list display if table wouldn't fit */
> - list_display(str, names, cols);
> + if (install_targets) {
> + list_display(install_header, install, cols);
> + }
> + if (upgrade_targets) {
> + list_display(upgrade_header, upgrade, cols);
> + }
> + if (remove_targets) {
> + list_display(remove_header, remove, cols);
> + }
This does not match the way packages are displayed without verbose.
> }
> } else {
> - list_display(str, names, cols);
> + if (install_targets) {
> + list_display(install_header, install, cols);
> + if (upgrade_targets || remove_targets) {
> + printf("\n");
> + }
> + }
> + if (upgrade_targets) {
> + list_display(upgrade_header, upgrade, cols);
> + if (remove_targets) {
> + printf("\n");
> + }
> + }
> + if (remove_targets) {
> + list_display(remove_header, remove, cols);
> + }
> }
> printf("\n");
>
> table_free(header, rows);
> - FREELIST(names);
> + if (install) alpm_list_free(install);
> + if (upgrade) alpm_list_free(upgrade);
> + if (remove) alpm_list_free(remove);
These should be FREELIST just like the line they're replacing.
> free(str);
> + if (install_header) free(install_header);
> + if (upgrade_header) free(upgrade_header);
> + if (remove_header) free(remove_header);
> rows = NULL;
>
> if(dlsize > 0 || config->op_s_downloadonly) {
> --
> 2.26.0
--
apg
