Subject: Re: bug#58165: [Patch] [GNU ELPA] rcirc-color: Allow recoloring nicks

From: Philip Kaludercic <[email-masked]>
Date: Thu, 29 Sep 2022 21:02:27 +0000

Thuna <[email-masked]> writes:

> I think rcirc-color should allow a nick to be "recolored" with the
> command `/color nick', instead of signalling an error as it does now.
>
> This is useful, for example, when two similar nicks are assigned the
> same color but there's no specific color you want to reassign to either
> of them.
>
>>>From 4462df8aa6c0f9986629d5fab7df43e502552ba3 Mon Sep 17 00:00:00 2001
> From: Thuna <[email-masked]>
> Date: Thu, 29 Sep 2022 14:12:20 +0200
> Subject: [PATCH] rcirc-color: Select a random color on `/color nick'
>
> * rcirc-color.el: When `/color' is called with a nick but no color,
> choose a random color from `rcirc-colors' instead of signalling an
> error.
> ---
>  rcirc-color.el | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/rcirc-color.el b/rcirc-color.el
> index 2eff965267..7bcc46c91d 100644
> --- a/rcirc-color.el
> +++ b/rcirc-color.el
> @@ -134,7 +134,9 @@ NICK is the nick for which the new color ist set; if nil, all the
>  nicks in `rcirc-color-mapping' are shown with their corresponding
>  faces.
>  
> -COLOR is the color to use as the new foreground-color.
> +COLOR is the color to use as the new foreground-color.  If COLOR
> +is not supplied, a random color from `rcirc-colors' is used
> +instead.
>  
>  PROCESS and TARGET are the standard arguments for rcirc
>  commands."
> @@ -149,9 +151,12 @@ commands."
>                   rcirc-color-mapping)
>          (rcirc-print process (rcirc-nick process) "NOTICE" target
>                       (mapconcat 'identity names " ")))
> -    (unless color
> -      (error "Use what color?"))
> -    (puthash nick (cons 'foreground-color color) rcirc-color-mapping)))
> +    (puthash nick
> +             (cons 'foreground-color
> +                   (or color
> +                       (elt rcirc-colors
> +                            (random (length rcirc-colors)))))
> +             rcirc-color-mapping)))

While at it one could replace the `foreground-color', since (elisp)
Special Properties says:

        • A cons cell of the form ‘(foreground-color . COLOR-NAME)’ or
          ‘(background-color . COLOR-NAME)’.  This specifies the
          foreground or background color, similar to ‘(:foreground
          COLOR-NAME)’ or ‘(:background COLOR-NAME)’.  This form is
          supported for backward compatibility only, and should be
          avoided.

Also, it would be nice to also reformat the code using a let* block.
Something like

        (let* ((index (random (length rcirc-colors)))
               (color (elt rcirc-colors index))
               (face (cons 'foreground-color color)))
          (puthash nick face rcirc-color-mapping))

>  (advice-add 'rcirc-handler-NICK :before #'rcirc-color--handler-NICK)
>  (defun rcirc-color--handler-NICK (_process sender args _text)