[pacman-dev] Colour support in pacman!

newer
[pacman-dev] Cleaning up pretty...

Simon Gomizelj

1 Mar 2013 1 Mar '13

9:32 p.m.

Colourize pacman output! This patchset duplicates about 95% of the colour support pacman-color provides. I don't duplicate everything and I vary slightly in how I colour certain messages to avoid massively restructuring pacman. Its a good starting point which can be improved on. Work on improving this means improving a lot of pacman's pretty printing code. Anyhow... Colours can be enabled in two ways: - Add 'Color' to pacman.conf. This enables automatic colouring. - 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. None disables colours (overrides config file), auto enables colours when stdout is a tty, and always enables colours no matter what. I do not read pacman-color's color.conf. Probably never will. - We can't configure makepkg's colours - The config file has _very_ strange semantics (eg Red = intensive red) doc/pacman.8.txt | 3 ++ 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 | 22 +++++++++++- src/pacman/package.c | 91 ++++++++++++++++++++++++++++++++++++++++++----- src/pacman/package.h | 3 ++ src/pacman/pacman.c | 15 ++++++++ src/pacman/query.c | 59 ++----------------------------- src/pacman/remove.c | 2 +- src/pacman/sync.c | 95 ++++++++----------------------------------------- src/pacman/util.c | 98 ++++++++++++++++++++++++++++++++++++++------------- src/pacman/util.h | 5 +-- 14 files changed, 289 insertions(+), 186 deletions(-)

Show replies by date

Simon Gomizelj

1 Mar 1 Mar

9:32 p.m.

New subject: [pacman-dev] [PATCH 01/16] refactor common code in query_search/sync_search

Signed-off-by: Simon Gomizelj --- src/pacman/package.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/pacman/package.h | 3 ++ src/pacman/query.c | 56 +------------------------------------- src/pacman/sync.c | 76 +++------------------------------------------------ 4 files changed, 85 insertions(+), 127 deletions(-) diff --git a/src/pacman/package.c b/src/pacman/package.c index 330f7ab..6daf745 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -341,4 +341,81 @@ 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); + } + } +} + +/* search a database for a matching package */ +int dump_pkg_search(alpm_db_t *db, alpm_list_t *targets, alpm_db_t *installed_in) +{ + 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 (installed_in) + print_installed(installed_in, 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; +} + /* vim: set ts=2 sw=2 noet: */ diff --git a/src/pacman/package.h b/src/pacman/package.h index 47fbcad..635d5f7 100644 --- a/src/pacman/package.h +++ b/src/pacman/package.h @@ -28,6 +28,9 @@ void dump_pkg_backups(alpm_pkg_t *pkg); void dump_pkg_files(alpm_pkg_t *pkg, int quiet); void dump_pkg_changelog(alpm_pkg_t *pkg); +void print_installed(alpm_db_t *db_local, alpm_pkg_t *pkg); +int dump_pkg_search(alpm_db_t *db, alpm_list_t *targets, alpm_db_t *installed_in); + #endif /* _PM_PACKAGE_H */ /* vim: set ts=2 sw=2 noet: */ diff --git a/src/pacman/query.c b/src/pacman/query.c index 1247c3d..416f92c 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -262,62 +262,8 @@ targcleanup: /* search the local database for a matching package */ static int query_search(alpm_list_t *targets) { - alpm_list_t *i, *searchlist; - int freelist; alpm_db_t *db_local = alpm_get_localdb(config->handle); - unsigned short cols; - - /* if we have a targets list, search for packages matching it */ - if(targets) { - searchlist = alpm_db_search(db_local, targets); - freelist = 1; - } else { - searchlist = alpm_db_get_pkgcache(db_local); - 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) { - printf(LOCAL_PREFIX "%s %s", alpm_pkg_get_name(pkg), alpm_pkg_get_version(pkg)); - } else { - fputs(alpm_pkg_get_name(pkg), stdout); - } - - - if(!config->quiet) { - 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(')'); - } - - /* 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; + return dump_pkg_search(db_local, targets, NULL); } static int query_group(alpm_list_t *targets) diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 924cdf5..b1cc3fe 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -352,87 +352,19 @@ static int sync_synctree(int level, alpm_list_t *syncs) return (success > 0); } -static 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); - } - } -} - /* search the sync dbs for a matching package */ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets) { - alpm_list_t *i, *j, *ret; - int freelist; - int found = 0; + alpm_list_t *i; alpm_db_t *db_local = alpm_get_localdb(config->handle); + int found = 0; for(i = syncs; i; i = alpm_list_next(i)) { alpm_db_t *db = i->data; - unsigned short cols; - /* if we have a targets list, search for packages matching it */ - if(targets) { - ret = alpm_db_search(db, targets); - freelist = 1; - } else { - ret = alpm_db_get_pkgcache(db); - freelist = 0; - } - if(ret == NULL) { - continue; - } else { - found = 1; - } - cols = getcols(fileno(stdout)); - for(j = ret; j; j = alpm_list_next(j)) { - alpm_list_t *grp; - alpm_pkg_t *pkg = j->data; - - if(!config->quiet) { - printf("%s/%s %s", alpm_db_get_name(db), alpm_pkg_get_name(pkg), - alpm_pkg_get_version(pkg)); - } else { - fputs(alpm_pkg_get_name(pkg), stdout); - } - - if(!config->quiet) { - 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(')'); - } - - 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(ret); - } + found += !dump_pkg_search(db, targets, db_local); } - return !found; + return (found == 0); } static int sync_group(int level, alpm_list_t *syncs, alpm_list_t *targets) -- 1.8.1.4

Andrew Gregory

2 Mar 2 Mar

4:27 a.m.

New subject: [pacman-dev] [PATCH 01/16] refactor common code in query_search/sync_search

On 03/01/13 at 04:32pm, Simon Gomizelj wrote:

...

Signed-off-by: Simon Gomizelj --- src/pacman/package.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/pacman/package.h | 3 ++ src/pacman/query.c | 56 +------------------------------------- src/pacman/sync.c | 76 +++------------------------------------------------ 4 files changed, 85 insertions(+), 127 deletions(-)

diff --git a/src/pacman/package.c b/src/pacman/package.c index 330f7ab..6daf745 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -341,4 +341,81 @@ void dump_pkg_changelog(alpm_pkg_t *pkg) <snip> + if (installed_in) + print_installed(installed_in, pkg);

style: always use braces and don't add space in front of parentheses. apg

Allan McRae

6 Mar 6 Mar

11:01 a.m.

New subject: [pacman-dev] [PATCH 01/16] refactor common code in query_search/sync_search

On 02/03/13 07:32, Simon Gomizelj wrote:

...

Signed-off-by: Simon Gomizelj --- src/pacman/package.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/pacman/package.h | 3 ++ src/pacman/query.c | 56 +------------------------------------- src/pacman/sync.c | 76 +++------------------------------------------------ 4 files changed, 85 insertions(+), 127 deletions(-)

diff --git a/src/pacman/package.c b/src/pacman/package.c index 330f7ab..6daf745 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -341,4 +341,81 @@ 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); + } + } +} + +/* search a database for a matching package */ +int dump_pkg_search(alpm_db_t *db, alpm_list_t *targets, alpm_db_t *installed_in)

We should change that installed_in parameter to something else... status_db? Or just replace it with a true/false parameter to print the installation status? In fact... do this (even though it means calling alpm_get_localdb for every configured repo - tiny overhead). Also, add some doxygen comment at the top so we know what the parameter does...

...

