GNU bug report logs - #58165
[Patch] [GNU ELPA] rcirc-color: Allow recoloring nicks

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 58165 in the body.
You can then email your comments to 58165 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Thuna <thuna.cing <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [Patch] [GNU ELPA] rcirc-color: Allow recoloring nicks
Date: Thu, 29 Sep 2022 16:58:21 +0200

[Message part 1 (text/plain, inline)]

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.

[0001-rcirc-color-Select-a-random-color-on-color-nick.patch (text/x-patch, attachment)]

Message #8 received at 58165 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Thuna <thuna.cing <at> gmail.com>
Cc: 58165 <at> debbugs.gnu.org
Subject: Re: bug#58165: [Patch] [GNU ELPA] rcirc-color: Allow recoloring nicks
Date: Thu, 29 Sep 2022 21:02:27 +0000

Thuna <thuna.cing <at> gmail.com> 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 <thuna.cing <at> gmail.com>
> 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)

Message #11 received at 58165 <at> debbugs.gnu.org (full text, mbox):

From: Thuna <thuna.cing <at> gmail.com>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 58165 <at> debbugs.gnu.org
Subject: Re: bug#58165: [Patch] [GNU ELPA] rcirc-color: Allow recoloring nicks
Date: Fri, 30 Sep 2022 02:30:22 +0200

> 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.

I avoided it on the off chance something would break, but I don't mind
either way.

> 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))

Seems ok to me.

Message #16 received at 58165-done <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Thuna <thuna.cing <at> gmail.com>
Cc: 58165-done <at> debbugs.gnu.org
Subject: Re: bug#58165: [Patch] [GNU ELPA] rcirc-color: Allow recoloring nicks
Date: Fri, 30 Sep 2022 11:24:23 +0000

Thuna <thuna.cing <at> gmail.com> writes:

>> 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.
>
> I avoided it on the off chance something would break, but I don't mind
> either way.

I think I can do this.

>> 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))
>
> Seems ok to me.

I'll pull your patch and make this change in a subsequent commit.

So thank you for the patch.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.