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#69860: 29.2; ERC 5.6-git: erc: Incorrect face formatting applied for fg=99 bg=x (irccontrols module with erc-interpret-mirc-color=t) Date: Wed, 20 Mar 2024 07:27:55 -0700 Message-ID: <875xxhkmfo.fsf__13375.4314085067$1710944936$gmane$org@neverwas.me> References: <874jd4q0pb.fsf@tilde.club> <87wmq0srrz.fsf@neverwas.me> <87zfuwojpt.fsf@tilde.club> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="21330"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: 69860@debbugs.gnu.org, Alcor via General discussion about ERC To: Alcor Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Mar 20 15:28:48 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 1rmwvt-0005HX-IE for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 20 Mar 2024 15:28:47 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rmwvY-0003ek-Fq; Wed, 20 Mar 2024 10:28:24 -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 1rmwvW-0003eD-QM for bug-gnu-emacs@gnu.org; Wed, 20 Mar 2024 10:28:22 -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 1rmwvW-00084g-Eb for bug-gnu-emacs@gnu.org; Wed, 20 Mar 2024 10:28:22 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rmww9-0002ys-JS for bug-gnu-emacs@gnu.org; Wed, 20 Mar 2024 10:29:01 -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, 20 Mar 2024 14:29:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 69860 X-GNU-PR-Package: emacs Original-Received: via spool by 69860-submit@debbugs.gnu.org id=B69860.171094493211432 (code B ref 69860); Wed, 20 Mar 2024 14:29:01 +0000 Original-Received: (at 69860) by debbugs.gnu.org; 20 Mar 2024 14:28:52 +0000 Original-Received: from localhost ([127.0.0.1]:48009 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rmwvx-0002yB-7l for submit@debbugs.gnu.org; Wed, 20 Mar 2024 10:28:52 -0400 Original-Received: from mail-108-mta70.mxroute.com ([136.175.108.70]:46091) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rmwvs-0002xu-3W for 69860@debbugs.gnu.org; Wed, 20 Mar 2024 10:28:47 -0400 Original-Received: from filter006.mxroute.com ([136.175.111.2] filter006.mxroute.com) (Authenticated sender: mN4UYu2MZsgR) by mail-108-mta70.mxroute.com (ZoneMTA) with ESMTPSA id 18e5c42fff70003bea.001 for <69860@debbugs.gnu.org> (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384); Wed, 20 Mar 2024 14:27:58 +0000 X-Zone-Loop: d5d005c649034db259b75436dc981ce787e5ad410796 X-Originating-IP: [136.175.111.2] 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=7NZ6rRFtyn3+1vTo7yieV2G0/LCHiSqoddFvP+vKuKw=; b=Dcz/XXONvHfCK7wWrdSlTaVvUh cXTZEaBw/Xfawm81piugYDI9Gfui3Fc4bsutxu597FMvMXDaSBDstXhW07RmaFyRAaTi7ix98hrR5 J1hxboaTByx3ohMXa/cBCHIfVigSxCvx3Y6TxDdeeADoSGZOenpNsyT9ulFWk9ISwciXbcr0/+1z/ 0+GH1b3HCyBgsZ545cNjaiNJgAfc4Lw/l8P4aP576JW5ZsG9x6hjKghkhSIyhpNS/IEl/d0/bE1e4 +ELb4XUTw2TxOAIfqM4Xb2I/8Q4zdbUHjH4JkUKOlic9QT4LIK+IKbvtYHFuD6heUf4pi2T1/N/Wu EQmEdPRg==; In-Reply-To: <87zfuwojpt.fsf@tilde.club> (alcor@tilde.club's message of "Sun, 17 Mar 2024 18:23:58 +0100") 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:281863 Archived-At: --=-=-= Content-Type: text/plain Alcor writes: > "J.P." writes: > >> Oof. Looks like `erc-get-fg-color-face' sets `erc-control-default-bg' in >> its catch-all `cond' case. As you say, this produces: >> >> #("THIS TEXT IS FORMATTED" >> 0 22 (font-lock-face (erc-control-default-bg bg:erc-color-face4))) >> >> And `erc-get-bg-color-face' sets `erc-control-default-fg'. Clearly, >> whoever's responsible for this outrage should be banished. (Thanks.) > > Yes, that's part of the issue. However, `erc-controls-propertize' should > also avoid adding the default fg/bg to the font lock face if n=99 to > avoid overriding an existing fg,bg setting if fg=99,bg=x or bg=99,fg=x. Here's my current understanding of what you're saying. When there's an existing fg/bg combo in effect for a given span, and the parsing logic encounters a ^Cx,99 (or a ^C99,x), the 99 shouldn't induce a clobbering of the corresponding "incumbent" fg (or bg) face with a dedicated (and superfluous) default face but instead trigger the _removal_ of said incumbent face. This has the effect of falling through to honor the relevant attributes of the underlying `erc-default-face', which ships as a no-op. For example, given ^C03,08 hello ^C99,07 world the "hello " portion should be green on yellow and " world" should be ${default-foreground} (likely black or white) on orange. Likewise, if "^C99,07" were instead "^C04", then " world" should be red on yellow. This behavior aligns with that exhibited by Weechat and (I'm hoping) other popular clients. The revised patch set (attached) attempts to mimic this in ERC. Corrections or alternatives welcome. Thanks. --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0001-5.6-Remove-unused-faces-from-various-erc-goodies-tes.patch >From dd9302210706cc21ed1548a3a5ea769366c2e7be Mon Sep 17 00:00:00 2001 From: "F. Jason Park" Date: Tue, 19 Mar 2024 23:51:46 -0700 Subject: [PATCH 1/2] [5.6] Remove unused faces from various erc-goodies tests ; A note to anyone running ERC's test suite while bisecting and ; unlucky enough to land on this commit. Apologies for the ; inconvenience. It fails because it includes adjustments for fixes ; only introduced by the subsequent commit. This is obviously ; objectionable but was done knowingly in order to duck the ; copyright-exemption threshold for new contributors. * test/lisp/erc/erc-goodies-tests.el (erc-controls-highlight--spoilers) (erc-controls-highlight--inverse): Remove all mention of faces `erc-control-default-fg' and `erc-control-default-bg'. (erc-controls-highlight/default-foreground) (erc-controls-highlight/default-background): New tests. (Bug#69860) --- test/lisp/erc/erc-goodies-tests.el | 127 ++++++++++++++++++++++++++--- 1 file changed, 116 insertions(+), 11 deletions(-) diff --git a/test/lisp/erc/erc-goodies-tests.el b/test/lisp/erc/erc-goodies-tests.el index c8fb0544a72..7cbaa39d3f7 100644 --- a/test/lisp/erc/erc-goodies-tests.el +++ b/test/lisp/erc/erc-goodies-tests.el @@ -167,15 +167,13 @@ erc-controls-highlight--spoilers '(fg:erc-color-face1 bg:erc-color-face1)) ;; Masked in all black. (erc-goodies-tests--assert-face - 20 "BlackOnBlack" '(fg:erc-color-face1 bg:erc-color-face1) - '(erc-control-default-fg erc-control-default-bg)) + 20 "BlackOnBlack" '(fg:erc-color-face1 bg:erc-color-face1) nil) ;; Explicit "default" code ignoerd. (erc-goodies-tests--assert-face - 34 "Default" '(erc-control-default-fg erc-control-default-bg) + 34 "Default" '(erc-default-face) '(fg:erc-color-face1 bg:erc-color-face1)) (erc-goodies-tests--assert-face - 43 "END" 'erc-default-face - '(erc-control-default-bg erc-control-default-fg)))) + 43 "END" 'erc-default-face nil))) (when noninteractive (erc-tests-common-kill-buffers))) @@ -214,17 +212,124 @@ erc-controls-highlight--inverse nil) ;; The inverse of `default' because reverse still in effect. (erc-goodies-tests--assert-face - 32 "ReversedDefault" '(erc-inverse-face erc-control-default-fg - erc-control-default-bg) + 32 "ReversedDefault" '(erc-inverse-face erc-default-face) '(fg:erc-color-face3 bg:erc-color-face13)) (erc-goodies-tests--assert-face - 49 "NormalDefault" '(erc-control-default-fg - erc-control-default-bg) + 49 "NormalDefault" '(erc-default-face) '(erc-inverse-face fg:erc-color-face1 bg:erc-color-face1)) (erc-goodies-tests--assert-face 64 "END" 'erc-default-face - '( erc-control-default-fg erc-control-default-bg - fg:erc-color-face0 bg:erc-color-face0)))) + '(fg:erc-color-face0 bg:erc-color-face0)))) + (when noninteractive + (erc-tests-common-kill-buffers))) + +;; This is meant to assert two behavioral properties: +;; +;; 1) The background is preserved when only a new foreground is +;; defined, in accordance with this bit from the spec: "If only the +;; foreground color is set, the background color stays the same." +;; https://modern.ircdocs.horse/formatting#color +;; +;; 2) The same holds true for a new, lone foreground of 99. Rather +;; than prepend `erc-default-face', this causes the removal of an +;; existing foreground face and likewise doesn't clobber the +;; existing background. +(ert-deftest erc-controls-highlight/default-foreground () + (should (eq t erc-interpret-controls-p)) + (erc-tests-common-make-server-buf) + (with-current-buffer (erc--open-target "#chan") + (setq-local erc-interpret-mirc-color t) + (defvar erc-fill-column) + (let ((erc-fill-column 90)) + (erc-display-message nil nil (current-buffer) + (erc-format-privmessage + "bob" (concat "BEGIN " + "\C-c03,08 GreenOnYellow " + "\C-c99 BlackOnYellow " + "\C-o END") + nil t))) + (forward-line -1) + (should (search-forward " " nil t)) + (should (erc-tests-common-equal-with-props + (erc--remove-text-properties + (buffer-substring (point) (line-end-position))) + #("BEGIN GreenOnYellow BlackOnYellow END" + 0 6 (font-lock-face erc-default-face) + 6 21 (font-lock-face (fg:erc-color-face3 + bg:erc-color-face8 + erc-default-face)) + 21 36 (font-lock-face (bg:erc-color-face8 + erc-default-face)) + 36 40 (font-lock-face (erc-default-face))))) + (should (search-forward "BlackOnYellow")) + (let ((faces (get-text-property (point) 'font-lock-face))) + (should (equal (face-background (car faces) nil (cdr faces)) + "yellow"))) + + ;; Redefine background color alongside default foreground. + (let ((erc-fill-column 90)) + (erc-display-message nil nil (current-buffer) + (erc-format-privmessage + "bob" (concat "BEGIN " + "\C-c03,08 GreenOnYellow " + "\C-c99,07 BlackOnOrange " + "\C-o END") + nil t))) + (should (search-forward " " nil t)) + (should (erc-tests-common-equal-with-props + (erc--remove-text-properties + (buffer-substring (point) (line-end-position))) + #("BEGIN GreenOnYellow BlackOnOrange END" + 0 6 (font-lock-face erc-default-face) + 6 21 (font-lock-face (fg:erc-color-face3 + bg:erc-color-face8 + erc-default-face)) + 21 36 (font-lock-face (bg:erc-color-face7 + erc-default-face)) + 36 40 (font-lock-face (erc-default-face))))) + (should (search-forward "BlackOnOrange")) + (let ((faces (get-text-property (point) 'font-lock-face))) + (should (equal (face-background (car faces) nil (cdr faces)) + "orange")))) ; as opposed to white or black + (when noninteractive + (erc-tests-common-kill-buffers))) + +;; This merely asserts our current interpretation of "default faces": +;; that they reflect the foreground and background exhibited by normal +;; chat messages before any control-code formatting is applied (rather +;; than, e.g., some sort of negation or no-op). +(ert-deftest erc-controls-highlight/default-background () + (should (eq t erc-interpret-controls-p)) + (erc-tests-common-make-server-buf) + (with-current-buffer (erc--open-target "#chan") + (setq-local erc-interpret-mirc-color t) + (defvar erc-fill-column) + (let ((erc-fill-column 90)) + (erc-display-message nil nil (current-buffer) + (erc-format-privmessage + "bob" (concat "BEGIN " + "\C-c03,08 GreenOnYellow " + "\C-c05,99 BrownOnWhite " + "\C-o END") + nil t))) + (forward-line -1) + (should (search-forward " " nil t)) + (should (erc-tests-common-equal-with-props + (erc--remove-text-properties + (buffer-substring (point) (line-end-position))) + #("BEGIN GreenOnYellow BrownOnWhite END" + 0 6 (font-lock-face erc-default-face) + 6 21 (font-lock-face (fg:erc-color-face3 + bg:erc-color-face8 + erc-default-face)) + 21 35 (font-lock-face (fg:erc-color-face5 + erc-default-face)) + 35 39 (font-lock-face (erc-default-face))))) + ;; Ensure the background is white or black, rather than yellow. + (should (search-forward "BrownOnWhite")) + (let ((faces (get-text-property (point) 'font-lock-face))) + (should (equal (face-background (car faces) nil `(,@(cdr faces) default)) + (face-background 'default))))) (when noninteractive (erc-tests-common-kill-buffers))) -- 2.44.0 --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=0002-5.6-Remove-mishandled-erc-control-default-fg-bg-face.patch >From 22083686d58c4e43ab7ea05e0c54ff8ade2540f0 Mon Sep 17 00:00:00 2001 From: "F. Moukayed" Date: Sun, 17 Mar 2024 16:43:36 +0000 Subject: [PATCH 2/2] [5.6] Remove mishandled erc-control-default-{fg,bg} faces Partially revert the portions of 7b4ca9e609e "Leverage inverse-video for erc-inverse-face" that introduced explicit faces for the "default" 99 color code. * lisp/erc/erc-goodies.el (erc-control-default-fg) (erc-control-default-bg): Remove unused faces originally meant to be new in ERC 5.6. (erc-get-fg-color-face, erc-get-bg-color-face): Return nil for n=99. (erc-controls-interpret, erc-controls-highlight): Preserve an interval's existing background so that "if only the foreground color is set, the background color stays the same". See https://modern.ircdocs.horse/formatting#color. (Bug#69860) Copyright-paperwork-exempt: yes --- lisp/erc/erc-goodies.el | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el index da14f5bd728..883f64d3109 100644 --- a/lisp/erc/erc-goodies.el +++ b/lisp/erc/erc-goodies.el @@ -673,14 +673,6 @@ erc-underline-face "ERC underline face." :group 'erc-faces) -(defface erc-control-default-fg '((t :inherit default)) - "ERC foreground face for the \"default\" color code." - :group 'erc-faces) - -(defface erc-control-default-bg '((t :inherit default)) - "ERC background face for the \"default\" color code." - :group 'erc-faces) - ;; FIXME rename these to something like `erc-control-color-N-fg', ;; and deprecate the old names via `define-obsolete-face-alias'. (defface fg:erc-color-face0 '((t :foreground "White")) @@ -812,7 +804,7 @@ erc-get-bg-color-face (intern (concat "bg:erc-color-face" (number-to-string n)))) ((< 15 n 99) (list :background (aref erc--controls-additional-colors (- n 16)))) - (t (erc-log (format " Wrong color: %s" n)) 'erc-control-default-fg)))) + (t (erc-log (format " Wrong color: %s" n)) nil)))) (defun erc-get-fg-color-face (n) "Fetches the right face for foreground color N (0-15)." @@ -828,7 +820,7 @@ erc-get-fg-color-face (intern (concat "fg:erc-color-face" (number-to-string n)))) ((< 15 n 99) (list :foreground (aref erc--controls-additional-colors (- n 16)))) - (t (erc-log (format " Wrong color: %s" n)) 'erc-control-default-bg)))) + (t (erc-log (format " Wrong color: %s" n)) nil)))) ;;;###autoload(autoload 'erc-irccontrols-mode "erc-goodies" nil t) (define-erc-module irccontrols nil @@ -883,7 +875,7 @@ erc-controls-interpret (setq s (replace-match "" nil nil s 1)) (cond ((and erc-interpret-mirc-color (or fg-color bg-color)) (setq fg fg-color) - (setq bg bg-color)) + (when bg-color (setq bg bg-color))) ((string= control "\C-b") (setq boldp (not boldp))) ((string= control "\C-]") @@ -944,7 +936,7 @@ erc-controls-highlight (replace-match "" nil nil nil 1) (cond ((and erc-interpret-mirc-color (or fg-color bg-color)) (setq fg fg-color) - (setq bg bg-color)) + (when bg-color (setq bg bg-color))) ((string= control "\C-b") (setq boldp (not boldp))) ((string= control "\C-]") -- 2.44.0 --=-=-=--