[pacman-dev] [PATCH 00/15] Colour support in pacman V2!
I believe I've addressed all the current concerns with the patchset. Submitting again for further review. https://github.com/vodik/pacman/ https://github.com/vodik/pacman/compare/cb2edc60cf...master Simon Gomizelj (15): refactor common code in query_search/sync_search remove :: prefix from all message split "Packages (%zd):" message remove format from statistic messages standardize format functions add a config settings and flag for colours introduce colstr for colourizing colourize colon_printf and question colourize warnings and errors colourize table output colourize the output if -Qi/Si colourize -Ss/-Qs colourize -Sl/-Ql colourize -Q update documentation and config files doc/pacman.8.txt | 5 +++ doc/pacman.conf.5.txt | 3 ++ etc/pacman.conf.in | 1 + src/pacman/callback.c | 26 ++++++------ src/pacman/conf.c | 52 ++++++++++++++++++++++++ src/pacman/conf.h | 25 +++++++++++- src/pacman/package.c | 107 ++++++++++++++++++++++++++++++++++++++++++++----- src/pacman/package.h | 3 ++ src/pacman/pacman.c | 16 ++++++++ src/pacman/query.c | 60 ++-------------------------- src/pacman/remove.c | 2 +- src/pacman/sync.c | 95 +++++++------------------------------------- src/pacman/util.c | 108 ++++++++++++++++++++++++++++++++++++++------------ src/pacman/util.h | 5 ++- 14 files changed, 320 insertions(+), 188 deletions(-) -- 1.8.1.5
Signed-off-by: Simon Gomizelj
On 07/03/13 03:51, Simon Gomizelj wrote:
Signed-off-by: Simon Gomizelj
--- src/pacman/package.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/pacman/package.h | 3 ++ src/pacman/query.c | 56 +---------------------------------- src/pacman/sync.c | 75 ++-------------------------------------------- 4 files changed, 91 insertions(+), 127 deletions(-) diff --git a/src/pacman/package.c b/src/pacman/package.c index 330f7ab..e94c295 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -341,4 +341,88 @@ void dump_pkg_changelog(alpm_pkg_t *pkg) } }
+void print_installed(alpm_db_t *db_local, alpm_pkg_t *pkg) +{ + const char *pkgname = alpm_pkg_get_name(pkg); + const char *pkgver = alpm_pkg_get_version(pkg); + alpm_pkg_t *lpkg = alpm_db_get_pkg(db_local, pkgname); + if(lpkg) { + const char *lpkgver = alpm_pkg_get_version(lpkg); + if(strcmp(lpkgver, pkgver) == 0) { + printf(" [%s]", _("installed")); + } else { + printf(" [%s: %s]", _("installed"), lpkgver); + } + } +} + +/** + * Display the defails of a search. + * @param db the database we're searching + * @param targets the targets we're searching for + * @param show_status show if the package is also in the local db + */ +int dump_pkg_search(alpm_db_t *db, alpm_list_t *targets, int show_status) +{ + int freelist = 0; + alpm_list_t *i, *searchlist; + unsigned short cols; + + /* if we have a targets list, search for packages matching it */ + if(targets) { + searchlist = alpm_db_search(db, targets); + freelist = 1; + } else { + searchlist = alpm_db_get_pkgcache(db); + freelist = 0; + } + if(searchlist == NULL) { + return 1; + } + + cols = getcols(fileno(stdout)); + for(i = searchlist; i; i = alpm_list_next(i)) { + alpm_list_t *grp; + alpm_pkg_t *pkg = i->data; + + if(config->quiet) { + fputs(alpm_pkg_get_name(pkg), stdout); + } else { + printf("%s/%s %s", alpm_db_get_name(db), + alpm_pkg_get_name(pkg), alpm_pkg_get_version(pkg)); + + if((grp = alpm_pkg_get_groups(pkg)) != NULL) { + alpm_list_t *k; + fputs(" (", stdout); + for(k = grp; k; k = alpm_list_next(k)) { + const char *group = k->data; + fputs(group, stdout); + if(alpm_list_next(k)) { + /* only print a spacer if there are more groups */ + putchar(' '); + } + } + putchar(')'); + } + + if(show_status) { + alpm_db_t *db_local = alpm_get_localdb(config->handle);
We don't need to do this for every package... Declare it at the start and do a "if(show_status && !config->quiet)" after the initial checks.
+ print_installed(db_local, pkg); + } + + /* we need a newline and initial indent first */ + fputs("\n ", stdout); + indentprint(alpm_pkg_get_desc(pkg), 4, cols); + } + fputc('\n', stdout); + } + + /* we only want to free if the list was a search list */ + if(freelist) { + alpm_list_free(searchlist); + } + + return 0; +} +
Will do. FWIW if(show_status) is enough, the parent if already checks for !config->quiet > We don't need to do this for every package... Declare it at the start > and do a "if(show_status && !config->quiet)" after the initial > checks.
Derp.
if(show_status) if enough since the code only runs if its not quiet already.
On Thu, Mar 7, 2013 at 12:28 AM, Simon Gomizelj
Will do.
FWIW if(show_status) is enough, the parent if already checks for !config->quiet > We don't need to do this for every package... Declare it at the start > and do a "if(show_status && !config->quiet)" after the initial > checks.
This will substantially simplify the logic to add colours to messages.
Signed-off-by: Simon Gomizelj
Basically all translation messages that need colouring but _also_ happen
to be format strings need to be split up.
This makes it easy to conditionally embed colour codes into the output
at runtime.
Signed-off-by: Simon Gomizelj
Remove the format component of the "Total Download Size" and related
messages. The heading will be colourized, the size won't.
However since the length of these messages can vary by language, we need
a pretty printer to format them nicely.
Signed-off-by: Simon Gomizelj
On 07/03/13 03:51, Simon Gomizelj wrote:
Remove the format component of the "Total Download Size" and related messages. The heading will be colourized, the size won't.
However since the length of these messages can vary by language, we need a pretty printer to format them nicely.
Signed-off-by: Simon Gomizelj
--- src/pacman/util.c | 56 +++++++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 10 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 90892af..aa352e3 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -47,6 +47,11 @@ #include "callback.h"
+struct table_row_t { + const char *label; + int size; +}; + int trans_init(alpm_transflag_t flags, int check_valid) { int ret; @@ -824,15 +829,48 @@ static alpm_list_t *create_verbose_row(pm_target_t *target) return ret; }
+static void add_table_row(alpm_list_t **rows, const char *label, int size)
I'd like a more descriptive name for this. How about add_transaction_sizes_row()? Also, can we change "rows" to "table". This makes the variable names more distinct.
+{ + struct table_row_t *row = malloc(sizeof(struct table_row_t)); + + row->label = label; + row->size = size; + + *rows = alpm_list_add(*rows, row); +} + +static void display_transaction_sizes(alpm_list_t *table) +{ + alpm_list_t *i; + int max_len = 0; + + for(i = table; i; i = alpm_list_next(i)) { + struct table_row_t *row = i->data; + int len = string_length(row->label); + + if(len > max_len) + max_len = len; + } + + max_len += 2; + + for(i = table; i; i = alpm_list_next(i)) { + struct table_row_t *row = i->data; + const char *units; + double s = humanize_size(row->size, 'M', 2, &units); + + printf("%-*s %.2f %s\n", max_len, row->label, s, units); + } +} + /* prepare a list of pkgs to display */ static void _display_targets(alpm_list_t *targets, int verbose) { char *str; - const char *label; - double size; off_t isize = 0, rsize = 0, dlsize = 0; unsigned short cols; alpm_list_t *i, *rows = NULL, *names = NULL; + alpm_list_t *table = NULL;
Combine into above line.
if(!targets) { return; @@ -897,24 +935,22 @@ static void _display_targets(alpm_list_t *targets, int verbose) free(str);
if(dlsize > 0 || config->op_s_downloadonly) { - size = humanize_size(dlsize, 'M', 2, &label); - printf(_("Total Download Size: %.2f %s\n"), size, label); + add_table_row(&table, _("Total Download Size:"), dlsize); } if(!config->op_s_downloadonly) { if(isize > 0) { - size = humanize_size(isize, 'M', 2, &label); - printf(_("Total Installed Size: %.2f %s\n"), size, label); + add_table_row(&table, _("Total Installed Size:"), isize); } if(rsize > 0 && isize == 0) { - size = humanize_size(rsize, 'M', 2, &label); - printf(_("Total Removed Size: %.2f %s\n"), size, label); + add_table_row(&table, _("Total Removed Size:"), rsize); } /* only show this net value if different from raw installed size */ if(isize > 0 && rsize > 0) { - size = humanize_size(isize - rsize, 'M', 2, &label); - printf(_("Net Upgrade Size: %.2f %s\n"), size, label); + add_table_row(&table, _("Net Upgrade Size:"), isize - rsize); } } + display_transaction_sizes(table); + FREELIST(table); }
static int target_cmp(const void *p1, const void *p2)
Signed-off-by: Simon Gomizelj
Colours can be enabled in two ways:
- Add Color to pacman.conf. This enables colours automatically.
- Use --color=WHEN where WHEN is none/auto/always.
WHEN as 'never' disables colours (overrides config file), as 'auto'
enables colours when stdout is a tty, and 'always' enables colours no
matter what.
Signed-off-by: Simon Gomizelj
colstr_t colstr will hold the colourizing agents.
Signed-off-by: Simon Gomizelj
Signed-off-by: Simon Gomizelj
Signed-off-by: Simon Gomizelj
Signed-off-by: Simon Gomizelj
Signed-off-by: Simon Gomizelj
Signed-off-by: Simon Gomizelj
Signed-off-by: Simon Gomizelj
Signed-off-by: Simon Gomizelj
Signed-off-by: Simon Gomizelj
participants (2)
-
Allan McRae
-
Simon Gomizelj