From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: "J.P." Newsgroups: gmane.emacs.bugs Subject: bug#73580: 29.4; ERC 5.6.1-git: erc-nicks does not respect pal and fool faces Date: Tue, 01 Oct 2024 20:39:22 -0700 Message-ID: <87y137i36d.fsf__34405.3362192184$1727848061$gmane$org@neverwas.me> References: <87y13796p1.fsf@trevarch.mail-host-address-is-not-set> Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="20855"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: 73580@debbugs.gnu.org, emacs-erc@gnu.org To: Trevor Arjeski Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Oct 02 07:47:33 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1svsCy-0005Gm-Q0 for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 02 Oct 2024 07:47:33 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1svsCY-0004qC-9c; Wed, 02 Oct 2024 01:47:06 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1svsCU-0004gY-Ua for bug-gnu-emacs@gnu.org; Wed, 02 Oct 2024 01:47:03 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1svsCU-0004U7-LR for bug-gnu-emacs@gnu.org; Wed, 02 Oct 2024 01:47:02 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=MIME-Version:Date:References:In-Reply-To:From:To:Subject; bh=JcdpUzZcYkMdiZ3oBtFYsDHNGqTXETvRVDJT3b8Mfls=; b=tnrz/jtpjhnaW5cFkKNvW03d8s5jAhhSe8zGhopgYVus/zE1nxADNuMnecHEZD+EDGssTSNW3mCAHYx5DFtommpHlJnH/ll+8sKjiEaIOxyKl/w81IdJXd712fkbB0EYQn8A6kxpbsm2sueDhYInv1GWBufseooIbRI+oojUxMwYVdXVL3Wq0hDBmpY7vnW4V11yq6BPA6K7auwUuwDppXIM4tfEyeIsmQgfT3apT+Sw19j299tEsWXmoDZAUDLPbrJ0W6/nQmnfTBpZXZARKuwiqRGTKCSUpQqisM8QKEL6wFeVrK77hD4KSQnpUqSg4S/uy6ayBtRWjsm4ctBw+g==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1svsCU-0000ft-LO for bug-gnu-emacs@gnu.org; Wed, 02 Oct 2024 01:47:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: "J.P." Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 02 Oct 2024 05:47:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 73580 X-GNU-PR-Package: emacs Original-Received: via spool by 73580-submit@debbugs.gnu.org id=B73580.17278480082552 (code B ref 73580); Wed, 02 Oct 2024 05:47:02 +0000 Original-Received: (at 73580) by debbugs.gnu.org; 2 Oct 2024 05:46:48 +0000 Original-Received: from localhost ([127.0.0.1]:56295 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1svsCF-0000f5-1K for submit@debbugs.gnu.org; Wed, 02 Oct 2024 01:46:47 -0400 Original-Received: from relay3-s.mailbaby.net ([208.73.205.254]:60030) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1svsCC-0000en-68 for 73580@debbugs.gnu.org; Wed, 02 Oct 2024 01:46:45 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mailbaby.net; q=dns/txt; s=bambino; bh=JcdpUzZcYkMdiZ3oBtFYsDHNGqTXETvRVDJT3b8Mfls=; h=from:subject:date:message-id:to:cc:mime-version:content-type:in-reply-to:references:feedback-id; b=VPaz9L4yT7+Vs50DQd9TSuPC7JQl7xlUAD/smNESHSB6VMpQ2iAlK18vd+mORIsK+PAoU6mlC X0OXMQaXv+Pqp+WTCuq5Gs3N1DmKD9adfyY4hBPOIbLhPAs+B+a9ZBEGo/CnimVqXkcQa2YpYeq lPq52WJiOcbD5yfFxkzhY/w= Original-Received: from zmta1 ([45.76.59.163] 45.76.59.163.vultrusercontent.com) (Authenticated sender: mb25440) by relay3-s.mailbaby.net (MailBabyMTA) with ESMTPSA id 1924b5494e4000dfd0.001 for <73580@debbugs.gnu.org> (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384); Wed, 02 Oct 2024 03:44:47 +0000 X-Zone-Loop: 232440aa5e1469939066829a5cc4ec4f0c58f1d8584b X-MB-ID: mb25440 Feedback-ID: 25440:1924b5494e4000dfd0:45.76.59.163:mbaby Original-Received: from filter006.mxroute.com ([136.175.111.3] filter006.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by zmta1 (ZoneMTA) with ESMTPSA id 1924b4fabd10003e01.001 for <73580@debbugs.gnu.org> (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384); Wed, 02 Oct 2024 03:39:25 +0000 X-Zone-Loop: 94ed57269cf9388b5f46defbfa5e72285390ab0f287d X-Originating-IP: [136.175.111.3] DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=neverwas.me ; s=x; h=Content-Type:MIME-Version:Message-ID:Date:References:In-Reply-To: Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=JcdpUzZcYkMdiZ3oBtFYsDHNGqTXETvRVDJT3b8Mfls=; b=KbwZ1i3D3vFzaRuWIjd9agbfmq /1cB0/ngWEX1lFixO4UmhtT+v7BqVNzgSaVb+OM5IapL1RLWFRzhRKEBn5XgTwfwZHsp+uOTKU/UQ NrmjXljC5iZWCXdNHtx6aEG/NUi/gMVfxIWt8ODKjrh+wrJVOJjrl844QvJVmaRDOF6hTtuahlTD7 9S3iETFQGXlv3TmzS924Ps1hepl75CMFXry1u/J/JFrTl8LDED7xd69mVpqS9FeDov8FiHvTXUnau Dz9fe5TbtQj6LTjzgPQBTD97UJOHE4l2m2WySFv28jU79Pf9G9vHZCdPDqfKaP19yh/AkhWMckR+m 0PanM3Zg==; In-Reply-To: <87y13796p1.fsf@trevarch.mail-host-address-is-not-set> (Trevor Arjeski's message of "Tue, 01 Oct 2024 18:35:54 +0300") X-Authenticated-Id: masked@neverwas.me X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:292813 Archived-At: Trevor Arjeski 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 () 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 "" 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 > 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.