[PATCH 1/2] Diff color version strings
In package table, with VerbosePkgLists and Color, color the part of package version strings that changes. Signed-off-by: David Gustavsson <david.e.gustavsson@gmail.com> --- src/pacman/conf.c | 4 ++++ src/pacman/conf.h | 2 ++ src/pacman/util.c | 35 +++++++++++++++++++++++++++++++---- src/pacman/util.h | 1 + 4 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/pacman/conf.c b/src/pacman/conf.c index f9edf75b..d5c598b9 100644 --- a/src/pacman/conf.c +++ b/src/pacman/conf.c @@ -80,6 +80,8 @@ void enable_colors(int colors) colstr->err = BOLDRED; colstr->faint = GREY46; colstr->nocolor = NOCOLOR; + colstr->diffrem = RED; + colstr->diffadd = GREEN; } else { colstr->colon = ":: "; colstr->title = ""; @@ -91,6 +93,8 @@ void enable_colors(int colors) colstr->err = ""; colstr->faint = ""; colstr->nocolor = ""; + colstr->diffrem = ""; + colstr->diffadd = ""; } } diff --git a/src/pacman/conf.h b/src/pacman/conf.h index f7916ca9..782b5758 100644 --- a/src/pacman/conf.h +++ b/src/pacman/conf.h @@ -33,6 +33,8 @@ typedef struct __colstr_t { const char *err; const char *faint; const char *nocolor; + const char *diffrem; + const char *diffadd; } colstr_t; typedef struct __config_repo_t { diff --git a/src/pacman/util.c b/src/pacman/util.c index 53833d6f..10d7c573 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -840,8 +840,10 @@ static alpm_list_t *create_verbose_row(pm_target_t *target) { char *str; off_t size = 0; + size_t i = 0; double human_size; const char *label; + const char *version; alpm_list_t *ret = NULL; /* a row consists of the package name, */ @@ -858,12 +860,28 @@ static alpm_list_t *create_verbose_row(pm_target_t *target) add_table_cell(&ret, str, CELL_NORMAL | CELL_FREE); /* old and new versions */ - pm_asprintf(&str, "%s", - target->remove != NULL ? alpm_pkg_get_version(target->remove) : ""); + version = alpm_pkg_get_version(target->remove); + char old_version[strlen(version) + strlen(config->colstr.diffrem) + strlen(config->colstr.nocolor) + 1]; + memcpy(old_version, (char *)version, strlen(version) + 1); + version = alpm_pkg_get_version(target->install); + char new_version[strlen(version) + strlen(config->colstr.diffadd) + strlen(config->colstr.nocolor) + 1]; + memcpy(new_version, (char *)version, strlen(version) + 1); + + i = 0; + while (old_version[i] != '\0' && new_version[i] != '\0' && old_version[i] == new_version[i]) { + i++; + } + insertstring(old_version, config->colstr.diffrem, i); + insertstring(new_version, config->colstr.diffadd, i); + snprintf(old_version + i + strlen(old_version + i), + strlen(config->colstr.nocolor) + 1, "%s", config->colstr.nocolor); + snprintf(new_version + i + strlen(new_version + i), + strlen(config->colstr.nocolor) + 1, "%s", config->colstr.nocolor); + + pm_asprintf(&str, "%s", old_version); add_table_cell(&ret, str, CELL_NORMAL | CELL_FREE); - pm_asprintf(&str, "%s", - target->install != NULL ? alpm_pkg_get_version(target->install) : ""); + pm_asprintf(&str, "%s", new_version); add_table_cell(&ret, str, CELL_NORMAL | CELL_FREE); /* and size */ @@ -1859,3 +1877,12 @@ void console_erase_line(void) { printf("\x1B[K"); } + +/* inserts string insert into string at index i, modifying string */ +void insertstring(char *string, const char *insert, size_t i) +{ + char temp[strlen(string) - i + 1]; + memcpy(temp, string + i, strlen(string + i) + 1); + memcpy(string + i, insert, strlen(insert) + 1); + memcpy(string + i + strlen(insert), temp, strlen(temp) + 1); +} diff --git a/src/pacman/util.h b/src/pacman/util.h index 52e79915..003961eb 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -88,6 +88,7 @@ void console_cursor_move_down(unsigned int lines); void console_cursor_move_end(void); /* Erases line from the current cursor position till the end of the line */ void console_erase_line(void); +void insertstring(char *string, const char *insert, size_t i); 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))); -- 2.35.1
Use non-alphanumerics as branching points for diff coloring. Signed-off-by: David Gustavsson <david.e.gustavsson@gmail.com> --- src/pacman/util.c | 83 +++++++++++++++++++++++++++++++++-------------- src/pacman/util.h | 1 + 2 files changed, 60 insertions(+), 24 deletions(-) diff --git a/src/pacman/util.c b/src/pacman/util.c index 10d7c573..242af8b2 100644 --- a/src/pacman/util.c +++ b/src/pacman/util.c @@ -34,6 +34,7 @@ #include <limits.h> #include <wchar.h> #include <wctype.h> +#include <ctype.h> #ifdef HAVE_TERMIOS_H #include <termios.h> /* tcflush */ #endif @@ -840,10 +841,13 @@ static alpm_list_t *create_verbose_row(pm_target_t *target) { char *str; off_t size = 0; - size_t i = 0; + int split; double human_size; const char *label; - const char *version; + const char *old_version_in; + const char *new_version_in; + char *old_version; + char *new_version; alpm_list_t *ret = NULL; /* a row consists of the package name, */ @@ -859,24 +863,29 @@ static alpm_list_t *create_verbose_row(pm_target_t *target) } add_table_cell(&ret, str, CELL_NORMAL | CELL_FREE); + /* old and new versions */ - version = alpm_pkg_get_version(target->remove); - char old_version[strlen(version) + strlen(config->colstr.diffrem) + strlen(config->colstr.nocolor) + 1]; - memcpy(old_version, (char *)version, strlen(version) + 1); - version = alpm_pkg_get_version(target->install); - char new_version[strlen(version) + strlen(config->colstr.diffadd) + strlen(config->colstr.nocolor) + 1]; - memcpy(new_version, (char *)version, strlen(version) + 1); - - i = 0; - while (old_version[i] != '\0' && new_version[i] != '\0' && old_version[i] == new_version[i]) { - i++; - } - insertstring(old_version, config->colstr.diffrem, i); - insertstring(new_version, config->colstr.diffadd, i); - snprintf(old_version + i + strlen(old_version + i), - strlen(config->colstr.nocolor) + 1, "%s", config->colstr.nocolor); - snprintf(new_version + i + strlen(new_version + i), - strlen(config->colstr.nocolor) + 1, "%s", config->colstr.nocolor); + if (target->remove) { + old_version_in = alpm_pkg_get_version(target->remove); + } else { + old_version_in = ""; + } + old_version = malloc((strlen(old_version_in) + strlen(config->colstr.diffrem) + strlen(config->colstr.nocolor) + 1)*sizeof(char)); + memcpy(old_version, (char *)old_version_in, strlen(old_version_in) + 1); + + if (target->install) { + new_version_in = alpm_pkg_get_version(target->install); + } else { + new_version_in = ""; + } + new_version = malloc((strlen(new_version_in) + strlen(config->colstr.diffadd) + strlen(config->colstr.nocolor) + 1)*sizeof(char)); + memcpy(new_version, (char *)new_version_in, strlen(new_version_in) + 1); + + split = alpm_diff(old_version_in, new_version_in); + insertstring(old_version, config->colstr.diffrem, split); + insertstring(old_version, config->colstr.nocolor, strlen(old_version)); + insertstring(new_version, config->colstr.diffadd, split); + insertstring(new_version, config->colstr.nocolor, strlen(new_version)); pm_asprintf(&str, "%s", old_version); add_table_cell(&ret, str, CELL_NORMAL | CELL_FREE); @@ -884,6 +893,9 @@ static alpm_list_t *create_verbose_row(pm_target_t *target) pm_asprintf(&str, "%s", new_version); add_table_cell(&ret, str, CELL_NORMAL | CELL_FREE); + free(old_version); + free(new_version); + /* and size */ size -= target->remove ? alpm_pkg_get_isize(target->remove) : 0; size += target->install ? alpm_pkg_get_isize(target->install) : 0; @@ -1878,11 +1890,34 @@ void console_erase_line(void) printf("\x1B[K"); } -/* inserts string insert into string at index i, modifying string */ +/* inserts string insert into string after index i, modifying string */ void insertstring(char *string, const char *insert, size_t i) { - char temp[strlen(string) - i + 1]; - memcpy(temp, string + i, strlen(string + i) + 1); - memcpy(string + i, insert, strlen(insert) + 1); - memcpy(string + i + strlen(insert), temp, strlen(temp) + 1); + char *temp = malloc((strlen(string) - i)*sizeof(char)); + memcpy(temp, string + i + 1, strlen(string + i)); + memcpy(string + i + 1, insert, strlen(insert) + 1); + memcpy(string + i + strlen(insert) + 1, temp, strlen(temp) + 1); + free(temp); +} + +int alpm_diff(const char *a, const char *b) +{ + /* + * Find the index of the last non-alphanumeric character + * before `a` and `b` differ + */ + int i = 0; + int split = -1; + while(a[i] == b[i]) { + if(a[i] == '\0') { + split = i; + return split; + } + if(isalnum(a[i]) == 0) { + /* Non-alphanumeric character */ + split = i; + } + i++; + } + return split; } diff --git a/src/pacman/util.h b/src/pacman/util.h index 003961eb..afac74c1 100644 --- a/src/pacman/util.h +++ b/src/pacman/util.h @@ -89,6 +89,7 @@ void console_cursor_move_end(void); /* Erases line from the current cursor position till the end of the line */ void console_erase_line(void); void insertstring(char *string, const char *insert, size_t i); +int alpm_diff(const char *a, const char *b); 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))); -- 2.35.1
On 1/3/22 15:02, David Gustavsson wrote:
Use non-alphanumerics as branching points for diff coloring.
Signed-off-by: David Gustavsson <david.e.gustavsson@gmail.com> ---
There is a memory leak somewhere in here: $ sudo ./build/pacman -Su :: Starting full system upgrade... resolving dependencies... looking for conflicting packages... free(): invalid next size (fast) Aborted I have not looked for it yet, because I am still deciding the utility of these patches. For discussion, consider these versions: 5.18.14.arch1-1 5.18.15.arch1-1 With the first patch in this series applied (have not looked to see if this changes with the second), this starts highlighting after "5.18.1". But if we were going from 5.18.9 to 5.18.10, it would highlight after the "5.18.". These upgrades are no different in that versioning scheme. This should definitely always highlight from the version level separator. Also, I personally do not find this colouring useful, but I also do not find the verbose package lists before update useful... So I am open to opinions on this. Here is an example of the output after the patch 1/2 applied: http://allanmcrae.com/tmp/pacman-version-change-highlight.png I'll wait for more feedback before looking into this further. Allan
The memory leak was an off-by-one-error, I can't reproduce it after removing some needless pointer arithmetic. Yes, I had some constructive pointers about semantic diffing between the two previous patches. The attached patch looks like this: [image: 2022-08-18T155817.png] Your examples would go 5.18.*14.arch1-1* 5.18.*15arch1-1* 5.18.*9* 5.18.*10* To me, this is very useful. Especially when there's many rows, it helps when figuring out which release notes to read after an upgrade. Disabling `color` or `verbose` of course removes this. Den sön 31 juli 2022 kl 07:05 skrev Allan McRae <allan@archlinux.org>:
On 1/3/22 15:02, David Gustavsson wrote:
Use non-alphanumerics as branching points for diff coloring.
Signed-off-by: David Gustavsson <david.e.gustavsson@gmail.com> ---
There is a memory leak somewhere in here:
$ sudo ./build/pacman -Su :: Starting full system upgrade... resolving dependencies... looking for conflicting packages... free(): invalid next size (fast) Aborted
I have not looked for it yet, because I am still deciding the utility of these patches.
For discussion, consider these versions:
5.18.14.arch1-1 5.18.15.arch1-1
With the first patch in this series applied (have not looked to see if this changes with the second), this starts highlighting after "5.18.1". But if we were going from 5.18.9 to 5.18.10, it would highlight after the "5.18.". These upgrades are no different in that versioning scheme. This should definitely always highlight from the version level separator.
Also, I personally do not find this colouring useful, but I also do not find the verbose package lists before update useful... So I am open to opinions on this. Here is an example of the output after the patch 1/2 applied: http://allanmcrae.com/tmp/pacman-version-change-highlight.png
I'll wait for more feedback before looking into this further.
Allan
participants (2)
-
Allan McRae
-
David Gustavsson