diff mbox series

Diff color version strings

Message ID 20220110183840.13962-1-david.e.gustavsson@gmail.com
State Superseded, archived
Headers show
Series Diff color version strings | expand

Commit Message

David Gustavsson Jan. 10, 2022, 6:38 p.m. UTC 
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(-)

Comments

morganamilo Jan. 10, 2022, 7:01 p.m. UTC | #1 
On 10/01/2022 18:38, David Gustavsson wrote:
> 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 c0d62826..e998fa98 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 */
> @@ -1849,3 +1867,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)));


My suggestion was to use coloring that follows how alpm's version
comparison works, not just where the first change is.

You can find an example of the code here:
https://github.com/Morganamilo/paru/blob/501e0ba7d1bb356e415f07838057d2f73282bcb7/src/upgrade.rs#L49

Still these are just my comments and I know Allan quite dislikes colour
so I can't say if the patch would be accepted either way.
David Gustavsson Jan. 10, 2022, 9:35 p.m. UTC | #2 
Sorry, I misunderstood you. I know neither Rust nor Go, so I missed that
nuance.
A new patch is on its way. I'm not used to mailing list workflow, do you
want squashed patches or sequential commits?

Surely the fact that some people dislike colour is the reason there is a
`Color` option in the config, no?
Anyway, what is a patch submission but a very specific feature request?
I'll thank you for the comments and your good work, and cross my fingers.



Den mån 10 jan. 2022 kl 20:02 skrev Morgan Adamiec <
morganamilo@archlinux.org>:

>
>
> On 10/01/2022 18:38, David Gustavsson wrote:
> > 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 c0d62826..e998fa98 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 */
> > @@ -1849,3 +1867,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)));
>
>
> My suggestion was to use coloring that follows how alpm's version
> comparison works, not just where the first change is.
>
> You can find an example of the code here:
>
> https://github.com/Morganamilo/paru/blob/501e0ba7d1bb356e415f07838057d2f73282bcb7/src/upgrade.rs#L49
>
> Still these are just my comments and I know Allan quite dislikes colour
> so I can't say if the patch would be accepted either way.
>
diff mbox series

Patch

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 c0d62826..e998fa98 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 */
@@ -1849,3 +1867,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)));