bug#73580: 29.4; ERC 5.6.1-git: erc-nicks does not respect pal and fool faces

bug#73580: 29.4; ERC 5.6.1-git: erc-nicks does not respect pal and fool faces

[Trevor Arjeski]

[-- Attachment #1: Type: text/plain, Size: 799 bytes --]


The erc-nicks module does not respect erc-pal-face and erc-fool-face
when assigning a face for a nick, specifically when a user writes a
message that includes a nick of a pal or fool, leading to the faces
being different in the nick tag (<your_nick>) and the message
(ex. "your_nick: hi").

Reproduction config:

   (use-package erc
     :defer t
     :config
     (custom-set-faces
      `(erc-pal-face ((t (:foreground "red")))))
     (setopt erc-modules
             (seq-union '(nicks) erc-modules))
     :custom
     (erc-nicks-colors '("yellow"))
     (erc-pals '("trev")))
   
In the above config, you will notice that "<trev>" will be red, but when
someone mentions "trev" in a message, the nick will be yellow (instead
of red).

Attached is a proposed patch. Feedback needed and welcome!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: proposed patch for fix --]
[-- Type: text/x-patch, Size: 1361 bytes --]

From b4edb60f15a6b49c6b5443f0769a0d2cf33e1ff8 Mon Sep 17 00:00:00 2001
From: Trevor Arjeski <tmarjeski@gmail.com>
Date: Tue, 1 Oct 2024 18:21:02 +0300
Subject: [PATCH] Make erc-nicks respect pal and fool faces

The erc-nicks colors should not highlight over a nick that is a pal or a
fool.

This change checks if the nick is a pal or a fool and uses the respective face
in erc-nicks--face-table from there on out.
---
 erc-nicks.el | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/erc-nicks.el b/erc-nicks.el
index a0d6d17..e8258ef 100644
--- a/erc-nicks.el
+++ b/erc-nicks.el
@@ -67,6 +67,7 @@
 ;;; Code:
 
 (require 'erc-button)
+(require 'erc-match)
 (require 'color)
 
 (defgroup erc-nicks nil
@@ -464,6 +465,9 @@ Favor a custom erc-nicks-NICK@NETWORK-face when defined."
                                               (erc-network-name) "-face")))
                    ((or (and (facep face) face)
                         (erc-nicks--revive face face nick (erc-network))))))
+	(let ((face (or (when (erc-match-pal-p nick nil) 'erc-pal-face)
+                        (when (erc-match-fool-p nick nil) 'erc-fool-face))))
+          (puthash nick face table))
         (let ((color (erc-nicks--determine-color key))
               (new-face (make-symbol (concat "erc-nicks-" nick "-face"))))
           (put new-face 'erc-nicks--nick nick)
-- 
2.46.2


[-- Attachment #3: Type: text/plain, Size: 14920 bytes --]




In GNU Emacs 29.4 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.43,
cairo version 1.18.0)
Windowing system distributor 'The X.Org Foundation', version 11.0.12101013
System Description: Arch Linux

Configured using:
 'configure --with-x-toolkit=gtk3 --with-native-compilation=aot
 --sysconfdir=/etc --prefix=/usr --libexecdir=/usr/lib
 --with-tree-sitter --localstatedir=/var --with-cairo
 --disable-build-details --with-harfbuzz --with-libsystemd
 --with-modules 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt
 -fexceptions -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security
 -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer
 -mno-omit-leaf-frame-pointer -g
 -ffile-prefix-map=/build/emacs/src=/usr/src/debug/emacs -flto=auto'
 'LDFLAGS=-Wl,-O1 -Wl,--sort-common -Wl,--as-needed -Wl,-z,relro
 -Wl,-z,now -Wl,-z,pack-relative-relocs -flto=auto'
 'CXXFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fno-plt -fexceptions
 -Wp,-D_FORTIFY_SOURCE=3 -Wformat -Werror=format-security
 -fstack-clash-protection -fcf-protection -fno-omit-frame-pointer
 -mno-omit-leaf-frame-pointer -Wp,-D_GLIBCXX_ASSERTIONS -g
 -ffile-prefix-map=/build/emacs/src=/usr/src/debug/emacs -flto=auto''

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSYSTEMD LIBXML2 M17N_FLT MODULES NATIVE_COMP NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LC_ALL: en_US.UTF-8
  value of $LC_CTYPE: en_US.UTF-8
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: ELisp/l

Minor modes in effect:
  erc-track-mode: t
  erc-track-minor-mode: t
  erc-spelling-mode: t
  erc-ring-mode: t
  erc-netsplit-mode: t
  erc-menu-mode: t
  erc-match-mode: t
  erc-list-mode: t
  erc-irccontrols-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  erc-scrolltobottom-mode: t
  erc-imenu-mode: t
  erc-pcomplete-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-stamp-mode: t
  erc-autojoin-mode: t
  erc-networks-mode: t
  global-git-commit-mode: t
  shell-dirtrack-mode: t
  server-mode: t
  envrc-global-mode: t
  envrc-mode: t
  editorconfig-mode: t
  ws-butler-global-mode: t
  ws-butler-mode: t
  global-treesit-auto-mode: t
  diff-hl-flydiff-mode: t
  global-diff-hl-mode: t
  diff-hl-mode: t
  electric-pair-mode: t
  rainbow-delimiters-mode: t
  apheleia-global-mode: t
  apheleia-mode: t
  corfu-popupinfo-mode: t
  global-corfu-mode: t
  corfu-mode: t
  marginalia-mode: t
  vertico-mode: t
  which-key-mode: t
  global-ligature-mode: t
  ligature-mode: t
  global-auto-revert-mode: t
  global-display-line-numbers-mode: t
  display-line-numbers-mode: t
  desktop-save-mode: t
  recentf-mode: t
  save-place-mode: t
  straight-use-package-mode: t
  straight-package-neutering-mode: t
  override-global-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  column-number-mode: t
  line-number-mode: t
  auto-fill-function: do-auto-fill
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/trev/.emacs.d/straight/build/external-completion/external-completion hides /usr/share/emacs/29.4/lisp/external-completion
/home/trev/.emacs.d/straight/build/transient/transient hides /usr/share/emacs/29.4/lisp/transient
/home/trev/.emacs.d/straight/build/jsonrpc/jsonrpc hides /usr/share/emacs/29.4/lisp/jsonrpc
/home/trev/.emacs.d/straight/build/eglot/eglot hides /usr/share/emacs/29.4/lisp/progmodes/eglot
/home/trev/.emacs.d/straight/build/xref/xref hides /usr/share/emacs/29.4/lisp/progmodes/xref
/home/trev/.emacs.d/straight/build/project/project hides /usr/share/emacs/29.4/lisp/progmodes/project
/home/trev/.emacs.d/straight/build/flymake/flymake hides /usr/share/emacs/29.4/lisp/progmodes/flymake
/home/trev/.emacs.d/straight/build/erc/erc-button hides /usr/share/emacs/29.4/lisp/erc/erc-button
/home/trev/.emacs.d/straight/build/erc/erc-backend hides /usr/share/emacs/29.4/lisp/erc/erc-backend
/home/trev/.emacs.d/straight/build/erc/erc-ibuffer hides /usr/share/emacs/29.4/lisp/erc/erc-ibuffer
/home/trev/.emacs.d/straight/build/erc/erc-compat hides /usr/share/emacs/29.4/lisp/erc/erc-compat
/home/trev/.emacs.d/straight/build/erc/erc-capab hides /usr/share/emacs/29.4/lisp/erc/erc-capab
/home/trev/.emacs.d/straight/build/erc/erc hides /usr/share/emacs/29.4/lisp/erc/erc
/home/trev/.emacs.d/straight/build/erc/erc-status-sidebar hides /usr/share/emacs/29.4/lisp/erc/erc-status-sidebar
/home/trev/.emacs.d/straight/build/erc/erc-identd hides /usr/share/emacs/29.4/lisp/erc/erc-identd
/home/trev/.emacs.d/straight/build/erc/erc-replace hides /usr/share/emacs/29.4/lisp/erc/erc-replace
/home/trev/.emacs.d/straight/build/erc/erc-sasl hides /usr/share/emacs/29.4/lisp/erc/erc-sasl
/home/trev/.emacs.d/straight/build/erc/erc-speedbar hides /usr/share/emacs/29.4/lisp/erc/erc-speedbar
/home/trev/.emacs.d/straight/build/erc/erc-notify hides /usr/share/emacs/29.4/lisp/erc/erc-notify
/home/trev/.emacs.d/straight/build/erc/erc-pcomplete hides /usr/share/emacs/29.4/lisp/erc/erc-pcomplete
/home/trev/.emacs.d/straight/build/erc/erc-list hides /usr/share/emacs/29.4/lisp/erc/erc-list
/home/trev/.emacs.d/straight/build/erc/erc-autoaway hides /usr/share/emacs/29.4/lisp/erc/erc-autoaway
/home/trev/.emacs.d/straight/build/erc/erc-xdcc hides /usr/share/emacs/29.4/lisp/erc/erc-xdcc
/home/trev/.emacs.d/straight/build/erc/erc-networks hides /usr/share/emacs/29.4/lisp/erc/erc-networks
/home/trev/.emacs.d/straight/build/erc/erc-page hides /usr/share/emacs/29.4/lisp/erc/erc-page
/home/trev/.emacs.d/straight/build/erc/erc-truncate hides /usr/share/emacs/29.4/lisp/erc/erc-truncate
/home/trev/.emacs.d/straight/build/erc/erc-lang hides /usr/share/emacs/29.4/lisp/erc/erc-lang
/home/trev/.emacs.d/straight/build/erc/erc-sound hides /usr/share/emacs/29.4/lisp/erc/erc-sound
/home/trev/.emacs.d/straight/build/erc/erc-fill hides /usr/share/emacs/29.4/lisp/erc/erc-fill
/home/trev/.emacs.d/straight/build/erc/erc-loaddefs hides /usr/share/emacs/29.4/lisp/erc/erc-loaddefs
/home/trev/.emacs.d/straight/build/erc/erc-ring hides /usr/share/emacs/29.4/lisp/erc/erc-ring
/home/trev/.emacs.d/straight/build/erc/erc-join hides /usr/share/emacs/29.4/lisp/erc/erc-join
/home/trev/.emacs.d/straight/build/erc/erc-desktop-notifications hides /usr/share/emacs/29.4/lisp/erc/erc-desktop-notifications
/home/trev/.emacs.d/straight/build/erc/erc-stamp hides /usr/share/emacs/29.4/lisp/erc/erc-stamp
/home/trev/.emacs.d/straight/build/erc/erc-netsplit hides /usr/share/emacs/29.4/lisp/erc/erc-netsplit
/home/trev/.emacs.d/straight/build/erc/erc-goodies hides /usr/share/emacs/29.4/lisp/erc/erc-goodies
/home/trev/.emacs.d/straight/build/erc/erc-track hides /usr/share/emacs/29.4/lisp/erc/erc-track
/home/trev/.emacs.d/straight/build/erc/erc-ezbounce hides /usr/share/emacs/29.4/lisp/erc/erc-ezbounce
/home/trev/.emacs.d/straight/build/erc/erc-common hides /usr/share/emacs/29.4/lisp/erc/erc-common
/home/trev/.emacs.d/straight/build/erc/erc-imenu hides /usr/share/emacs/29.4/lisp/erc/erc-imenu
/home/trev/.emacs.d/straight/build/erc/erc-services hides /usr/share/emacs/29.4/lisp/erc/erc-services
/home/trev/.emacs.d/straight/build/erc/erc-spelling hides /usr/share/emacs/29.4/lisp/erc/erc-spelling
/home/trev/.emacs.d/straight/build/erc/erc-match hides /usr/share/emacs/29.4/lisp/erc/erc-match
/home/trev/.emacs.d/straight/build/erc/erc-menu hides /usr/share/emacs/29.4/lisp/erc/erc-menu
/home/trev/.emacs.d/straight/build/erc/erc-dcc hides /usr/share/emacs/29.4/lisp/erc/erc-dcc
/home/trev/.emacs.d/straight/build/erc/erc-log hides /usr/share/emacs/29.4/lisp/erc/erc-log
/home/trev/.emacs.d/straight/build/eldoc/eldoc hides /usr/share/emacs/29.4/lisp/emacs-lisp/eldoc
/home/trev/.emacs.d/straight/build/seq/seq hides /usr/share/emacs/29.4/lisp/emacs-lisp/seq

Features:
(shadow sort mail-extr emacsbug vc-hg vc-bzr vc-src vc-sccs vc-svn
vc-cvs vc-rcs bug-reference magit-patch magit-subtree magit-extras
magit-gitignore magit-ediff ediff ediff-merg ediff-mult ediff-wind
ediff-diff ediff-help ediff-init ediff-util face-remap debug backtrace
semantic/symref/grep grep semantic/symref semantic/util-modes
semantic/util semantic semantic/tag semantic/lex semantic/fw mode-local
cedet pulse dabbrev files-x cl-print erc-ibuffer erc-log erc-notify
erc-page erc-services erc-sound erc-speedbar speedbar ezimage dframe
erc-truncate erc-xdcc erc-dcc shortdoc help-fns radix-tree
network-stream nsm erc-track erc-spelling flyspell ispell erc-ring
erc-nicks erc-netsplit erc-menu erc-match erc-list erc-goodies erc-imenu
erc-pcomplete erc-button erc-fill erc-stamp erc-join cus-start epa-file
erc erc-backend erc-networks erc-common erc-compat erc-loaddefs
term/xterm xterm nerd-icons-dired nerd-icons nerd-icons-faces
nerd-icons-data nerd-icons-data-mdicon nerd-icons-data-flicon
nerd-icons-data-codicon nerd-icons-data-devicon nerd-icons-data-sucicon
nerd-icons-data-wicon nerd-icons-data-faicon nerd-icons-data-powerline
nerd-icons-data-octicon nerd-icons-data-pomicon nerd-icons-data-ipsicon
magit-bookmark magit-submodule magit-blame magit-stash magit-reflog
magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-tag
magit-merge magit-branch magit-reset magit-files magit-refs magit-status
magit package url-handlers magit-repos magit-apply magit-wip magit-log
which-func magit-diff smerge-mode git-commit log-edit message sendmail
yank-media puny rfc822 mml mml-sec epa derived epg rfc6068 epg-config
gnus-util mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047
rfc2045 mm-util ietf-drums mail-prsvr mailabbrev mail-utils gmm-utils
mailheader add-log magit-core magit-autorevert magit-margin
magit-transient magit-process with-editor shell server magit-mode
browse-url benchmark magit-git magit-base magit-section cursor-sensor
crm dash apheleia apheleia-rcs apheleia-dp apheleia-formatters
apheleia-utils apheleia-log apheleia-formatter-context casual-ibuffer
casual-ibuffer-filter ibuf-ext casual-ibuffer-settings
casual-ibuffer-version casual-ibuffer-utils ibuffer ibuffer-loaddefs
casual-dired casual-dired-settings dired-aux casual-dired-version
casual-dired-sort-by casual-dired-utils casual-dired-variables elint
checkdoc lisp-mnt thingatpt image-dired image-dired-tags
image-dired-external image-dired-util xdg image-mode exif wdired dired-x
dired dired-loaddefs casual-avy casual-avy-version casual-lib
casual-lib-version org ob ob-tangle ob-ref ob-lob ob-table ob-exp
org-macro org-src ob-comint org-pcomplete pcomplete org-list
org-footnote org-faces org-entities noutline outline ob-emacs-lisp
ob-core ob-eval org-cycle org-table ol org-fold org-fold-core org-keys
oc org-loaddefs find-func cal-menu calendar cal-loaddefs org-version
org-compat org-macs imenu transient format-spec envrc inheritenv
editorconfig editorconfig-core editorconfig-core-handle
editorconfig-fnmatch ws-butler treesit-auto treesit cape
corfu-doc-terminal avl-tree generator corfu-doc corfu-terminal popon
orderless consult bookmark undo-fu ace-window avy xref project compile
text-property-search comint ansi-osc ring comp comp-cstr warnings
mule-util jka-compr time-date diff-hl-flydiff diff diff-hl log-view
pcvs-util vc-dir ewoc vc rainbow-mode ansi-color color vc-git diff-mode
vc-dispatcher parinfer-rust-mode parinfer-rust-changes parinfer-rust
track-changes parinfer-rust-helper url url-proxy url-privacy url-expand
url-methods url-history url-cookie generate-lisp-file url-domsuf
url-util url-parse auth-source eieio eieio-core password-cache json map
url-vars mailcap elec-pair rainbow-delimiters init erc-init cus-edit pp
cus-load icons erc-autoloads markdown-mode-autoloads
geiser-guile-autoloads geiser-autoloads rust-mode-autoloads
parinfer-rust-mode-autoloads eglot-autoloads track-changes-autoloads
jsonrpc-autoloads flymake-autoloads project-autoloads
external-completion-autoloads eldoc-autoloads vterm-autoloads
nerd-icons-dired-autoloads nerd-icons-autoloads diff-hl-autoloads
magit-autoloads with-editor-autoloads magit-section-autoloads
dash-autoloads rainbow-mode-autoloads apheleia-autoloads
casual-ibuffer-autoloads casual-dired-autoloads casual-avy-autoloads
casual-lib-autoloads transient-autoloads envrc-autoloads
inheritenv-autoloads editorconfig-autoloads rainbow-delimiters-autoloads
ws-butler-autoloads treesit-auto-autoloads cape-autoloads
corfu-doc-terminal-autoloads corfu-doc-autoloads
corfu-terminal-autoloads popon-autoloads orderless-autoloads
corfu-popupinfo byte-opt corfu corfu-autoloads consult-autoloads
marginalia marginalia-autoloads vertico compat compat-30
vertico-autoloads compat-autoloads info seq-autoloads undo-fu-autoloads
ace-window-autoloads avy-autoloads which-key which-key-autoloads
xref-autoloads use-package-diminish diminish diminish-autoloads
doom-themes-ext-org doom-nord-theme pcase doom-themes doom-themes-base
doom-themes-autoloads finder-inf rx ligature ligature-autoloads
autorevert filenotify display-line-numbers desktop frameset recentf
tree-widget wid-edit saveplace edmacro kmacro straight-autoloads
straight subr-x cl-extra help-mode cl-macs gv use-package-bind-key
bind-key easy-mmode cl-seq use-package-core cl-loaddefs cl-lib bytecomp
byte-compile early-init rmc iso-transl tooltip cconv eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit xinput2 x multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 879903 1108074)
 (symbols 48 37842 200)
 (strings 32 188756 93623)
 (string-bytes 1 7685901)
 (vectors 16 93443)
 (vector-slots 8 2283381 1172880)
 (floats 8 774 3386)
 (intervals 56 25857 51054)
 (buffers 984 53))

bug#73580: 29.4; ERC 5.6.1-git: erc-nicks does not respect pal and fool faces

[Trevor Arjeski]

[-- Attachment #1: Type: text/plain, Size: 123 bytes --]


I did a bit more testing and realized that the problem is also occuring for erc-current-nick.

Here is an updated patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch fix v2 --]
[-- Type: text/x-patch, Size: 1486 bytes --]

From 88b01b15219d86aac2c8670f86d6001368bb04d1 Mon Sep 17 00:00:00 2001
From: Trevor Arjeski <tmarjeski@gmail.com>
Date: Tue, 1 Oct 2024 20:48:04 +0300
Subject: [PATCH] Make erc-nicks respect priority faces

The erc-nicks colors should not highlight over a nick that is a pal, fool or
current nick.

This change, when preparing a nick face, checks if a nick is a pal, fool or
current nick and uses the respective face in erc-nicks--face-table from there on
out.
---
 erc-nicks.el | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/erc-nicks.el b/erc-nicks.el
index a0d6d17..360b7fa 100644
--- a/erc-nicks.el
+++ b/erc-nicks.el
@@ -67,6 +67,7 @@
 ;;; Code:
 
 (require 'erc-button)
+(require 'erc-match)
 (require 'color)
 
 (defgroup erc-nicks nil
@@ -464,6 +465,10 @@ Favor a custom erc-nicks-NICK@NETWORK-face when defined."
                                               (erc-network-name) "-face")))
                    ((or (and (facep face) face)
                         (erc-nicks--revive face face nick (erc-network))))))
+        (let ((face (cond ((erc-match-pal-p nick t) 'erc-pal-face)
+                          ((erc-match-fool-p nick t) 'erc-fool-face)
+                          ((equal nick (erc-current-nick)) 'erc-my-nick-face))))
+          (puthash nick face table))
         (let ((color (erc-nicks--determine-color key))
               (new-face (make-symbol (concat "erc-nicks-" nick "-face"))))
           (put new-face 'erc-nicks--nick nick)
-- 
2.46.2

bug#73580: 29.4; ERC 5.6.1-git: erc-nicks does not respect pal and fool faces

[J.P.]

Trevor Arjeski <tmarjeski@gmail.com> writes:

> The erc-nicks module does not respect erc-pal-face and erc-fool-face
> when assigning a face for a nick, specifically when a user writes a
> message that includes a nick of a pal or fool, leading to the faces
> being different in the nick tag (<your_nick>) and the message
> (ex. "your_nick: hi").
>
> Reproduction config:
>
>    (use-package erc
>      :defer t
>      :config
>      (custom-set-faces
>       `(erc-pal-face ((t (:foreground "red")))))
>      (setopt erc-modules
>              (seq-union '(nicks) erc-modules))
>      :custom
>      (erc-nicks-colors '("yellow"))
>      (erc-pals '("trev")))
>    
> In the above config, you will notice that "<trev>" will be red, but when
> someone mentions "trev" in a message, the nick will be yellow (instead
> of red).
>
> Attached is a proposed patch. Feedback needed and welcome!

Thanks for the bug report. The good news is I can definitely reproduce
this. :)

But whether it's a bug and how to go about addressing it is a bit more
involved, I'm afraid. As luck would have it, this issue or something
similar actually comes up every now and again but not so much in the
context of `nicks' (or `erc-highlight-nicks' before it).

Anyway, as you may have noticed, the `nicks' module formally depends on
the `button' module and a new (5.6+) `button'-provided interface:
`erc-button--modify-nick-function'. IMO, this coupling is an acceptable
trade-off because `nicks' can piggyback on the token scanning that
`button' provides. The `button' module itself runs its code at depth 30
on `erc-insert-modify-hook', which is earlier than the `match' module's
50. This means it applies its faces _before_ `match' ever touches them.

What probably threw you off in perhaps thinking `match' had a say before
`nicks' was the presence of useless faces from `match' in the default
value of the option `erc-nicks-skip-faces'. That's indeed my bad: they
shouldn't be there at all (and a patch to fix this would be most
welcome). To that end, I'd much prefer we kept `nicks' and `match' as
loosely coupled as possible for the sake of long-term maintainability,
although I'm sure your current proposal is quite effective from a purely
pragmatic POV.

FWIW, the way `match' has always "worked" is that each category ("fool",
"keyword", etc.) has an accompanying "match type," like `keyword',
`nick', `message', etc., configured by an option like
`erc-current-nick-highlight-type'. However, unlike the "current-nick"
and "keyword" categories, "pal" and "fool" don't offer an equivalent of
`nick-or-keyword' or `keyword' for their corresponding highlight
options. This accounts for the undesirable behavior you observe (why
mentions aren't ever highlighted).

It likely won't surprise you to hear that expanding the match-type
selection of "fool" and "pal" to full parity with those of the other
categories has actually been suggested many times over in the past. In
fact, ISTR at least one patch/gist thingy floating around somewhere on
the wiki webs that purports to tackle this.

In terms of approaches, I think a more "modern" solution would address
this task by mimicking the way `nicks' modifies message text (but only
when `button' is loaded or slated to be via membership in
`erc-modules'). That is, it'd apply its faces indirectly by hooking into
`erc-button--modify-nick-function'. However, such an approach would also
likely involve more surgery (possibly a full refactor of
`erc-match-message'). And without adequate test coverage, I'd be
reticent to go that route. (BTW, the entire `match' module is severely
under-covered, so anything to remedy the situation would also be
welcome.)

As for a more traditional, non-`button' approach: I might be amenable to
a super minimal patch so long as it leaves a very light footprint. A
rough plan would be something like:

1. Add `keyword' and `nick-or-keyword' to highlight-type options for
   fools and pals.

2. Ensure the "-p" predicates for fools and pals consider the entire
   message. IIRC, only one of them does.

3. In, `erc-match-message', either modify the `keyword' case in the
   giant `cond' or add a new one for non-"current-nick" `keyword' and
   `nick-or-keyword' types to search and replace all pals and fools in
   the message body (possibly including the speaker tag, for the
   `nicks-or-keyword' variant).

(I'm probably missing something, but you get the idea.) If you're
willing to try this, I can help with benchmarking and adding tests. No
pressure though, obviously. Also, I very much appreciate all code
contributions, even those we don't end up using.


> I did a bit more testing and realized that the problem is also occuring for erc-current-nick.
>
> Here is an updated patch:
>
> From 88b01b15219d86aac2c8670f86d6001368bb04d1 Mon Sep 17 00:00:00 2001
> From: Trevor Arjeski <tmarjeski@gmail.com>
> Date: Tue, 1 Oct 2024 20:48:04 +0300
> Subject: [PATCH] Make erc-nicks respect priority faces
>
[...]
>  
>  (defgroup erc-nicks nil
> @@ -464,6 +465,10 @@ Favor a custom erc-nicks-NICK@NETWORK-face when defined."
>                                                (erc-network-name) "-face")))
>                     ((or (and (facep face) face)
>                          (erc-nicks--revive face face nick (erc-network))))))
> +        (let ((face (cond ((erc-match-pal-p nick t) 'erc-pal-face)
> +                          ((erc-match-fool-p nick t) 'erc-fool-face)
> +                          ((equal nick (erc-current-nick)) 'erc-my-nick-face))))
                                                               ~~~~~~~~~~~~~~~~

Actually, when I said I could reproduce the bug, I only meant WRT pals
and fools. This last face is used for so-called "input" messages, which
are outgoing messages taken from input at the prompt (which you probably
already knew). If you're saying `nicks' _should_ highlight your own
speaker tags (or should optionally do so), please explain.

Thanks.

bug#73580: 29.4; ERC 5.6.1-git: erc-nicks does not respect pal and fool faces

[Trevor Arjeski]

"J.P." <jp@neverwas.me> writes:

Agreed that it's not good to couple nicks and match.

> Thanks for the bug report. The good news is I can definitely reproduce
> this. :)
>
> But whether it's a bug and how to go about addressing it is a bit more
> involved, I'm afraid. As luck would have it, this issue or something
> similar actually comes up every now and again but not so much in the
> context of `nicks' (or `erc-highlight-nicks' before it).
>
> Anyway, as you may have noticed, the `nicks' module formally depends on
> the `button' module and a new (5.6+) `button'-provided interface:
> `erc-button--modify-nick-function'. IMO, this coupling is an acceptable
> trade-off because `nicks' can piggyback on the token scanning that
> `button' provides. The `button' module itself runs its code at depth 30
> on `erc-insert-modify-hook', which is earlier than the `match' module's
> 50. This means it applies its faces _before_ `match' ever touches
> them.

I see now. I did notice the depths being different but couldn't really
grok what was going on at the moment and went for more of a "hack it
till it works" approach.

> What probably threw you off in perhaps thinking `match' had a say before
> `nicks' was the presence of useless faces from `match' in the default
> value of the option `erc-nicks-skip-faces'. That's indeed my bad: they
> shouldn't be there at all (and a patch to fix this would be most
> welcome). To that end, I'd much prefer we kept `nicks' and `match' as
> loosely coupled as possible for the sake of long-term maintainability,
> although I'm sure your current proposal is quite effective from a purely
> pragmatic POV.

I did notice this and tinkered with it, but didn't really find anything
significant when removing the pal and fool faces from it.

Thanks for the detailed explanation. I did go through with the
"traditional" approach of adding 'nick-or-keyword, but sadly gave up.

Honestly, the more I looked at ERC, the more I realized that I don't
want pals to be highlighted at all. This makes my first patch irrelevant
since the whole idea is to use one single face color every time a nick
appears (similar to weechat's nick highlighting). The same goes for a
fool, who may appear dimmed, but when someone else mentions the fool you
will see their nick as the color that erc-nicks assigns.

> If you're saying `nicks' _should_ highlight your own
> speaker tags (or should optionally do so), please explain.

This is again about consistency with seeing your own nick within
brackets (<your_nick>) and in messages that other people send (when
erc-current-nick-highlight-type is 'nick-or-keyword).

Not sure what to do! I think we can just close this bug for now.

bug#73580: 29.4; ERC 5.6.1-git: erc-nicks does not respect pal and fool faces

[J.P.]

Trevor Arjeski <tmarjeski@gmail.com> writes:

> Thanks for the detailed explanation. I did go through with the
> "traditional" approach of adding 'nick-or-keyword, but sadly gave up.

Actually, I think I steered you wrong with that outline thingy
(apologies if that led to a wild goose chase). As you've rightly
observed, the only implementation of `nick-or-keyword' is for
"current-nick", and it behaves rather unintuitively (the one for
"keyword" is a no-op). From the doc:

  `nick-or-keyword' - highlight the nick of the user who typed your
                      nickname, or all instances of the current nickname
                      if there was no sending user

Anyway, mimicking that behavior for "fool" and "pal" obviously won't do.
I think we'd instead want a new variant, something like a `keyword-all'
to highlight all occurrences of a given nick, whether they be in a
speaker tag, an opening address "mention: ", or otherwise.

> Honestly, the more I looked at ERC, the more I realized that I don't
> want pals to be highlighted at all. This makes my first patch irrelevant
> since the whole idea is to use one single face color every time a nick
> appears (similar to weechat's nick highlighting). The same goes for a
> fool, who may appear dimmed, but when someone else mentions the fool you
> will see their nick as the color that erc-nicks assigns.

I believe what I said above applies here too re the either/or
proposition imposed by the "match types" currently on offer.

>> If you're saying `nicks' _should_ highlight your own
>> speaker tags (or should optionally do so), please explain.
>
> This is again about consistency with seeing your own nick within
> brackets (<your_nick>) and in messages that other people send (when
> erc-current-nick-highlight-type is 'nick-or-keyword).

Hm, I guess I was confused by the `erc-my-nick-face' in

  ((equal nick (erc-current-nick)) 'erc-my-nick-face))))

because that concerns the face your speaker tag appears in when you
submit a comment at the prompt. Confusingly, it's actually unrelated to
`erc-current-nick-face', which is the face your mentions appear in after
being treated by `match' (or, as noted above, the face your mentioners
appear in with match types `nick-or-keyword' and `nick').

To avoid such confusion in the future, I think we should supplement
those non-namespaced faces in erc-match.el with preferred aliases, like
`erc-match-current-nick' (new faces aren't supposed to be suffixed with
"-face", I've learned).

Anyway, FWIW, there are actually existing, though perhaps somewhat
roundabout ways to force all occurrences of your nick to use the same
face (if that's ultimately what you're after):

With `match':

  (use-package erc
    :custom
    (erc-modules `(nicks ,@erc-modules)))

  (use-package erc-match
    :custom-face
    (erc-current-nick-face ((t ( :weight unspecified
                                 :foreground unspecified
                                 :inherit erc-my-nick-face)))))

Without `match':

  (use-package erc
    :custom
    (erc-modules `(nicks ,@(remq 'match erc-modules))))

  (use-package erc-nicks
    :custom-face
    (erc-nicks-trev@Libera.Chat-face
     ((t (:inherit (erc-button-nick-default-face
                    erc-my-nick-face))))))

Perhaps one of these should go in the manual's Sample Config.


> Not sure what to do! I think we can just close this bug for now.

I've gone ahead and done that, but feel free to continue discussing
should anything related arise (though you may have to unarchive the bug
beforehand if weeks go by). Cheers.