[pacman-dev] [PATCH] scripts/library: fix typo in README
Simply fix a typo: in written -> is written Signed-off-by: Michael Straube <michael.straube@posteo.de> --- scripts/library/README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/library/README b/scripts/library/README index b99b0bc8..d20f117a 100644 --- a/scripts/library/README +++ b/scripts/library/README @@ -5,6 +5,6 @@ human_to_size.sh: A function to convert human readable sizes (such as "5.3 GiB") to raw byte equivalents. base10 and base2 suffixes are supported, case sensitively. If successful, the converted byte value is written to stdout and the function -returns 0. If an error occurs, nothing in written and the function returns 1. +returns 0. If an error occurs, nothing is written and the function returns 1. Results may be inaccurate when using a broken implementation of awk, such as mawk or busybox awk. -- 2.19.2
Colorize [installed] and [pending] when printing optional dependencies. FS#34920 Signed-off-by: Michael Straube <michael.straube@posteo.de> --- src/pacman/package.c | 3 +-- src/pacman/util.c | 14 ++++++++++++-- src/pacman/util.h | 1 + 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/pacman/package.c b/src/pacman/package.c index d8880837..d85a8834 100644 --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ -172,8 +172,7 @@ static void optdeplist_display(alpm_pkg_t *pkg, unsigned short cols) if(alpm_pkg_get_origin(pkg) == ALPM_PKG_FROM_LOCALDB) { if(alpm_find_satisfier(alpm_db_get_pkgcache(localdb), depstring)) { const char *installed = _(" [installed]"); - depstring = realloc(depstring, strlen(depstring) + strlen(installed) + 1); - strcpy(depstring + strlen(depstring), installed); + optstring_append_status(&depstring, installed); } } text = alpm_list_add(text, depstring); diff --git a/src/pacman/util.c b/src/pacman/util.c index 74e1037d..7e7e080e 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -1214,6 +1214,17 @@ static int depend_cmp(const void *d1, const void *d2) return ret; } +void optstring_append_status(char **optstring, const char *status) +{ + const colstr_t *colstr = &config->colstr; + size_t len; + + len = strlen(colstr->meta) + strlen(status) + strlen(colstr->nocolor); + *optstring = realloc(*optstring, strlen(*optstring) + len + 1); + snprintf(*optstring + strlen(*optstring), len + 1, "%s%s%s", + colstr->meta, status, colstr->nocolor); +} + static char *make_optstring(alpm_depend_t *optdep) { alpm_db_t *localdb = alpm_get_localdb(config->handle); @@ -1225,8 +1236,7 @@ static char *make_optstring(alpm_depend_t *optdep) status = _(" [pending]"); } if(status) { - optstring = realloc(optstring, strlen(optstring) + strlen(status) + 1); - strcpy(optstring + strlen(optstring), status); + optstring_append_status(&optstring, status); } return optstring; } diff --git a/src/pacman/util.h b/src/pacman/util.h index a1fbef46..0f0d506b 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -67,6 +67,7 @@ void signature_display(const char *title, alpm_siglist_t *siglist, unsigned short maxcols); void display_targets(void); int str_cmp(const void *s1, const void *s2); +void optstring_append_status(char **optstring, const char *status); void display_new_optdepends(alpm_pkg_t *oldpkg, alpm_pkg_t *newpkg); void display_optdepends(alpm_pkg_t *pkg); void print_packages(const alpm_list_t *packages); -- 2.19.2
On 10/12/18 3:31 am, Michael Straube wrote:
Colorize [installed] and [pending] when printing optional dependencies.
FS#34920
Signed-off-by: Michael Straube <michael.straube@posteo.de> ---
Thanks for this patch. I think there was a reason this bug was sitting untouched since 2013... To me, adding colour here is very distracting during an update: http://allanmcrae.com/tmp/color.png I'd prefer having less colours in our search operations, rather than bringing those colours into other operations. Anyone want to give other opinions on this? Allan
On 12 Dec 2018, at 11:31 am +1000, Allan McRae wrote:
On 10/12/18 3:31 am, Michael Straube wrote:
Colorize [installed] and [pending] when printing optional dependencies.
I'd prefer having less colours in our search operations, rather than bringing those colours into other operations.
Anyone want to give other opinions on this?
Agreed; I tend to prefer black-and-white output. Also, tools like lesspipe and multitail can colorize any program's stdout without having to add color support to the original program (or in this case, further complicate existing color support). Cheers, Ivy ("escondida")
Am 12.12.18 um 03:29 schrieb Ivy Foster:
On 12 Dec 2018, at 11:31 am +1000, Allan McRae wrote:
On 10/12/18 3:31 am, Michael Straube wrote:
Colorize [installed] and [pending] when printing optional dependencies.
I'd prefer having less colours in our search operations, rather than bringing those colours into other operations.
Anyone want to give other opinions on this?
Agreed; I tend to prefer black-and-white output. Also, tools like lesspipe and multitail can colorize any program's stdout without having to add color support to the original program (or in this case, further complicate existing color support).
Cheers, Ivy ("escondida")
I also think it can be irritating and/or distracting. In terms of simplicity I agree with Ivy. Well, the bug was on the list for v6.0, so I thought it was wanted to implement. ;) Michael
Change the warning message to reflect the reason when skipping duplicate targets. skipping target -> skipping duplicate target FS#49377 Signed-off-by: Michael Straube <michael.straube@posteo.de> --- src/pacman/remove.c | 2 +- src/pacman/sync.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pacman/remove.c b/src/pacman/remove.c index a2269ed8..f80514b4 100644 --- a/src/pacman/remove.c +++ b/src/pacman/remove.c @@ -46,7 +46,7 @@ static int remove_target(const char *target) alpm_errno_t err = alpm_errno(config->handle); if(err == ALPM_ERR_TRANS_DUP_TARGET) { /* just skip duplicate targets */ - pm_printf(ALPM_LOG_WARNING, _("skipping target: %s\n"), target); + pm_printf(ALPM_LOG_WARNING, _("skipping duplicate target: %s\n"), target); return 0; } else { pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", target, alpm_strerror(err)); diff --git a/src/pacman/sync.c b/src/pacman/sync.c index 57677a42..e2710a2d 100644 --- a/src/pacman/sync.c +++ b/src/pacman/sync.c @@ -523,7 +523,7 @@ static int process_pkg(alpm_pkg_t *pkg) alpm_errno_t err = alpm_errno(config->handle); if(err == ALPM_ERR_TRANS_DUP_TARGET) { /* just skip duplicate targets */ - pm_printf(ALPM_LOG_WARNING, _("skipping target: %s\n"), alpm_pkg_get_name(pkg)); + pm_printf(ALPM_LOG_WARNING, _("skipping duplicate target: %s\n"), alpm_pkg_get_name(pkg)); return 0; } else { pm_printf(ALPM_LOG_ERROR, "'%s': %s\n", alpm_pkg_get_name(pkg), -- 2.19.2
On 12/09/18 at 06:31pm, Michael Straube wrote:
Change the warning message to reflect the reason when skipping duplicate targets. skipping target -> skipping duplicate target
FS#49377
Signed-off-by: Michael Straube <michael.straube@posteo.de> --- src/pacman/remove.c | 2 +- src/pacman/sync.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Should we just remove the error altogether and move the message to DEBUG? The user doesn't need to do anything in response to it and I can't think of any reason a front-end would want to actually die from it. It seems to just be useless line noise that requires front-ends to check for it specifically just to ignore it.
Am 09.12.18 um 19:47 schrieb Andrew Gregory:
On 12/09/18 at 06:31pm, Michael Straube wrote:
Change the warning message to reflect the reason when skipping duplicate targets. skipping target -> skipping duplicate target
FS#49377
Signed-off-by: Michael Straube <michael.straube@posteo.de> --- src/pacman/remove.c | 2 +- src/pacman/sync.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Should we just remove the error altogether and move the message to DEBUG? The user doesn't need to do anything in response to it and I can't think of any reason a front-end would want to actually die from it. It seems to just be useless line noise that requires front-ends to check for it specifically just to ignore it.
Sounds reasonable. On the other hand I can imagine that some people would complain that too much from what's going on is hidden from the user. What do others think? P.S.: You mean? pm_printf(ALPM_LOG_DEBUG, _("skipping duplicate target: %s\n"), target);
On 12/11/18 at 06:14pm, Michael Straube wrote:
Am 09.12.18 um 19:47 schrieb Andrew Gregory:
On 12/09/18 at 06:31pm, Michael Straube wrote:
Change the warning message to reflect the reason when skipping duplicate targets. skipping target -> skipping duplicate target
FS#49377
Signed-off-by: Michael Straube <michael.straube@posteo.de> --- src/pacman/remove.c | 2 +- src/pacman/sync.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Should we just remove the error altogether and move the message to DEBUG? The user doesn't need to do anything in response to it and I can't think of any reason a front-end would want to actually die from it. It seems to just be useless line noise that requires front-ends to check for it specifically just to ignore it.
Sounds reasonable. On the other hand I can imagine that some people would complain that too much from what's going on is hidden from the user. What do others think?
P.S.: You mean? pm_printf(ALPM_LOG_DEBUG, _("skipping duplicate target: %s\n"), target);
I realized after sending this that adding a duplicate could actually be an error if they are two separate packages with the same name. So, I'm going to say to add a check in add_pkg for whether the duplicate is actually the same package (simple pointer cmp) and, if they are the same, log a debug message and return success, if they are different return the error as we do now. process_pkg in pacman should then treat that error just like any other instead of printing a warning and continuing on. alpm_remove_pkg should just log the debug message and return success. apg
Am 11.12.18 um 20:51 schrieb Andrew Gregory:
On 12/11/18 at 06:14pm, Michael Straube wrote:
Am 09.12.18 um 19:47 schrieb Andrew Gregory:
On 12/09/18 at 06:31pm, Michael Straube wrote:
Change the warning message to reflect the reason when skipping duplicate targets. skipping target -> skipping duplicate target
FS#49377
Signed-off-by: Michael Straube <michael.straube@posteo.de> --- src/pacman/remove.c | 2 +- src/pacman/sync.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Should we just remove the error altogether and move the message to DEBUG? The user doesn't need to do anything in response to it and I can't think of any reason a front-end would want to actually die from it. It seems to just be useless line noise that requires front-ends to check for it specifically just to ignore it.
Sounds reasonable. On the other hand I can imagine that some people would complain that too much from what's going on is hidden from the user. What do others think?
P.S.: You mean? pm_printf(ALPM_LOG_DEBUG, _("skipping duplicate target: %s\n"), target);
I realized after sending this that adding a duplicate could actually be an error if they are two separate packages with the same name. So, I'm going to say to add a check in add_pkg for whether the duplicate is actually the same package (simple pointer cmp) and, if they are the same, log a debug message and return success, if they are different return the error as we do now. process_pkg in pacman should then treat that error just like any other instead of printing a warning and continuing on. alpm_remove_pkg should just log the debug message and return success.
apg
Ok, I will send a patch the next days.
participants (4)
-
Allan McRae
-
Andrew Gregory
-
Ivy Foster
-
Michael Straube