+{ + 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 (installed_in) + print_installed(installed_in, 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; +} + /* vim: set ts=2 sw=2 noet: */ diff --git a/src/pacman/package.h b/src/pacman/package.h index 47fbcad..635d5f7 100644 --- a/src/pacman/package.h +++ b/src/pacman/package.h @@ -28,6 +28,9 @@ void dump_pkg_backups(alpm_pkg_t *pkg); void dump_pkg_files(alpm_pkg_t *pkg, int quiet); void dump_pkg_changelog(alpm_pkg_t *pkg);

+void print_installed(alpm_db_t *db_local, alpm_pkg_t *pkg); +int dump_pkg_search(alpm_db_t *db, alpm_list_t *targets, alpm_db_t *installed_in); + #endif /* _PM_PACKAGE_H */

/* vim: set ts=2 sw=2 noet: */ diff --git a/src/pacman/query.c b/src/pacman/query.c index 1247c3d..416f92c 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -262,62 +262,8 @@ targcleanup: /* search the local database for a matching package */ static int query_search(alpm_list_t *targets) { - alpm_list_t *i, *searchlist; - int freelist; alpm_db_t *db_local = alpm_get_localdb(config->handle); - unsigned short cols; - - /* if we have a targets list, search for packages matching it */ - if(targets) { - searchlist = alpm_db_search(db_local, targets); - freelist = 1; - } else { - searchlist = alpm_db_get_pkgcache(db_local); - 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) { - printf(LOCAL_PREFIX "%s %s", alpm_pkg_get_name(pkg), alpm_pkg_get_version(pkg)); - } else { - fputs(alpm_pkg_get_name(pkg), stdout); - } - - - if(!config->quiet) { - 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(')'); - } - - /* 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; + return dump_pkg_search(db_local, targets, NULL); }

static int query_group(alpm_list_t *targets) diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 924cdf5..b1cc3fe 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -352,87 +352,19 @@ static int sync_synctree(int level, alpm_list_t *syncs) return (success > 0); }

-static 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); - } - } -} - /* search the sync dbs for a matching package */ static int sync_search(alpm_list_t *syncs, alpm_list_t *targets) { - alpm_list_t *i, *j, *ret; - int freelist; - int found = 0; + alpm_list_t *i; alpm_db_t *db_local = alpm_get_localdb(config->handle); + int found = 0;

for(i = syncs; i; i = alpm_list_next(i)) { alpm_db_t *db = i->data; - unsigned short cols; - /* if we have a targets list, search for packages matching it */ - if(targets) { - ret = alpm_db_search(db, targets); - freelist = 1; - } else { - ret = alpm_db_get_pkgcache(db); - freelist = 0; - } - if(ret == NULL) { - continue; - } else { - found = 1; - } - cols = getcols(fileno(stdout)); - for(j = ret; j; j = alpm_list_next(j)) { - alpm_list_t *grp; - alpm_pkg_t *pkg = j->data; - - if(!config->quiet) { - printf("%s/%s %s", alpm_db_get_name(db), alpm_pkg_get_name(pkg), - alpm_pkg_get_version(pkg)); - } else { - fputs(alpm_pkg_get_name(pkg), stdout); - } - - if(!config->quiet) { - 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(')'); - } - - 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(ret); - } + found += !dump_pkg_search(db, targets, db_local); }

- return !found; + return (found == 0); }

static int sync_group(int level, alpm_list_t *syncs, alpm_list_t *targets)

Simon Gomizelj

1 Mar 1 Mar

9:32 p.m.

New subject: [pacman-dev] [PATCH 02/16] remove :: prefix from all message

This will substantially simplify the logic to add colours to messages. Signed-off-by: Simon Gomizelj --- src/pacman/callback.c | 26 +++++++++++++------------- src/pacman/remove.c | 2 +- src/pacman/sync.c | 14 +++++++------- src/pacman/util.c | 15 ++++++++++++++- src/pacman/util.h | 1 + 5 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/pacman/callback.c b/src/pacman/callback.c index edd5b39..7014377 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -244,7 +244,7 @@ void cb_event(alpm_event_t event, void *data1, void *data2) fputs((const char *)data1, stdout); break; case ALPM_EVENT_RETRIEVE_START: - printf(_(":: Retrieving packages ...\n")); + colon_printf(_("Retrieving packages ...\n")); break; case ALPM_EVENT_DISKSPACE_START: if(config->noprogressbar) { @@ -252,7 +252,7 @@ void cb_event(alpm_event_t event, void *data1, void *data2) } break; case ALPM_EVENT_OPTDEP_REQUIRED: - printf(_(":: %s optionally requires %s\n"), alpm_pkg_get_name(data1), + printf(_("%s optionally requires %s\n"), alpm_pkg_get_name(data1), alpm_dep_compute_string(data2)); break; case ALPM_EVENT_DATABASE_MISSING: @@ -290,14 +290,14 @@ void cb_question(alpm_question_t event, void *data1, void *data2, switch(event) { case ALPM_QUESTION_INSTALL_IGNOREPKG: if(!config->op_s_downloadonly) { - *response = yesno(_(":: %s is in IgnorePkg/IgnoreGroup. Install anyway?"), + *response = yesno(_("%s is in IgnorePkg/IgnoreGroup. Install anyway?"), alpm_pkg_get_name(data1)); } else { *response = 1; } break; case ALPM_QUESTION_REPLACE_PKG: - *response = yesno(_(":: Replace %s with %s/%s?"), + *response = yesno(_("Replace %s with %s/%s?"), alpm_pkg_get_name(data1), (char *)data3, alpm_pkg_get_name(data2)); @@ -306,12 +306,12 @@ void cb_question(alpm_question_t event, void *data1, void *data2, /* data parameters: target package, local package, conflict (strings) */ /* print conflict only if it contains new information */ if(strcmp(data1, data3) == 0 || strcmp(data2, data3) == 0) { - *response = noyes(_(":: %s and %s are in conflict. Remove %s?"), + *response = noyes(_("%s and %s are in conflict. Remove %s?"), (char *)data1, (char *)data2, (char *)data2); } else { - *response = noyes(_(":: %s and %s are in conflict (%s). Remove %s?"), + *response = noyes(_("%s and %s are in conflict (%s). Remove %s?"), (char *)data1, (char *)data2, (char *)data3, @@ -328,9 +328,9 @@ void cb_question(alpm_question_t event, void *data1, void *data2, (char *)alpm_pkg_get_name(i->data)); count++; } - printf(_n( - ":: The following package cannot be upgraded due to unresolvable dependencies:\n", - ":: The following packages cannot be upgraded due to unresolvable dependencies:\n", + colon_printf(_n( + "The following package cannot be upgraded due to unresolvable dependencies:\n", + "The following packages cannot be upgraded due to unresolvable dependencies:\n", count)); list_display(" ", namelist, getcols(fileno(stdout))); printf("\n"); @@ -346,7 +346,7 @@ void cb_question(alpm_question_t event, void *data1, void *data2, alpm_list_t *providers = data1; size_t count = alpm_list_count(providers); char *depstring = alpm_dep_compute_string((alpm_depend_t *)data2); - printf(_(":: There are %zd providers available for %s:\n"), count, + colon_printf(_("There are %zd providers available for %s:\n"), count, depstring); free(depstring); select_display(providers); @@ -355,7 +355,7 @@ void cb_question(alpm_question_t event, void *data1, void *data2, break; case ALPM_QUESTION_LOCAL_NEWER: if(!config->op_s_downloadonly) { - *response = yesno(_(":: %s-%s: local version is newer. Upgrade anyway?"), + *response = yesno(_("%s-%s: local version is newer. Upgrade anyway?"), alpm_pkg_get_name(data1), alpm_pkg_get_version(data1)); } else { @@ -363,7 +363,7 @@ void cb_question(alpm_question_t event, void *data1, void *data2, } break; case ALPM_QUESTION_CORRUPTED_PKG: - *response = yesno(_(":: File %s is corrupted (%s).\n" + *response = yesno(_("File %s is corrupted (%s).\n" "Do you want to delete it?"), (char *)data1, alpm_strerror(*(alpm_errno_t *)data2)); @@ -380,7 +380,7 @@ void cb_question(alpm_question_t event, void *data1, void *data2, revoked = " (revoked)"; } - *response = yesno(_(":: Import PGP key %d%c/%s, \"%s\", created: %s%s?"), + *response = yesno(_("Import PGP key %d%c/%s, \"%s\", created: %s%s?"), key->length, key->pubkey_algo, key->fingerprint, key->uid, created, revoked); } break; diff --git a/src/pacman/remove.c b/src/pacman/remove.c index 943a555..8cf3ebc 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -119,7 +119,7 @@ int pacman_remove(alpm_list_t *targets) for(i = data; i; i = alpm_list_next(i)) { alpm_depmissing_t *miss = i->data; char *depstring = alpm_dep_compute_string(miss->depend); - printf(_(":: %s: requires %s\n"), miss->target, depstring); + colon_printf(_("%s: requires %s\n"), miss->target, depstring); free(depstring); } break; diff --git a/src/pacman/sync.c b/src/pacman/sync.c index b1cc3fe..12e965b 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -593,7 +593,7 @@ static int process_group(alpm_list_t *dbs, const char *group, int error) } if(config->print == 0) { - printf(_(":: There are %d members in group %s:\n"), count, + colon_printf(_("There are %d members in group %s:\n"), count, group); select_display(pkgs); char *array = malloc(count); @@ -718,7 +718,7 @@ static int sync_trans(alpm_list_t *targets) } if(config->op_s_upgrade) { - printf(_(":: Starting full system upgrade...\n")); + colon_printf(_("Starting full system upgrade...\n")); alpm_logaction(config->handle, PACMAN_CALLER_PREFIX, "starting full system upgrade\n"); if(alpm_sync_sysupgrade(config->handle, config->op_s_upgrade >= 2) == -1) { @@ -745,14 +745,14 @@ int sync_prepare_execute(void) case ALPM_ERR_PKG_INVALID_ARCH: for(i = data; i; i = alpm_list_next(i)) { const char *pkg = i->data; - printf(_(":: package %s does not have a valid architecture\n"), pkg); + colon_printf(_("package %s does not have a valid architecture\n"), pkg); } break; case ALPM_ERR_UNSATISFIED_DEPS: for(i = data; i; i = alpm_list_next(i)) { alpm_depmissing_t *miss = i->data; char *depstring = alpm_dep_compute_string(miss->depend); - printf(_(":: %s: requires %s\n"), miss->target, depstring); + colon_printf(_("%s: requires %s\n"), miss->target, depstring); free(depstring); } break; @@ -761,11 +761,11 @@ int sync_prepare_execute(void) alpm_conflict_t *conflict = i->data; /* only print reason if it contains new information */ if(conflict->reason->mod == ALPM_DEP_MOD_ANY) { - printf(_(":: %s and %s are in conflict\n"), + colon_printf(_("%s and %s are in conflict\n"), conflict->package1, conflict->package2); } else { char *reason = alpm_dep_compute_string(conflict->reason); - printf(_(":: %s and %s are in conflict (%s)\n"), + colon_printf(_("%s and %s are in conflict (%s)\n"), conflict->package1, conflict->package2, reason); free(reason); } @@ -890,7 +890,7 @@ int pacman_sync(alpm_list_t *targets) if(config->op_s_sync) { /* grab a fresh package list */ - printf(_(":: Synchronizing package databases...\n")); + colon_printf(_("Synchronizing package databases...\n")); alpm_logaction(config->handle, PACMAN_CALLER_PREFIX, "synchronizing package lists\n"); if(!sync_synctree(config->op_s_sync, sync_dbs)) { diff --git a/src/pacman/util.c b/src/pacman/util.c index 3270c74..d5fc32c 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1219,7 +1219,6 @@ static void display_repo_list(const char *dbname, alpm_list_t *list, { const char *prefix = " "; - printf(":: "); printf(_("Repository %s\n"), dbname); list_display(prefix, list, cols); } @@ -1461,6 +1460,7 @@ static int question(short preset, char *fmt, va_list args) fflush(stdout); fflush(stderr); + fprintf(stream, ":: "); vfprintf(stream, fmt, args); if(preset) { @@ -1522,6 +1522,19 @@ int noyes(char *fmt, ...) return ret; } +int colon_printf(const char *fmt, ...) +{ + int ret; + va_list args; + + printf(":: "); + va_start(args, fmt); + ret = vprintf(fmt, args); + va_end(args); + + return ret; +} + int pm_printf(alpm_loglevel_t level, const char *format, ...) { int ret; diff --git a/src/pacman/util.h b/src/pacman/util.h index 2d1e698..f579b7e 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -72,6 +72,7 @@ void print_packages(const alpm_list_t *packages); void select_display(const alpm_list_t *pkglist); int select_question(int count); int multiselect_question(char *array, int count); +int colon_printf(const char *fmt, ...) __attribute__((format(printf, 1, 2))); int yesno(char *fmt, ...) __attribute__((format(printf, 1, 2))); int noyes(char *fmt, ...) __attribute__((format(printf, 1, 2))); -- 1.8.1.4

Andrew Gregory

2 Mar 2 Mar

4:31 a.m.

New subject: [pacman-dev] [PATCH 02/16] remove :: prefix from all message

On 03/01/13 at 04:32pm, Simon Gomizelj wrote:

...

This will substantially simplify the logic to add colours to messages.

Signed-off-by: Simon Gomizelj --- src/pacman/callback.c | 26 +++++++++++++------------- src/pacman/remove.c | 2 +- src/pacman/sync.c | 14 +++++++------- src/pacman/util.c | 15 ++++++++++++++- src/pacman/util.h | 1 + 5 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/src/pacman/callback.c b/src/pacman/callback.c index edd5b39..7014377 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -244,7 +244,7 @@ void cb_event(alpm_event_t event, void *data1, void *data2) fputs((const char *)data1, stdout); break; case ALPM_EVENT_RETRIEVE_START: - printf(_(":: Retrieving packages ...\n")); + colon_printf(_("Retrieving packages ...\n")); break; case ALPM_EVENT_DISKSPACE_START: if(config->noprogressbar) { @@ -252,7 +252,7 @@ void cb_event(alpm_event_t event, void *data1, void *data2) } break; case ALPM_EVENT_OPTDEP_REQUIRED: - printf(_(":: %s optionally requires %s\n"), alpm_pkg_get_name(data1), + printf(_("%s optionally requires %s\n"), alpm_pkg_get_name(data1),

Did you intend to remove the colon and not change this to colon_printf?

...

alpm_dep_compute_string(data2)); break; case ALPM_EVENT_DATABASE_MISSING:

<snip>

...

diff --git a/src/pacman/util.c b/src/pacman/util.c index 3270c74..d5fc32c 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1219,7 +1219,6 @@ static void display_repo_list(const char *dbname, alpm_list_t *list, { const char *prefix = " ";

- printf(":: "); printf(_("Repository %s\n"), dbname);

Did you intend to remove the colon and not change this to colon_printf?

...

list_display(prefix, list, cols); } @@ -1461,6 +1460,7 @@ static int question(short preset, char *fmt, va_list args) fflush(stdout); fflush(stderr);

+ fprintf(stream, ":: ");

You can use fputs here instead of fprintf.

...

vfprintf(stream, fmt, args);

if(preset) { @@ -1522,6 +1522,19 @@ int noyes(char *fmt, ...) return ret; }

+int colon_printf(const char *fmt, ...) +{ + int ret; + va_list args; + + printf(":: ");

Any reason not to add a colon_vprintf that could be used here and consolidate all of the colon prefixing in one place instead of having it in colon_printf and here?

...

+ va_start(args, fmt); + ret = vprintf(fmt, args); + va_end(args); + + return ret; +} + int pm_printf(alpm_loglevel_t level, const char *format, ...) { int ret; diff --git a/src/pacman/util.h b/src/pacman/util.h index 2d1e698..f579b7e 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -72,6 +72,7 @@ void print_packages(const alpm_list_t *packages); void select_display(const alpm_list_t *pkglist); int select_question(int count); int multiselect_question(char *array, int count); +int colon_printf(const char *fmt, ...) __attribute__((format(printf, 1, 2))); int yesno(char *fmt, ...) __attribute__((format(printf, 1, 2))); int noyes(char *fmt, ...) __attribute__((format(printf, 1, 2)));

-- 1.8.1.4

apg

Simon Gomizelj

1 Mar 1 Mar

9:32 p.m.

New subject: [pacman-dev] [PATCH 03/16] split "Packages (%zd):" message

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 --- src/pacman/util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index d5fc32c..29b9f38 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -871,7 +871,7 @@ static void _display_targets(alpm_list_t *targets, int verbose) } /* print to screen */ - pm_asprintf(&str, _("Packages (%zd):"), alpm_list_count(targets)); + pm_asprintf(&str, "%s (%zd):", _("Packages"), alpm_list_count(targets)); printf("\n"); cols = getcols(fileno(stdout)); -- 1.8.1.4

Simon Gomizelj

9:32 p.m.

New subject: [pacman-dev] [PATCH 04/16] remove format from statistic messages

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 | 51 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 29b9f38..45ae983 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,35 @@ static alpm_list_t *create_verbose_row(pm_target_t *target) return ret; } +static void _print_table(struct table_row_t *rows, int spacing) +{ + struct table_row_t *row; + int max_len = 0; + + /* max length line */ + for(row = rows; row && row->label; ++row) { + int len = string_length(row->label); + if(len > max_len) + max_len = len; + } + + max_len += spacing; + + for(row = rows; row && row->label; ++row) { + 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; + struct table_row_t tbl_rows[5], *row = tbl_rows; if(!targets) { return; @@ -897,24 +922,30 @@ 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); + row->label = _("Total Download Size:"); + row->size = dlsize; + ++row; } if(!config->op_s_downloadonly) { if(isize > 0) { - size = humanize_size(isize, 'M', 2, &label); - printf(_("Total Installed Size: %.2f %s\n"), size, label); + row->label = _("Total Installed Size:"); + row->size = isize; + ++row; } if(rsize > 0 && isize == 0) { - size = humanize_size(rsize, 'M', 2, &label); - printf(_("Total Removed Size: %.2f %s\n"), size, label); + row->label = _("Total Removed Size:"); + row->size = rsize; + ++row; } /* 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); + row->label = _("Net Upgrade Size:"); + row->size = isize - rsize; + ++row; } } + row->label = NULL; + _print_table(tbl_rows, 2); } static int target_cmp(const void *p1, const void *p2) -- 1.8.1.4

Allan McRae

6 Mar 6 Mar

3:16 p.m.

New subject: [pacman-dev] [PATCH 04/16] remove format from statistic messages

On 02/03/13 07:32, 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 | 51 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 10 deletions(-)

I'm going to ask for quite some changes here, but they should all be easy...

...

diff --git a/src/pacman/util.c b/src/pacman/util.c index 29b9f38..45ae983 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,35 @@ static alpm_list_t *create_verbose_row(pm_target_t *target) return ret; }

+static void _print_table(struct table_row_t *rows, int spacing)

Kill the underscore at the start of the name. We don't do that... (see below). Also, lets just give this a descriptive name. display_transaction_sizes(). Why do we need the space parameter? This function is used in a single place... Delete it.

...

+{ + struct table_row_t *row; + int max_len = 0; + + /* max length line */ + for(row = rows; row && row->label; ++row) { + int len = string_length(row->label); + if(len > max_len) + max_len = len; + } + + max_len += spacing; + + for(row = rows; row && row->label; ++row) { + 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)

(not for this patch) This underscore needs to die too... but we already have display_targets. Descriptive name needed... Sigh!

...

{ 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; + struct table_row_t tbl_rows[5], *row = tbl_rows;

Not a fan of this at all... An array that is actually of variable length and at most the fifth element is used to confirm there is no more elements. Yuck! Just use an alpm_list_t here.

...

if(!targets) { return; @@ -897,24 +922,30 @@ 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); + row->label = _("Total Download Size:");

Can we add the ":" at the end rather than have it "translated"?

...

+ row->size = dlsize; + ++row; } if(!config->op_s_downloadonly) { if(isize > 0) { - size = humanize_size(isize, 'M', 2, &label); - printf(_("Total Installed Size: %.2f %s\n"), size, label); + row->label = _("Total Installed Size:"); + row->size = isize; + ++row; } if(rsize > 0 && isize == 0) { - size = humanize_size(rsize, 'M', 2, &label); - printf(_("Total Removed Size: %.2f %s\n"), size, label); + row->label = _("Total Removed Size:"); + row->size = rsize; + ++row; } /* 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); + row->label = _("Net Upgrade Size:"); + row->size = isize - rsize; + ++row; } } + row->label = NULL; + _print_table(tbl_rows, 2); }

static int target_cmp(const void *p1, const void *p2)

Simon Gomizelj

5:32 p.m.

New subject: [pacman-dev] [PATCH 04/16] remove format from statistic messages

...

...
if(!targets) { return; @@ -897,24 +922,30 @@ 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); + row->label = _("Total Download Size:");

Can we add the ":" at the end rather than have it "translated"?

Talked about this briefly on IRC, but just for the record here: I can't just yet due to how the alignment is done. Trailing colons in translation messages is a big problem is pacman and should get tackled properly.

Simon Gomizelj

1 Mar 1 Mar

9:32 p.m.

New subject: [pacman-dev] [PATCH 05/16] standarise format functions

Signed-off-by: Simon Gomizelj --- src/pacman/util.c | 16 ++++++++-------- src/pacman/util.h | 6 +++--- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 45ae983..6e06690 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1474,7 +1474,7 @@ int select_question(int count) /* presents a prompt and gets a Y/N answer */ __attribute__((format(printf, 2, 0))) -static int question(short preset, char *fmt, va_list args) +static int question(short preset, const char *format, va_list args) { char response[32]; FILE *stream; @@ -1492,7 +1492,7 @@ static int question(short preset, char *fmt, va_list args) fflush(stderr); fprintf(stream, ":: "); - vfprintf(stream, fmt, args); + vfprintf(stream, format, args); if(preset) { fprintf(stream, " %s ", _("[Y/n]")); @@ -1529,25 +1529,25 @@ static int question(short preset, char *fmt, va_list args) return 0; } -int yesno(char *fmt, ...) +int yesno(const char *format, ...) { int ret; va_list args; - va_start(args, fmt); - ret = question(1, fmt, args); + va_start(args, format); + ret = question(1, format, args); va_end(args); return ret; } -int noyes(char *fmt, ...) +int noyes(const char *format, ...) { int ret; va_list args; - va_start(args, fmt); - ret = question(0, fmt, args); + va_start(args, format); + ret = question(0, format, args); va_end(args); return ret; diff --git a/src/pacman/util.h b/src/pacman/util.h index f579b7e..0a2a6f7 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -72,9 +72,9 @@ void print_packages(const alpm_list_t *packages); void select_display(const alpm_list_t *pkglist); int select_question(int count); int multiselect_question(char *array, int count); -int colon_printf(const char *fmt, ...) __attribute__((format(printf, 1, 2))); -int yesno(char *fmt, ...) __attribute__((format(printf, 1, 2))); -int noyes(char *fmt, ...) __attribute__((format(printf, 1, 2))); +int colon_printf(const char *format, ...) __attribute__((format(printf, 1, 2))); +int yesno(const char *format, ...) __attribute__((format(printf, 1, 2))); +int noyes(const char *format, ...) __attribute__((format(printf, 1, 2))); int pm_printf(alpm_loglevel_t level, const char *format, ...) __attribute__((format(printf,2,3))); int pm_asprintf(char **string, const char *format, ...) __attribute__((format(printf,2,3))); -- 1.8.1.4

Allan McRae

6 Mar 6 Mar

3:19 p.m.

New subject: [pacman-dev] [PATCH 05/16] standarise format functions

On 02/03/13 07:32, Simon Gomizelj wrote:

...

Signed-off-by: Simon Gomizelj ---

Not sure what going from "fmt" to "format" gained us, but the const correctness is fine. Signed-off-by: Allan

Simon Gomizelj

4:06 p.m.

New subject: [pacman-dev] [PATCH 05/16] standarise format functions

Well some of the printf functions use fmt, others use format (eg. question, yesno)

...

Not sure what going from "fmt" to "format" gained us, but the const correctness is fine.

Simon Gomizelj

1 Mar 1 Mar

9:32 p.m.

New subject: [pacman-dev] [PATCH 06/16] add a config settings and flag for colors

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 --- src/pacman/conf.c | 4 ++++ src/pacman/conf.h | 9 ++++++++- src/pacman/pacman.c | 15 +++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 218ffb4..dca6e3e 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -436,6 +436,10 @@ static int _parse_options(const char *key, char *value, pm_printf(ALPM_LOG_DEBUG, "config: totaldownload\n"); } else if(strcmp(key, "CheckSpace") == 0) { config->checkspace = 1; + } else if(strcmp(key, "Color") == 0) { + if(config->color == PM_COLOR_UNSET) { + config->color = isatty(fileno(stdout)) ? PM_COLOR_ON : PM_COLOR_OFF; + } } else { pm_printf(ALPM_LOG_WARNING, _("config file %s, line %d: directive '%s' in section '%s' not recognized.\n"), diff --git a/src/pacman/conf.h b/src/pacman/conf.h index d85d11f..6cabd33 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -34,6 +34,7 @@ typedef struct __config_t { unsigned short print; unsigned short checkspace; unsigned short usesyslog; + unsigned short color; double deltaratio; char *arch; char *print_format; @@ -129,7 +130,8 @@ enum { OP_PRINTFORMAT, OP_GPGDIR, OP_DBONLY, - OP_FORCE + OP_FORCE, + OP_COLOR }; /* clean method */ @@ -145,6 +147,11 @@ enum { PKG_LOCALITY_FOREIGN = (1 << 1) }; +enum { + PM_COLOR_UNSET = 0, + PM_COLOR_OFF, + PM_COLOR_ON +}; /* global config variable */ extern config_t *config; diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 38b28e1..f565352 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -197,6 +197,7 @@ static void usage(int op, const char * const myname) addlist(_(" -v, --verbose be verbose\n")); addlist(_(" --arch <arch> set an alternate architecture\n")); addlist(_(" --cachedir <dir> set an alternate package cache location\n")); + addlist(_(" --color <when> colorize the output\n")); addlist(_(" --config <path> set an alternate configuration file\n")); addlist(_(" --debug display debug messages\n")); addlist(_(" --gpgdir <path> set an alternate home directory for GnuPG\n")); @@ -394,6 +395,19 @@ static int parsearg_global(int opt) check_optarg(); config->cachedirs = alpm_list_add(config->cachedirs, strdup(optarg)); break; + case OP_COLOR: + if (strcmp("never", optarg) == 0) { + config->color = PM_COLOR_OFF; + } else if (strcmp("auto", optarg) == 0) { + config->color = isatty(fileno(stdout)) ? PM_COLOR_ON : PM_COLOR_OFF; + } else if (strcmp("always", optarg) == 0) { + config->color = PM_COLOR_ON; + } else { + pm_printf(ALPM_LOG_ERROR, _("invalid agument '%s' for --color\n"), + optarg); + return 1; + } + break; case OP_CONFIG: check_optarg(); if(config->configfile) { @@ -632,6 +646,7 @@ static int parseargs(int argc, char *argv[]) {"print-format", required_argument, 0, OP_PRINTFORMAT}, {"gpgdir", required_argument, 0, OP_GPGDIR}, {"dbonly", no_argument, 0, OP_DBONLY}, + {"color", required_argument, 0, OP_COLOR}, {0, 0, 0, 0} }; -- 1.8.1.4

Andrew Gregory

2 Mar 2 Mar

4:32 a.m.

New subject: [pacman-dev] [PATCH 06/16] add a config settings and flag for colors

On 03/01/13 at 04:32pm, Simon Gomizelj wrote:

...

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 --- src/pacman/conf.c | 4 ++++ src/pacman/conf.h | 9 ++++++++- src/pacman/pacman.c | 15 +++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 218ffb4..dca6e3e 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -436,6 +436,10 @@ static int _parse_options(const char *key, char *value, pm_printf(ALPM_LOG_DEBUG, "config: totaldownload\n"); } else if(strcmp(key, "CheckSpace") == 0) { config->checkspace = 1; + } else if(strcmp(key, "Color") == 0) { + if(config->color == PM_COLOR_UNSET) { + config->color = isatty(fileno(stdout)) ? PM_COLOR_ON : PM_COLOR_OFF; + }

Should we allow an argument of always, never, auto here to match --color?

...

} else { pm_printf(ALPM_LOG_WARNING, _("config file %s, line %d: directive '%s' in section '%s' not recognized.\n"), diff --git a/src/pacman/conf.h b/src/pacman/conf.h index d85d11f..6cabd33 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -34,6 +34,7 @@ typedef struct __config_t { unsigned short print; unsigned short checkspace; unsigned short usesyslog; + unsigned short color; double deltaratio; char *arch; char *print_format; @@ -129,7 +130,8 @@ enum { OP_PRINTFORMAT, OP_GPGDIR, OP_DBONLY, - OP_FORCE + OP_FORCE, + OP_COLOR };

/* clean method */ @@ -145,6 +147,11 @@ enum { PKG_LOCALITY_FOREIGN = (1 << 1) };

+enum { + PM_COLOR_UNSET = 0, + PM_COLOR_OFF, + PM_COLOR_ON +};

/* global config variable */ extern config_t *config; diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 38b28e1..f565352 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -197,6 +197,7 @@ static void usage(int op, const char * const myname) addlist(_(" -v, --verbose be verbose\n")); addlist(_(" --arch <arch> set an alternate architecture\n")); addlist(_(" --cachedir <dir> set an alternate package cache location\n")); + addlist(_(" --color <when> colorize the output\n")); addlist(_(" --config <path> set an alternate configuration file\n")); addlist(_(" --debug display debug messages\n")); addlist(_(" --gpgdir <path> set an alternate home directory for GnuPG\n")); @@ -394,6 +395,19 @@ static int parsearg_global(int opt) check_optarg(); config->cachedirs = alpm_list_add(config->cachedirs, strdup(optarg)); break; + case OP_COLOR: + if (strcmp("never", optarg) == 0) { + config->color = PM_COLOR_OFF; + } else if (strcmp("auto", optarg) == 0) { + config->color = isatty(fileno(stdout)) ? PM_COLOR_ON : PM_COLOR_OFF; + } else if (strcmp("always", optarg) == 0) { + config->color = PM_COLOR_ON; + } else { + pm_printf(ALPM_LOG_ERROR, _("invalid agument '%s' for --color\n"), + optarg); + return 1; + }

You need to run init_colors() here.

...

+ break; case OP_CONFIG: check_optarg(); if(config->configfile) { @@ -632,6 +646,7 @@ static int parseargs(int argc, char *argv[]) {"print-format", required_argument, 0, OP_PRINTFORMAT}, {"gpgdir", required_argument, 0, OP_GPGDIR}, {"dbonly", no_argument, 0, OP_DBONLY}, + {"color", required_argument, 0, OP_COLOR}, {0, 0, 0, 0} };

-- 1.8.1.4

apg

Simon Gomizelj

4 Mar 4 Mar

10:22 p.m.

New subject: [pacman-dev] [PATCH 06/16] add a config settings and flag for colors

...

Should we allow an argument of always, never, auto here to match --color?

I initially considered to do that but decided against it. From my perspective, the only reason to ever use 'always' is when you want to preserve colour when piping and letting it be forced on at the config level will cause more headaches than its worth because users should be able to assume that piping pacman's output should just work. For example: pacman -Sl core > dumpfile Setting Color to always will pollute `dumpfile` with colour codes. We should be able to assume piped output will be clean (unless overridden). It makes much more sense to insist that forcing colours on must be explicit: pacman -Sl core --color=always | less -R

Allan McRae

6 Mar 6 Mar

3:28 p.m.

New subject: [pacman-dev] [PATCH 06/16] add a config settings and flag for colors

On 02/03/13 07:32, Simon Gomizelj wrote:

...

Colours can be enabled in two ways:

Good spelling of Colours!

...

- 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 --- src/pacman/conf.c | 4 ++++ src/pacman/conf.h | 9 ++++++++- src/pacman/pacman.c | 15 +++++++++++++++ 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 218ffb4..dca6e3e 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -436,6 +436,10 @@ static int _parse_options(const char *key, char *value, pm_printf(ALPM_LOG_DEBUG, "config: totaldownload\n"); } else if(strcmp(key, "CheckSpace") == 0) { config->checkspace = 1; + } else if(strcmp(key, "Color") == 0) { + if(config->color == PM_COLOR_UNSET) { + config->color = isatty(fileno(stdout)) ? PM_COLOR_ON : PM_COLOR_OFF; + } } else { pm_printf(ALPM_LOG_WARNING, _("config file %s, line %d: directive '%s' in section '%s' not recognized.\n"), diff --git a/src/pacman/conf.h b/src/pacman/conf.h index d85d11f..6cabd33 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -34,6 +34,7 @@ typedef struct __config_t { unsigned short print; unsigned short checkspace; unsigned short usesyslog; + unsigned short color; double deltaratio; char *arch; char *print_format; @@ -129,7 +130,8 @@ enum { OP_PRINTFORMAT, OP_GPGDIR, OP_DBONLY, - OP_FORCE + OP_FORCE, + OP_COLOR };

/* clean method */ @@ -145,6 +147,11 @@ enum { PKG_LOCALITY_FOREIGN = (1 << 1) };

+enum { + PM_COLOR_UNSET = 0, + PM_COLOR_OFF, + PM_COLOR_ON +};

/* global config variable */ extern config_t *config; diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 38b28e1..f565352 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -197,6 +197,7 @@ static void usage(int op, const char * const myname) addlist(_(" -v, --verbose be verbose\n")); addlist(_(" --arch <arch> set an alternate architecture\n")); addlist(_(" --cachedir <dir> set an alternate package cache location\n")); + addlist(_(" --color <when> colorize the output\n")); addlist(_(" --config <path> set an alternate configuration file\n")); addlist(_(" --debug display debug messages\n")); addlist(_(" --gpgdir <path> set an alternate home directory for GnuPG\n")); @@ -394,6 +395,19 @@ static int parsearg_global(int opt) check_optarg(); config->cachedirs = alpm_list_add(config->cachedirs, strdup(optarg)); break; + case OP_COLOR: + if (strcmp("never", optarg) == 0) { + config->color = PM_COLOR_OFF; + } else if (strcmp("auto", optarg) == 0) { + config->color = isatty(fileno(stdout)) ? PM_COLOR_ON : PM_COLOR_OFF; + } else if (strcmp("always", optarg) == 0) { + config->color = PM_COLOR_ON; + } else { + pm_printf(ALPM_LOG_ERROR, _("invalid agument '%s' for --color\n"),

spelling of "argument" and move "--color" out of the translation. In fact, change this to be similar to the output from config file parsing: _("invalid value for '%s' : '%s'\n"), "--color", optarg);

...

+ optarg); + return 1; + } + break; case OP_CONFIG: check_optarg(); if(config->configfile) { @@ -632,6 +646,7 @@ static int parseargs(int argc, char *argv[]) {"print-format", required_argument, 0, OP_PRINTFORMAT}, {"gpgdir", required_argument, 0, OP_GPGDIR}, {"dbonly", no_argument, 0, OP_DBONLY}, + {"color", required_argument, 0, OP_COLOR}, {0, 0, 0, 0} };

Simon Gomizelj

1 Mar 1 Mar

9:32 p.m.

New subject: [pacman-dev] [PATCH 07/16] introduce colstr for colourizing

colstr_t colstr will hold the colourizing agents. Signed-off-by: Simon Gomizelj --- src/pacman/conf.c | 36 ++++++++++++++++++++++++++++++++++++ src/pacman/conf.h | 7 +++++++ 2 files changed, 43 insertions(+) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index dca6e3e..34b4199 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -38,6 +38,40 @@ /* global config variable */ config_t *config = NULL; +colstr_t colstr; + +#define NC "\033[0m" + +#define BLACK "\033[0;30m" +#define RED "\033[0;31m" +#define GREEN "\033[0;32m" +#define YELLOW "\033[0;33m" +#define BLUE "\033[0;34m" +#define MAGENTA "\033[0;35m" +#define CYAN "\033[0;36m" +#define WHITE "\033[0;37m" + +#define BOLDBLACK "\033[1;30m" +#define BOLDRED "\033[1;31m" +#define BOLDGREEN "\033[1;32m" +#define BOLDYELLOW "\033[1;33m" +#define BOLDBLUE "\033[1;34m" +#define BOLDMAGENTA "\033[1;35m" +#define BOLDCYAN "\033[1;36m" +#define BOLDWHITE "\033[1;37m" + +static void init_colors(int colors) +{ + if(colors == PM_COLOR_ON) { + colstr.colon = BOLDBLUE "::" BOLDWHITE; + colstr.title = BOLDWHITE; + colstr.nc = NC; + } else { + colstr.colon = "::"; + colstr.title = ""; + colstr.nc = ""; + } +} config_t *config_new(void) { @@ -60,6 +94,7 @@ config_t *config_new(void) newconfig->remotefilesiglevel = ALPM_SIG_USE_DEFAULT; } + init_colors(PM_COLOR_OFF); return newconfig; } @@ -440,6 +475,7 @@ static int _parse_options(const char *key, char *value, if(config->color == PM_COLOR_UNSET) { config->color = isatty(fileno(stdout)) ? PM_COLOR_ON : PM_COLOR_OFF; } + init_colors(config->color); } else { pm_printf(ALPM_LOG_WARNING, _("config file %s, line %d: directive '%s' in section '%s' not recognized.\n"), diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 6cabd33..9207c6d 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -100,6 +100,12 @@ typedef struct __config_t { alpm_list_t *explicit_removes; } config_t; +typedef struct __colstr_t { + const char *colon; + const char *title; + const char *nc; +} colstr_t; + /* Operations */ enum { PM_OP_MAIN = 1, @@ -155,6 +161,7 @@ enum { /* global config variable */ extern config_t *config; +extern colstr_t colstr; config_t *config_new(void); int config_free(config_t *oldconfig); -- 1.8.1.4

Andrew Gregory

2 Mar 2 Mar

4:42 a.m.

New subject: [pacman-dev] [PATCH 07/16] introduce colstr for colourizing

On 03/01/13 at 04:32pm, Simon Gomizelj wrote:

...

colstr_t colstr will hold the colourizing agents.

Signed-off-by: Simon Gomizelj --- src/pacman/conf.c | 36 ++++++++++++++++++++++++++++++++++++ src/pacman/conf.h | 7 +++++++ 2 files changed, 43 insertions(+)

diff --git a/src/pacman/conf.c b/src/pacman/conf.c index dca6e3e..34b4199 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -38,6 +38,40 @@

/* global config variable */ config_t *config = NULL; +colstr_t colstr;

Why not add colstr as a member of config instead of adding another global variable?

...

+ +#define NC "\033[0m" + +#define BLACK "\033[0;30m" +#define RED "\033[0;31m" +#define GREEN "\033[0;32m" +#define YELLOW "\033[0;33m" +#define BLUE "\033[0;34m" +#define MAGENTA "\033[0;35m" +#define CYAN "\033[0;36m" +#define WHITE "\033[0;37m" + +#define BOLDBLACK "\033[1;30m" +#define BOLDRED "\033[1;31m" +#define BOLDGREEN "\033[1;32m" +#define BOLDYELLOW "\033[1;33m" +#define BOLDBLUE "\033[1;34m" +#define BOLDMAGENTA "\033[1;35m" +#define BOLDCYAN "\033[1;36m" +#define BOLDWHITE "\033[1;37m" + +static void init_colors(int colors) +{ + if(colors == PM_COLOR_ON) { + colstr.colon = BOLDBLUE "::" BOLDWHITE;

Why add "::" here instead of making this a plain color like all of the other members and adding "::" in colon_printf?

...

+ colstr.title = BOLDWHITE; + colstr.nc = NC; + } else { + colstr.colon = "::"; + colstr.title = ""; + colstr.nc = ""; + } +}

apg

Simon Gomizelj

4 Mar 4 Mar

10:28 p.m.

New subject: [pacman-dev] [PATCH 07/16] introduce colstr for colourizing

This makes implementing `colon_printf` easier. printf("%s ", colstr.colon); printf(fmt, args); ... instead of printf("%s::%s ", colstr.colon, colstr.title); printf(fmt, args); ... I can change it if you guys prefer but I figured setting colstr.colon like this would minimize the amount of work that needs to be done at pretty print time. (That said, to be consistent with my argument, I should probably add that whitespace to colstr.colon too.)

...

...
+static void init_colors(int colors) +{ + if(colors == PM_COLOR_ON) { + colstr.colon = BOLDBLUE "::" BOLDWHITE;

...

Why add "::" here instead of making this a plain color like all of the other members and adding "::" in colon_printf?

Simon Gomizelj

1 Mar 1 Mar

9:32 p.m.

New subject: [pacman-dev] [PATCH 08/16] colourize colon_printf

Signed-off-by: Simon Gomizelj --- src/pacman/util.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 6e06690..1cd955e 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1491,7 +1491,7 @@ static int question(short preset, const char *format, va_list args) fflush(stdout); fflush(stderr); - fprintf(stream, ":: "); + fprintf(stream, "%s ", colstr.colon); vfprintf(stream, format, args); if(preset) { @@ -1558,11 +1558,13 @@ int colon_printf(const char *fmt, ...) int ret; va_list args; - printf(":: "); va_start(args, fmt); + printf("%s ", colstr.colon); ret = vprintf(fmt, args); + printf("%s", colstr.nc); va_end(args); + fflush(stdout); return ret; } -- 1.8.1.4

Simon Gomizelj

9:32 p.m.

New subject: [pacman-dev] [PATCH 09/16] colourize warnings and errors

Signed-off-by: Simon Gomizelj --- src/pacman/conf.c | 4 ++++ src/pacman/conf.h | 2 ++ src/pacman/util.c | 4 ++-- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index 34b4199..de0df76 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -65,10 +65,14 @@ static void init_colors(int colors) if(colors == PM_COLOR_ON) { colstr.colon = BOLDBLUE "::" BOLDWHITE; colstr.title = BOLDWHITE; + colstr.warn = BOLDYELLOW; + colstr.err = BOLDRED; colstr.nc = NC; } else { colstr.colon = "::"; colstr.title = ""; + colstr.warn = "", + colstr.err = "", colstr.nc = ""; } } diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 9207c6d..0b015ef 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -103,6 +103,8 @@ typedef struct __config_t { typedef struct __colstr_t { const char *colon; const char *title; + const char *warn; + const char *err; const char *nc; } colstr_t; diff --git a/src/pacman/util.c b/src/pacman/util.c index 1cd955e..19ac795 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1661,10 +1661,10 @@ int pm_vfprintf(FILE *stream, alpm_loglevel_t level, const char *format, va_list /* print a prefix to the message */ switch(level) { case ALPM_LOG_ERROR: - fprintf(stream, _("error: ")); + fprintf(stream, "%s%s%s", colstr.err, _("error: "), colstr.nc); break; case ALPM_LOG_WARNING: - fprintf(stream, _("warning: ")); + fprintf(stream, "%s%s%s", colstr.warn, _("warning: "), colstr.nc); break; case ALPM_LOG_DEBUG: fprintf(stream, "debug: "); -- 1.8.1.4

Simon Gomizelj

9:32 p.m.

New subject: [pacman-dev] [PATCH 10/16] colourize table output

Signed-off-by: Simon Gomizelj --- src/pacman/util.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 19ac795..43afd5c 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -846,7 +846,8 @@ static void _print_table(struct table_row_t *rows, int spacing) for(row = rows; row && row->label; ++row) { const char *units; double s = humanize_size(row->size, 'M', 2, &units); - printf("%-*s %.2f %s\n", max_len, row->label, s, units); + printf("%s%-*s%s %.2f %s\n", colstr.title, max_len, row->label, + colstr.nc, s, units); } } -- 1.8.1.4

Simon Gomizelj

9:32 p.m.

New subject: [pacman-dev] [PATCH 11/16] have question properly reset itself

Signed-off-by: Simon Gomizelj --- src/pacman/util.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pacman/util.c b/src/pacman/util.c index 43afd5c..a190765 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1501,6 +1501,9 @@ static int question(short preset, const char *format, va_list args) fprintf(stream, " %s ", _("[y/N]")); } + fputs(colstr.nc, stream); + fflush(stream); + if(config->noconfirm) { fprintf(stream, "\n"); return preset; -- 1.8.1.4

Simon Gomizelj

9:32 p.m.

New subject: [pacman-dev] [PATCH 12/16] colorize the output if -Qi/Si

Signed-off-by: Simon Gomizelj --- src/pacman/package.c | 6 +++--- src/pacman/util.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/pacman/package.c b/src/pacman/package.c index 6daf745..1486f41 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -167,13 +167,13 @@ void dump_pkg_full(alpm_pkg_t *pkg, int extra) size = humanize_size(alpm_pkg_get_size(pkg), 'K', 2, &label); if(from == ALPM_PKG_FROM_SYNCDB) { - printf(_("Download Size : %6.2f %s\n"), size, label); + printf("%s%s%s %6.2f %s\n", colstr.title, "Download Size :", colstr.nc, size, label); } else if(from == ALPM_PKG_FROM_FILE) { - printf(_("Compressed Size: %6.2f %s\n"), size, label); + printf("%s%s%s %6.2f %s\n", colstr.title, "Compressed Size:", colstr.nc, size, label); } size = humanize_size(alpm_pkg_get_isize(pkg), 'K', 2, &label); - printf(_("Installed Size : %6.2f %s\n"), size, label); + printf("%s%s%s %6.2f %s\n", colstr.title, "Installed Size :", colstr.nc, size, label); string_display(_("Packager :"), alpm_pkg_get_packager(pkg), cols); string_display(_("Build Date :"), bdatestr, cols); diff --git a/src/pacman/util.c b/src/pacman/util.c index a190765..012318e 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -427,7 +427,7 @@ static size_t string_length(const char *s) void string_display(const char *title, const char *string, unsigned short cols) { if(title) { - printf("%s ", title); + printf("%s%s%s ", colstr.title, title, colstr.nc); } if(string == NULL || string[0] == '\0') { printf(_("None")); @@ -620,7 +620,7 @@ void list_display(const char *title, const alpm_list_t *list, if(title) { len = string_length(title) + 1; - printf("%s ", title); + printf("%s%s%s ", colstr.title, title, colstr.nc); } if(!list) { @@ -660,7 +660,7 @@ void list_display_linebreak(const char *title, const alpm_list_t *list, if(title) { len = (unsigned short)string_length(title) + 1; - printf("%s ", title); + printf("%s%s%s ", colstr.title, title, colstr.nc); } if(!list) { -- 1.8.1.4

Andrew Gregory

2 Mar 2 Mar

4:40 a.m.

New subject: [pacman-dev] [PATCH 12/16] colorize the output if -Qi/Si

On 03/01/13 at 04:32pm, Simon Gomizelj wrote:

...

Signed-off-by: Simon Gomizelj --- src/pacman/package.c | 6 +++--- src/pacman/util.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/pacman/package.c b/src/pacman/package.c index 6daf745..1486f41 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -167,13 +167,13 @@ void dump_pkg_full(alpm_pkg_t *pkg, int extra)

size = humanize_size(alpm_pkg_get_size(pkg), 'K', 2, &label); if(from == ALPM_PKG_FROM_SYNCDB) { - printf(_("Download Size : %6.2f %s\n"), size, label); + printf("%s%s%s %6.2f %s\n", colstr.title, "Download Size :", colstr.nc, size, label); } else if(from == ALPM_PKG_FROM_FILE) { - printf(_("Compressed Size: %6.2f %s\n"), size, label); + printf("%s%s%s %6.2f %s\n", colstr.title, "Compressed Size:", colstr.nc, size, label); }

size = humanize_size(alpm_pkg_get_isize(pkg), 'K', 2, &label); - printf(_("Installed Size : %6.2f %s\n"), size, label); + printf("%s%s%s %6.2f %s\n", colstr.title, "Installed Size :", colstr.nc, size, label);

This removes gettext translation for these labels.

...

string_display(_("Packager :"), alpm_pkg_get_packager(pkg), cols); string_display(_("Build Date :"), bdatestr, cols); diff --git a/src/pacman/util.c b/src/pacman/util.c index a190765..012318e 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -427,7 +427,7 @@ static size_t string_length(const char *s) void string_display(const char *title, const char *string, unsigned short cols) { if(title) { - printf("%s ", title); + printf("%s%s%s ", colstr.title, title, colstr.nc); } if(string == NULL || string[0] == '\0') { printf(_("None")); @@ -620,7 +620,7 @@ void list_display(const char *title, const alpm_list_t *list,

if(title) { len = string_length(title) + 1; - printf("%s ", title); + printf("%s%s%s ", colstr.title, title, colstr.nc); }

if(!list) { @@ -660,7 +660,7 @@ void list_display_linebreak(const char *title, const alpm_list_t *list,

if(title) { len = (unsigned short)string_length(title) + 1; - printf("%s ", title); + printf("%s%s%s ", colstr.title, title, colstr.nc); }

if(!list) { -- 1.8.1.4

Did you mean to leave "Backup Files:" (from -Qii) uncolored? apg

Simon Gomizelj

1 Mar 1 Mar

9:32 p.m.

New subject: [pacman-dev] [PATCH 13/16] colourize -Qs/-Ss

Signed-off-by: Simon Gomizelj --- src/pacman/conf.c | 28 ++++++++++++++++++---------- src/pacman/conf.h | 4 ++++ src/pacman/package.c | 13 +++++++------ 3 files changed, 29 insertions(+), 16 deletions(-) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index de0df76..165f021 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -63,17 +63,25 @@ colstr_t colstr; static void init_colors(int colors) { if(colors == PM_COLOR_ON) { - colstr.colon = BOLDBLUE "::" BOLDWHITE; - colstr.title = BOLDWHITE; - colstr.warn = BOLDYELLOW; - colstr.err = BOLDRED; - colstr.nc = NC; + colstr.colon = BOLDBLUE "::" BOLDWHITE; + colstr.title = BOLDWHITE; + colstr.repo = BOLDMAGENTA; + colstr.version = BOLDGREEN; + colstr.groups = BOLDBLUE; + colstr.meta = BOLDCYAN; + colstr.warn = BOLDYELLOW; + colstr.err = BOLDRED; + colstr.nc = NC; } else { - colstr.colon = "::"; - colstr.title = ""; - colstr.warn = "", - colstr.err = "", - colstr.nc = ""; + colstr.colon = "::"; + colstr.title = ""; + colstr.repo = ""; + colstr.version = ""; + colstr.groups = ""; + colstr.meta = ""; + colstr.warn = ""; + colstr.err = ""; + colstr.nc = ""; } } diff --git a/src/pacman/conf.h b/src/pacman/conf.h index 0b015ef..ec70c3c 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -103,6 +103,10 @@ typedef struct __config_t { typedef struct __colstr_t { const char *colon; const char *title; + const char *repo; + const char *version; + const char *groups; + const char *meta; const char *warn; const char *err; const char *nc; diff --git a/src/pacman/package.c b/src/pacman/package.c index 1486f41..db2cf76 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -349,9 +349,9 @@ void print_installed(alpm_db_t *db_local, alpm_pkg_t *pkg) if(lpkg) { const char *lpkgver = alpm_pkg_get_version(lpkg); if(strcmp(lpkgver, pkgver) == 0) { - printf(" [%s]", _("installed")); + printf(" %s[%s]%s", colstr.meta, _("installed"), colstr.nc); } else { - printf(" [%s: %s]", _("installed"), lpkgver); + printf(" %s[%s: %s]%s", colstr.meta, _("installed"), lpkgver, colstr.nc); } } } @@ -383,12 +383,13 @@ int dump_pkg_search(alpm_db_t *db, alpm_list_t *targets, alpm_db_t *installed_in 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)); + printf("%s%s/%s%s %s%s%s", colstr.repo, alpm_db_get_name(db), + colstr.title, alpm_pkg_get_name(pkg), + colstr.version, alpm_pkg_get_version(pkg), colstr.nc); if((grp = alpm_pkg_get_groups(pkg)) != NULL) { alpm_list_t *k; - fputs(" (", stdout); + printf(" %s(", colstr.groups); for(k = grp; k; k = alpm_list_next(k)) { const char *group = k->data; fputs(group, stdout); @@ -397,7 +398,7 @@ int dump_pkg_search(alpm_db_t *db, alpm_list_t *targets, alpm_db_t *installed_in putchar(' '); } } - putchar(')'); + printf(")%s", colstr.nc); } if (installed_in) -- 1.8.1.4

Simon Gomizelj

9:32 p.m.

New subject: [pacman-dev] [PATCH 14/16] colourize -Sl/-Ql

Signed-off-by: Simon Gomizelj --- src/pacman/package.c | 7 ++----- src/pacman/sync.c | 5 +++-- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/pacman/package.c b/src/pacman/package.c index db2cf76..cb4bb8b 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -303,12 +303,9 @@ void dump_pkg_files(alpm_pkg_t *pkg, int quiet) * Quiet : '<root><filepath>\n' */ if(!quiet) { - fputs(pkgname, stdout); - putchar(' '); + printf("%s%s%s ", colstr.title, pkgname, colstr.nc); } - fputs(root, stdout); - fputs(file->name, stdout); - putchar('\n'); + printf("%s%s\n", root, file->name); } fflush(stdout); diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 12e965b..a2248d3 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -523,8 +523,9 @@ static int sync_list(alpm_list_t *syncs, alpm_list_t *targets) alpm_pkg_t *pkg = j->data; if(!config->quiet) { - printf("%s %s %s", alpm_db_get_name(db), alpm_pkg_get_name(pkg), - alpm_pkg_get_version(pkg)); + printf("%s%s %s%s %s%s%s", colstr.repo, alpm_db_get_name(db), + colstr.title, alpm_pkg_get_name(pkg), + colstr.version, alpm_pkg_get_version(pkg), colstr.nc); print_installed(db_local, pkg); printf("\n"); } else { -- 1.8.1.4

Simon Gomizelj

9:32 p.m.

New subject: [pacman-dev] [PATCH 15/16] colourize -Q

Signed-off-by: Simon Gomizelj --- src/pacman/query.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/pacman/query.c b/src/pacman/query.c index 416f92c..39002e6 100644 --- a/src/pacman/query.c +++ b/src/pacman/query.c @@ -386,7 +386,8 @@ static int display(alpm_pkg_t *pkg) if(!config->op_q_info && !config->op_q_list && !config->op_q_changelog && !config->op_q_check) { if(!config->quiet) { - printf("%s %s\n", alpm_pkg_get_name(pkg), alpm_pkg_get_version(pkg)); + printf("%s%s %s%s%s\n", colstr.title, alpm_pkg_get_name(pkg), + colstr.version, alpm_pkg_get_version(pkg), colstr.nc); } else { printf("%s\n", alpm_pkg_get_name(pkg)); } -- 1.8.1.4

Simon Gomizelj

9:32 p.m.

New subject: [pacman-dev] [PATCH 16/16] update documentation and config files

Signed-off-by: Simon Gomizelj --- doc/pacman.8.txt | 3 +++ doc/pacman.conf.5.txt | 3 +++ etc/pacman.conf.in | 1 + 3 files changed, 7 insertions(+) diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 33a9421..7249d7d 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -146,6 +146,9 @@ Options and they are tried in the order they are passed to pacman. *NOTE*: this is an absolute path, the root path is not automatically prepended. +*\--color* <when>:: + Specify when to enable coloring, can be 'always', 'never' or 'auto'. + *\--config* <file>:: Specify an alternate configuration file. diff --git a/doc/pacman.conf.5.txt b/doc/pacman.conf.5.txt index aa28012..1ca3165 100644 --- a/doc/pacman.conf.5.txt +++ b/doc/pacman.conf.5.txt @@ -167,6 +167,9 @@ Options Log action messages through syslog(). This will insert log entries into +{localstatedir}/log/messages+ or equivalent. +*Color*:: + Automatically enable colors when pacman is outputting to a tty. + *UseDelta* [= ratio]:: Download delta files instead of complete packages if possible. Requires the `xdelta3` program to be installed. If a ratio is specified (e.g., diff --git a/etc/pacman.conf.in b/etc/pacman.conf.in index 4c72724..f610fa8 100644 --- a/etc/pacman.conf.in +++ b/etc/pacman.conf.in @@ -30,6 +30,7 @@ Architecture = auto # Misc options #UseSyslog +#Color #TotalDownload CheckSpace #VerbosePkgLists -- 1.8.1.4

Allan McRae

6 Mar 6 Mar

3:21 p.m.

New subject: [pacman-dev] [PATCH 16/16] update documentation and config files

On 02/03/13 07:32, Simon Gomizelj wrote:

...

Signed-off-by: Simon Gomizelj --- doc/pacman.8.txt | 3 +++ doc/pacman.conf.5.txt | 3 +++ etc/pacman.conf.in | 1 + 3 files changed, 7 insertions(+)

diff --git a/doc/pacman.8.txt b/doc/pacman.8.txt index 33a9421..7249d7d 100644 --- a/doc/pacman.8.txt +++ b/doc/pacman.8.txt @@ -146,6 +146,9 @@ Options and they are tried in the order they are passed to pacman. *NOTE*: this is an absolute path, the root path is not automatically prepended.

+*\--color* <when>:: + Specify when to enable coloring, can be 'always', 'never' or 'auto'. +

Clarify what auto does here. Allan

Allan McRae

4 Mar 4 Mar

11:33 a.m.

On 02/03/13 07:32, Simon Gomizelj wrote:

...

Colourize pacman output!

<snip> I had a quick look and did not see anything obvious to change beyond Andrew's comments. (But I could be tougher when I get more time...) Anyway - can you find somewhere to push you git repo with these patches once you make the needed changes? It would make my life a lot easier. Allan

Simon Gomizelj

10:31 p.m.

I put my repo up on github: https://github.com/vodik/pacman/commits/master I hope this is what you had in mind. On Mon, Mar 4, 2013 at 6:33 AM, Allan McRae wrote:

...

On 02/03/13 07:32, Simon Gomizelj wrote:

...
Colourize pacman output!

<snip>

I had a quick look and did not see anything obvious to change beyond Andrew's comments. (But I could be tougher when I get more time...)

Anyway - can you find somewhere to push you git repo with these patches once you make the needed changes? It would make my life a lot easier.

Allan

Simon Gomizelj

10:49 p.m.

I believe I addressed all the criticisms thus far. I'll just share this instead of spamming the mailing list with yet another patchset. https://github.com/vodik/pacman/compare/5770425...master On Mon, Mar 4, 2013 at 5:31 PM, Simon Gomizelj wrote:

...

I put my repo up on github: https://github.com/vodik/pacman/commits/master

I hope this is what you had in mind.

On Mon, Mar 4, 2013 at 6:33 AM, Allan McRae wrote:

...
On 02/03/13 07:32, Simon Gomizelj wrote:

...
Colourize pacman output!

<snip>

I had a quick look and did not see anything obvious to change beyond Andrew's comments. (But I could be tougher when I get more time...)

Anyway - can you find somewhere to push you git repo with these patches once you make the needed changes? It would make my life a lot easier.

Allan

Simon Gomizelj

10:51 p.m.

Derp: https://github.com/vodik/pacman/compare/8e26...master

Allan McRae

5 Mar 5 Mar

10:09 a.m.

On 05/03/13 08:51, Simon Gomizelj wrote:

...

Derp: https://github.com/vodik/pacman/compare/8e26...master

Great - I just grab a copy of your git repo so I may get a look on the plane, provided my battery holds out... If not, it might take me a couple of days. If anyone else wants to give this version a final review, that would be really helpful! Allan

Christian Hesse

3:19 p.m.

Simon Gomizelj on Mon, 2013/03/04 17:49:

...

I believe I addressed all the criticisms thus far.

I'll just share this instead of spamming the mailing list with yet another patchset.

https://github.com/vodik/pacman/compare/5770425...master

Tested your patches, works really well so far. I would like pacman to color the actions it performs. Messages printed by pre_*() and post_*() functions would be a lot more visible then. The attached patch is just a "quick and dirty proof of concept. -- main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH" "CX:;",b;for(a/* Chris get my mail address: */=0;b=c[a++];) putchar(b-1/(/* gcc -o sig sig.c && ./sig */b/42*2-3)*42);}

Dave Reisner

3:31 p.m.

On Tue, Mar 05, 2013 at 04:19:13PM +0100, Christian Hesse wrote:

...

Simon Gomizelj on Mon, 2013/03/04 17:49:

...
I believe I addressed all the criticisms thus far.

I'll just share this instead of spamming the mailing list with yet another patchset.

https://github.com/vodik/pacman/compare/5770425...master

Tested your patches, works really well so far.

I would like pacman to color the actions it performs. Messages printed by pre_*() and post_*() functions would be a lot more visible then.

The attached patch is just a "quick and dirty proof of concept.

I'm not really in favor of doing this. We're already labelling output in the log file which makes it stand out quite well on its own. I don't think we need to do this as well on terminal output. Example: [2013-03-04 16:50] [PACMAN] Running 'pacman -Su' [2013-03-04 16:50] [PACMAN] starting full system upgrade [2013-03-04 16:51] [PACMAN] upgraded libldap (2.4.33-3 -> 2.4.34-1) [2013-03-04 16:51] [ALPM-SCRIPTLET] >>> Updating module dependencies. Please wait ... [2013-03-04 16:51] [ALPM-SCRIPTLET] >>> Generating initial ramdisk, using mkinitcpio. Please wait... [2013-03-04 16:51] [ALPM-SCRIPTLET] ==> Building image from preset: /etc/mkinitcpio.d/linux.preset: 'default' [2013-03-04 16:51] [ALPM-SCRIPTLET] -> -k /boot/vmlinuz-linux -c /etc/mkinitcpio.conf -g /boot/initramfs-linux.img [2013-03-04 16:51] [ALPM-SCRIPTLET] ==> Starting build: 3.8.2-1-ARCH [2013-03-04 16:51] [ALPM-SCRIPTLET] -> Running build hook: [base] [2013-03-04 16:51] [ALPM-SCRIPTLET] -> Running build hook: [timestamp] [2013-03-04 16:51] [ALPM-SCRIPTLET] -> Running build hook: [strip] [2013-03-04 16:51] [ALPM-SCRIPTLET] -> Running build hook: [shutdown] [2013-03-04 16:51] [ALPM-SCRIPTLET] ==> Generating module dependencies [2013-03-04 16:51] [ALPM-SCRIPTLET] ==> Creating cat initcpio image: /boot/initramfs-linux.img [2013-03-04 16:51] [ALPM-SCRIPTLET] ==> Image generation successful [2013-03-04 16:51] [PACMAN] upgraded linux (3.8.1-1 -> 3.8.2-1) [2013-03-04 16:51] [PACMAN] upgraded linux-headers (3.8.1-1 -> 3.8.2-1)

...

-- main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH" "CX:;",b;for(a/* Chris get my mail address: */=0;b=c[a++];) putchar(b-1/(/* gcc -o sig sig.c && ./sig */b/42*2-3)*42);}

...

diff --git a/src/pacman/callback.c b/src/pacman/callback.c index d0887cb..468ea38 100644 --- a/src/pacman/callback.c +++ b/src/pacman/callback.c @@ -517,8 +517,9 @@ void cb_progress(alpm_progress_t event, const char *pkgname, int percent,

}

- printf("(%*ld/%*ld) %ls%-*s", digits, (unsigned long)current, - digits, (unsigned long)howmany, wcstr, padwid, ""); + const colstr_t *colstr = &config->colstr; + printf("(%*ld/%*ld) %s%ls%s%-*s", digits, (unsigned long)current, + digits, (unsigned long)howmany, colstr->title, wcstr, colstr->nc, padwid, "");

free(wcstr);

...

Christian Hesse

3:40 p.m.

Dave Reisner on Tue, 2013/03/05 10:31:

...

On Tue, Mar 05, 2013 at 04:19:13PM +0100, Christian Hesse wrote:

...
Simon Gomizelj on Mon, 2013/03/04 17:49:

...
I believe I addressed all the criticisms thus far.

I'll just share this instead of spamming the mailing list with yet another patchset.

https://github.com/vodik/pacman/compare/5770425...master

Tested your patches, works really well so far.

I would like pacman to color the actions it performs. Messages printed by pre_*() and post_*() functions would be a lot more visible then.

The attached patch is just a "quick and dirty proof of concept.

I'm not really in favor of doing this. We're already labelling output in the log file which makes it stand out quite well on its own. I don't think we need to do this as well on terminal output.

Ah, did not notice that... Really nice! (I would still see it highlighted on the terminal, but I am happy that way, too. ;) -- main(a){char*c=/* Schoene Gruesse */"B?IJj;MEH" "CX:;",b;for(a/* Chris get my mail address: */=0;b=c[a++];) putchar(b-1/(/* gcc -o sig sig.c && ./sig */b/42*2-3)*42);}

4076

Age (days ago)

4081

Last active (days ago)

List overview

Download

38 comments

5 participants

participants (5)

Allan McRae
Andrew Gregory
Christian Hesse
Dave Reisner
Simon Gomizelj

[pacman-dev] Colour support in pacman!

tags

participants (5)