GNU bug report logs - #69597
29.2; ERC 5.6-git: Add a new customizable variable controlling how Erc displays spoilers

Previous Next

Package: emacs;

Reported by: Fadi Moukayed <smfadi <at> gmail.com>

Date: Wed, 6 Mar 2024 21:48:02 UTC

Severity: wishlist

Tags: patch

Found in version 29.2

Done: "J.P." <jp <at> neverwas.me>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#69597; Package emacs. (Wed, 06 Mar 2024 21:48:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Fadi Moukayed <smfadi <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Wed, 06 Mar 2024 21:48:02 GMT) Full text and rfc822 format available.

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

From: Fadi Moukayed <smfadi <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Cc: emacs-erc <at> gnu.org
Subject: 29.2; ERC 5.6-git: Add a new customizable variable controlling how
 Erc displays spoilers
Date: Wed, 6 Mar 2024 20:26:36 +0100
[Message part 1 (text/plain, inline)]
Tags: patch
Severity: wishlist

Hi all,

I am submitting a trivial patch adding a simple customizable variable
(erc-format-spoilers) to Erc, allowing the user to control how Erc
displays spoiler text when mIRC formatting code interpretation is
enabled.
This idea for this patch was discussed with the Erc maintainers on
#erc, and was deemed to be useful enough to warrant a submission.

Note that this is my first attempt to contribute to GNU Emacs.

In GNU Emacs 29.2 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.20,
 cairo version 1.16.0) of 2024-02-27 built on lcy02-amd64-095
Repository revision: ac89b1141a261b40ab5607f8d74209ede4c164cc
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12201001
System Description: Ubuntu 22.04.4 LTS

Configured using:
 'configure --prefix=/snap/emacs/current/usr --with-x-toolkit=gtk3
 --without-xaw3d --with-modules --with-cairo
 --with-native-compilation=aot --without-pgtk --with-xinput2
 --with-tree-sitter --with-json
 'CFLAGS=-isystem/build/emacs/parts/emacs/install/usr/include
 -isystem/build/emacs/parts/emacs/install/usr/include/x86_64-linux-gnu
 -isystem/build/emacs/stage/usr/include -O2'
 'CPPFLAGS=-isystem/build/emacs/parts/emacs/install/usr/include
 -isystem/build/emacs/parts/emacs/install/usr/include/x86_64-linux-gnu
 -isystem/build/emacs/stage/usr/include'
 'LDFLAGS=-L/build/emacs/parts/emacs/install/lib
 -L/build/emacs/parts/emacs/install/usr/lib
 -L/build/emacs/parts/emacs/install/lib/x86_64-linux-gnu
 -L/build/emacs/parts/emacs/install/usr/lib/x86_64-linux-gnu
 -L/build/emacs/stage/usr/lib''

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

Important settings:
  value of $LANG: de_DE.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: ERC

Minor modes in effect:
  erc-ring-mode: t
  erc-notifications-mode: t
  erc-nicks-mode: t
  erc-netsplit-mode: t
  erc-menu-mode: t
  erc-list-mode: t
  erc-irccontrols-mode: t
  erc-keep-place-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--phantom-users-mode: t
  erc-button-mode: t
  erc-fill-wrap-mode: t
  erc-fill-mode: t
  erc-stamp--date-mode: t
  erc-stamp--display-margin-mode: t
  erc-stamp-mode: t
  erc-bufbar-mode: t
  erc-track-mode: (t erc-nicks--setup-track-integration)
  erc-track-minor-mode: t
  erc-match-mode: t
  erc-autojoin-mode: t
  erc-autoaway-mode: t
  recentf-mode: t
  pixel-scroll-precision-mode: t
  minibuffer-depth-indicate-mode: t
  global-whitespace-mode: t
  global-goto-address-mode: t
  goto-address-mode: t
  global-auto-revert-mode: t
  fido-vertical-mode: t
  icomplete-vertical-mode: t
  icomplete-mode: t
  fido-mode: t
  erc-networks-mode: t
  desktop-save-mode: t
  windmove-mode: t
  tooltip-mode: t
  global-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
  visual-line-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Features:
(shadow sort mail-extr emacsbug mule-util hl-line network-stream nsm
transient edmacro kmacro display-line-numbers org-element org-persist
org-id org-refile avl-tree generator oc-basic ol-eww eww xdg url-queue
mm-url ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect gnus-art mm-uu
mml2015 mm-view mml-smime smime gnutls dig gnus-sum shr pixel-fill
kinsoku url-file svg dom gnus-group gnus-undo gnus-start gnus-dbus
gnus-cloud nnimap nnmail mail-source utf7 nnoo parse-time gnus-spec
gnus-int gnus-range message sendmail yank-media puny rfc822 mml mml-sec
epa derived epg rfc6068 epg-config mm-decode mm-bodies mm-encode
mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mailabbrev gmm-utils
mailheader gnus-win gnus nnheader gnus-util text-property-search
mail-utils range mm-util mail-prsvr ol-docview doc-view jka-compr
image-mode exif dired dired-loaddefs ol-bibtex bibtex iso8601 ol-bbdb
ol-w3m ol-doi org-link-doi org ob ob-tangle ob-ref ob-lob ob-table
ob-exp org-macro org-src ob-comint org-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
disp-table erc-ring erc-desktop-notifications notifications dbus xml
erc-nicks color erc-netsplit erc-menu erc-list erc-goodies erc-imenu
imenu erc-pcomplete time-date pcomplete comint ansi-osc ansi-color
erc-button erc-fill erc-stamp erc-status-sidebar erc-track erc-match
erc-join erc-autoaway leuven-dark-theme recentf tree-widget wid-edit
pixel-scroll cua-base ring mb-depth whitespace goto-addr thingatpt
autorevert filenotify icomplete erc format-spec erc-backend erc-networks
erc-common erc-compat compat erc-loaddefs desktop frameset cus-load
windmove site-start comp comp-cstr warnings icons rx cl-extra help-mode
erc-autoloads info compat-autoloads markdown-mode-autoloads package
browse-url url url-proxy url-privacy url-expand url-methods url-history
url-cookie generate-lisp-file url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs password-cache
json subr-x map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs
cl-lib 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 397463 44016)
 (symbols 48 27808 0)
 (strings 32 98182 4397)
 (string-bytes 1 3015974)
 (vectors 16 52846)
 (vector-slots 8 919020 87741)
 (floats 8 532 941)
 (intervals 56 4698 372)
 (buffers 984 16))
[0001-erc-format-spoilers-Add-a-new-custom.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69597; Package emacs. (Thu, 07 Mar 2024 20:30:01 GMT) Full text and rfc822 format available.

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

From: "J.P." <jp <at> neverwas.me>
To: Fadi Moukayed <smfadi <at> gmail.com>
Cc: emacs-erc <at> gnu.org, 69597 <at> debbugs.gnu.org
Subject: Re: bug#69597: 29.2; ERC 5.6-git: Add a new customizable variable
 controlling how Erc displays spoilers
Date: Thu, 07 Mar 2024 12:29:12 -0800
Hi Fadi,

Fadi Moukayed <smfadi <at> gmail.com> writes:

> Tags: patch
> Severity: wishlist
>
> Hi all,
>
> I am submitting a trivial patch adding a simple customizable variable
> (erc-format-spoilers) to Erc, allowing the user to control how Erc
> displays spoiler text when mIRC formatting code interpretation is
> enabled.
> This idea for this patch was discussed with the Erc maintainers on
> #erc, and was deemed to be useful enough to warrant a submission.
>
> Note that this is my first attempt to contribute to GNU Emacs.

Welcome! I think this would make a helpful addition.

However, I'm also starting to wonder if the way `erc-spoiler-face'
currently operates doesn't constitute buggy behavior. Skimming the
related commits and nonexistent discussion history, I see no clear
indication as to motivation or reasoning, making me think it was just
chucked in on a whim. Frankly, it calls into question the fitness of all
involved (ahem). Really, though, the only legitimate use case that comes
to mind is possibly emphasizing the presence of spoilers when an
IRC-formatted fg/bg combo matches the buffer's own defaults, meaning the
span might otherwise be mistaken for whitespace. But that doesn't seem
like a common occurrence, and I doubt anyone would intentionally make
`fg:erc-color-face0' or `fg:erc-color-face1' match `erc-default-face' or
the `default' face's :background.

So, basically, I wonder if we shouldn't (instead?) just redefine the
face's role to be one of indicating _revealed_ text, which is currently
the job of `erc-inverse-face' (`erc-spoilers-face' could just :inherit
it). And FWIW, a change like this would be justifiable without much fuss
if we deemed it a bug fix, since this feature hasn't made it into any
releases yet.

Another (competing) idea would be to instead have the option specify a
regexp pattern along with color combinations that ERC could use to
determine if a candidate is likely spoiler text, which would then be
shown accordingly. Somehow, though, I'm rather dubious anyone would
actually bother configuring such a thing.

(Also, on a related note, we should probably add a `cursor-face'
property to complement the `mouse-face' one currently added to spoilers
and maybe also mention `cursor-face-highlight-mode' in the doc string.)

Anyhow, I'm not suggesting you need to take on any of what I've just
mentioned, especially with the fifteen-line limit in effect (unless of
course your paperwork comes through in record time or you're feeling up
for the challenge). That said, I'd still like your input on these
matters if you don't mind. From the Transient project you shared and our
wider discussion in the channel, it's clear you've thought more about
this stuff than anyone else around, especially these days.

J.P.

> In GNU Emacs 29.2 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.20,
>  cairo version 1.16.0) of 2024-02-27 built on lcy02-amd64-095
[...]
>
> From 4d3b8fa17a975d6f04ba2a6ef4865d3938a76315 Mon Sep 17 00:00:00 2001
> From: "F. Moukayed" <smfadi+emacs <at> gmail.com>
> Date: Wed, 6 Mar 2024 18:33:46 +0000
> Subject: [PATCH] * lisp/erc/erc.el: (erc-format-spoilers): Add a new
>  customizable variable controling how Erc displays spoilers
                               ^
>
> ---
>  lisp/erc/erc-goodies.el | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el
> index 7e30b10..211d704 100644
> --- a/lisp/erc/erc-goodies.el
> +++ b/lisp/erc/erc-goodies.el
> @@ -645,6 +645,11 @@ emergency (message flood) it can be turned off to save processing time.  See
>  
>  (defcustom erc-interpret-mirc-color nil
>    "If non-nil, ERC will interpret mIRC color codes."
> +  :type 'boolean
> +  :group 'erc-control-characters)
> +
> +(defcustom erc-format-spoilers nil
> +  "If non-nil, ERC will format spoilers with `erc-spoiler-face'."
>    :type 'boolean)
>  
>  (defcustom erc-beep-p nil
> @@ -968,7 +973,7 @@ Also see `erc-interpret-controls-p' and `erc-interpret-mirc-color'."
>    "Prepend properties from IRC control characters between FROM and TO.
>  If optional argument STR is provided, apply to STR, otherwise prepend properties
>  to a region in the current buffer."
> -  (if (and fg bg (equal fg bg))
> +  (if (and fg bg (equal fg bg) erc-format-spoilers)
>        (progn
>          (setq fg 'erc-spoiler-face
>                bg nil)

P.S. These changes look fine, I think.





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69597; Package emacs. (Fri, 08 Mar 2024 09:39:02 GMT) Full text and rfc822 format available.

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

From: Fadi Moukayed <smfadi <at> gmail.com>
To: "J.P." <jp <at> neverwas.me>
Cc: emacs-erc <at> gnu.org, 69597 <at> debbugs.gnu.org
Subject: Re: bug#69597: 29.2; ERC 5.6-git: Add a new customizable variable
 controlling how Erc displays spoilers
Date: Fri, 8 Mar 2024 10:07:18 +0100
[Message part 1 (text/plain, inline)]
> So, basically, I wonder if we shouldn't (instead?) just redefine the
> face's role to be one of indicating _revealed_ text, which is currently
> the job of `erc-inverse-face' (`erc-spoilers-face' could just :inherit
> it). And FWIW, a change like this would be justifiable without much fuss
> if we deemed it a bug fix, since this feature hasn't made it into any
> releases yet.

After pondering this issue for a day or two, I've come around to agree
with this assessment. My gut feeling is that the KISS (as in "Keep It
Simple") solution would be to go the :inherit route and to reveal any
spoilers on user interaction.
Aside from changing the definition of the face, this would entail a
small modification/simplification in `erc-controls-propertize', where
the already-existing `put-text-property' calls is changed to set
'mouse-face to 'erc-spoiler-face. An illustrative patch doing this
change is included.

**However**, this is where I've seemingly hit another bug in Erc.
While setting 'mouse-face should - in theory - work, and cause the
propertized text to get revealed on mouse hover, in practice, it does
not. Some part of Erc's formatting machinery seems to strip away the
'mouse-face property off the text, so it does seem like the
`put-text-property' call in `erc-controls-propertize' has never really
worked for quite some time. Or at least, this is what I observe on my
own Emacs setup – would be helpful if others can confirm this
behavior.

Unfortunately, I haven't managed to find exactly where there the
'mouse-face property is removed, which is why I've termed the attached
patch "illustrative", aka. it does not quite resolve the issue fully.
Some help here would be appreciated.

Cheers,
FM.



Am Do., 7. März 2024 um 21:29 Uhr schrieb J.P. <jp <at> neverwas.me>:
>
> Hi Fadi,
>
> Fadi Moukayed <smfadi <at> gmail.com> writes:
>
> > Tags: patch
> > Severity: wishlist
> >
> > Hi all,
> >
> > I am submitting a trivial patch adding a simple customizable variable
> > (erc-format-spoilers) to Erc, allowing the user to control how Erc
> > displays spoiler text when mIRC formatting code interpretation is
> > enabled.
> > This idea for this patch was discussed with the Erc maintainers on
> > #erc, and was deemed to be useful enough to warrant a submission.
> >
> > Note that this is my first attempt to contribute to GNU Emacs.
>
> Welcome! I think this would make a helpful addition.
>
> However, I'm also starting to wonder if the way `erc-spoiler-face'
> currently operates doesn't constitute buggy behavior. Skimming the
> related commits and nonexistent discussion history, I see no clear
> indication as to motivation or reasoning, making me think it was just
> chucked in on a whim. Frankly, it calls into question the fitness of all
> involved (ahem). Really, though, the only legitimate use case that comes
> to mind is possibly emphasizing the presence of spoilers when an
> IRC-formatted fg/bg combo matches the buffer's own defaults, meaning the
> span might otherwise be mistaken for whitespace. But that doesn't seem
> like a common occurrence, and I doubt anyone would intentionally make
> `fg:erc-color-face0' or `fg:erc-color-face1' match `erc-default-face' or
> the `default' face's :background.
>
> So, basically, I wonder if we shouldn't (instead?) just redefine the
> face's role to be one of indicating _revealed_ text, which is currently
> the job of `erc-inverse-face' (`erc-spoilers-face' could just :inherit
> it). And FWIW, a change like this would be justifiable without much fuss
> if we deemed it a bug fix, since this feature hasn't made it into any
> releases yet.
>
> Another (competing) idea would be to instead have the option specify a
> regexp pattern along with color combinations that ERC could use to
> determine if a candidate is likely spoiler text, which would then be
> shown accordingly. Somehow, though, I'm rather dubious anyone would
> actually bother configuring such a thing.
>
> (Also, on a related note, we should probably add a `cursor-face'
> property to complement the `mouse-face' one currently added to spoilers
> and maybe also mention `cursor-face-highlight-mode' in the doc string.)
>
> Anyhow, I'm not suggesting you need to take on any of what I've just
> mentioned, especially with the fifteen-line limit in effect (unless of
> course your paperwork comes through in record time or you're feeling up
> for the challenge). That said, I'd still like your input on these
> matters if you don't mind. From the Transient project you shared and our
> wider discussion in the channel, it's clear you've thought more about
> this stuff than anyone else around, especially these days.
>
> J.P.
>
> > In GNU Emacs 29.2 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.20,
> >  cairo version 1.16.0) of 2024-02-27 built on lcy02-amd64-095
> [...]
> >
> > From 4d3b8fa17a975d6f04ba2a6ef4865d3938a76315 Mon Sep 17 00:00:00 2001
> > From: "F. Moukayed" <smfadi+emacs <at> gmail.com>
> > Date: Wed, 6 Mar 2024 18:33:46 +0000
> > Subject: [PATCH] * lisp/erc/erc.el: (erc-format-spoilers): Add a new
> >  customizable variable controling how Erc displays spoilers
>                                ^
> >
> > ---
> >  lisp/erc/erc-goodies.el | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el
> > index 7e30b10..211d704 100644
> > --- a/lisp/erc/erc-goodies.el
> > +++ b/lisp/erc/erc-goodies.el
> > @@ -645,6 +645,11 @@ emergency (message flood) it can be turned off to save processing time.  See
> >
> >  (defcustom erc-interpret-mirc-color nil
> >    "If non-nil, ERC will interpret mIRC color codes."
> > +  :type 'boolean
> > +  :group 'erc-control-characters)
> > +
> > +(defcustom erc-format-spoilers nil
> > +  "If non-nil, ERC will format spoilers with `erc-spoiler-face'."
> >    :type 'boolean)
> >
> >  (defcustom erc-beep-p nil
> > @@ -968,7 +973,7 @@ Also see `erc-interpret-controls-p' and `erc-interpret-mirc-color'."
> >    "Prepend properties from IRC control characters between FROM and TO.
> >  If optional argument STR is provided, apply to STR, otherwise prepend properties
> >  to a region in the current buffer."
> > -  (if (and fg bg (equal fg bg))
> > +  (if (and fg bg (equal fg bg) erc-format-spoilers)
> >        (progn
> >          (setq fg 'erc-spoiler-face
> >                bg nil)
>
> P.S. These changes look fine, I think.
>
[0001-lisp-erc-erc-goodies-redefine-rework-erc-spoilers.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69597; Package emacs. (Fri, 08 Mar 2024 15:06:01 GMT) Full text and rfc822 format available.

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

From: "J.P." <jp <at> neverwas.me>
To: Fadi Moukayed <smfadi <at> gmail.com>
Cc: emacs-erc <at> gnu.org, 69597 <at> debbugs.gnu.org
Subject: Re: bug#69597: 29.2; ERC 5.6-git: Add a new customizable variable
 controlling how Erc displays spoilers
Date: Fri, 08 Mar 2024 07:05:00 -0800
[Message part 1 (text/plain, inline)]
Fadi Moukayed <smfadi <at> gmail.com> writes:

>> So, basically, I wonder if we shouldn't (instead?) just redefine the
>> face's role to be one of indicating _revealed_ text, which is currently
>> the job of `erc-inverse-face' (`erc-spoilers-face' could just :inherit
>> it). And FWIW, a change like this would be justifiable without much fuss
>> if we deemed it a bug fix, since this feature hasn't made it into any
>> releases yet.
>
> After pondering this issue for a day or two, I've come around to agree
> with this assessment. My gut feeling is that the KISS (as in "Keep It
> Simple") solution would be to go the :inherit route and to reveal any
> spoilers on user interaction.
> Aside from changing the definition of the face, this would entail a
> small modification/simplification in `erc-controls-propertize', where
> the already-existing `put-text-property' calls is changed to set
> 'mouse-face to 'erc-spoiler-face. An illustrative patch doing this
> change is included.

Makes sense to me.

> **However**, this is where I've seemingly hit another bug in Erc.
> While setting 'mouse-face should - in theory - work, and cause the
> propertized text to get revealed on mouse hover, in practice, it does
> not. Some part of Erc's formatting machinery seems to strip away the
> 'mouse-face property off the text, so it does seem like the
> `put-text-property' call in `erc-controls-propertize' has never really
> worked for quite some time.

Your suspicions are likely spot on. Sad as it is, I don't think this
"feature" has _ever_ worked, especially if the unit test is anything to
go by. Basically, if I remove a lazy contrivance from the test
environment so it better reflects reality, the thing fails with exactly
the behavior you describe. FWIW, I've attached an improved version that
no longer suffers from this problem.

> Or at least, this is what I observe on my
> own Emacs setup – would be helpful if others can confirm this
> behavior.
>
> Unfortunately, I haven't managed to find exactly where there the
> 'mouse-face property is removed, which is why I've termed the attached
> patch "illustrative", aka. it does not quite resolve the issue fully.
> Some help here would be appreciated.

Ugh, sorry to have put you through all that. I've gone ahead and
attached a preliminary proposal for addressing the situation. If it
seems rather roundabout, it definitely is. Basically, we can't really
responsibly move `erc-controls-highlight' after `erc-button-add-buttons'
in `erc-insert-modify-hook' without causing general mayhem. So, absent a
smarter way to reconcile various interests (many of them legacy)
contending for the same real estate (e.g., `mouse-face'), we'll likely
have little choice but to settle for something in the vicinity of where
I've ended up (although I'd love to be wrong about that).

>
> Cheers,
> FM.
[...]
>>
>
> From 06e008d1de8a85c9e6b9a5a83f5ec5aefeb446c3 Mon Sep 17 00:00:00 2001
> From: "F. Moukayed" <smfadi+emacs <at> gmail.com>
> Date: Fri, 8 Mar 2024 08:39:03 +0000
> Subject: [PATCH] * lisp/erc/erc-goodies.el: redefine & rework
>  `erc-spoilers-face' to indicate revealed text
>
> ---
>  lisp/erc/erc-goodies.el | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el
> index 7e30b10..12f7f3c 100644
> --- a/lisp/erc/erc-goodies.el
> +++ b/lisp/erc/erc-goodies.el
> @@ -665,9 +665,7 @@ The value `erc-interpret-controls-p' must also be t for this to work."
>    "ERC inverse face."
>    :group 'erc-faces)
>  
> -(defface erc-spoiler-face
> -  '((((background light)) :foreground "DimGray" :background "DimGray")
> -    (((background dark)) :foreground "LightGray" :background "LightGray"))
> +(defface erc-spoiler-face '((t :inherit (erc-inverse-face)))
>    "ERC spoiler face."
>    :group 'erc-faces)
>  
> @@ -968,13 +966,10 @@ Also see `erc-interpret-controls-p' and `erc-interpret-mirc-color'."
>    "Prepend properties from IRC control characters between FROM and TO.
>  If optional argument STR is provided, apply to STR, otherwise prepend properties
>  to a region in the current buffer."
> -  (if (and fg bg (equal fg bg))
> -      (progn
> -        (setq fg 'erc-spoiler-face
> -              bg nil)
> -        (put-text-property from to 'mouse-face 'erc-inverse-face str))
> -    (when fg (setq fg (erc-get-fg-color-face fg)))
> -    (when bg (setq bg (erc-get-bg-color-face bg))))
> +  (when (and fg bg (equal fg bg))
> +    (put-text-property from to 'mouse-face 'erc-spoiler-face str))

Here's how I envision this working. So, in addition to your
`put-text-property' above, you'd have something like this:

  (erc--reserve-important-text-props from to
                                     '( mouse-face erc-spoiler-face
                                        cursor-face erc-spoiler-face))

If you want, you can add `cursor-face' as well, so people without mice
can optionally use the feature:

  (add-text-properties from to '( mouse-face erc-spoiler-face
                                  cursor-face erc-spoiler-face)))

Please take a look at and (if possible) try the changes when you have a
chance. Happy to explain whatever doesn't make sense. And, obviously, if
you have any improvements or a superior solution, please don't hesitate.

Many thanks, as always.

> +  (when fg (setq fg (erc-get-fg-color-face fg)))
> +  (when bg (setq bg (erc-get-bg-color-face bg)))
>    (font-lock-prepend-text-property
>     from
>     to

[0001-5.6-Fix-misleading-test-in-erc-goodies.patch (text/x-patch, attachment)]
[0002-5.6-Make-important-text-props-more-resilient-in-ERC.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69597; Package emacs. (Fri, 08 Mar 2024 17:40:03 GMT) Full text and rfc822 format available.

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

From: Fadi Moukayed <smfadi <at> gmail.com>
To: "J.P." <jp <at> neverwas.me>
Cc: emacs-erc <at> gnu.org, 69597 <at> debbugs.gnu.org
Subject: Re: bug#69597: 29.2; ERC 5.6-git: Add a new customizable variable
 controlling how Erc displays spoilers
Date: Fri, 8 Mar 2024 17:47:33 +0100
[Message part 1 (text/plain, inline)]
Thanks JP,

Good news: With both of your patches applied (and including the
revised patch for erc-goodies.el, attached to this message), things
seem to behave exactly as expected. Spoilers are displayed correctly,
and hovering the mouse on them causes them to get revealed as
`erc-spoiler-face'.

The only issue I noticed after applying the patches, is that the
following warning is emitted on the *Messages* buffer – (Note that I
have native compilation enabled):

> ⛔ Warning (comp): erc-button.el:532:4: Warning: the function ‘erc--restore-important-text-props’ is not known to be defined.

I assume this is a compilation order issue? Note that this only
happens with a clean ELN cache (The following Emacs loads are fine),
so not sure how significant it is.

>Happy to explain whatever doesn't make sense

One question regarding "FIXME use a region instead of point-min/max"
in patch #0002, is there a reason why (region-beginning) /
(region-end) is indeed not used instead? Just mentioning that because
IIRC, point-{min,max} is a range over the entire (narrowed) buffer,
including the (buttonized) nick, message text, possible timestamp (if
activated) as well. I checked the properties on the whole message line
while testing and it doesn't seem to have any negative side-effects,
aside from the fact that it operates on more text than it has to – I
believe it need only be applied to the message text.

Cheers,
FM

Am Fr., 8. März 2024 um 16:05 Uhr schrieb J.P. <jp <at> neverwas.me>:
>
> Fadi Moukayed <smfadi <at> gmail.com> writes:
>
> >> So, basically, I wonder if we shouldn't (instead?) just redefine the
> >> face's role to be one of indicating _revealed_ text, which is currently
> >> the job of `erc-inverse-face' (`erc-spoilers-face' could just :inherit
> >> it). And FWIW, a change like this would be justifiable without much fuss
> >> if we deemed it a bug fix, since this feature hasn't made it into any
> >> releases yet.
> >
> > After pondering this issue for a day or two, I've come around to agree
> > with this assessment. My gut feeling is that the KISS (as in "Keep It
> > Simple") solution would be to go the :inherit route and to reveal any
> > spoilers on user interaction.
> > Aside from changing the definition of the face, this would entail a
> > small modification/simplification in `erc-controls-propertize', where
> > the already-existing `put-text-property' calls is changed to set
> > 'mouse-face to 'erc-spoiler-face. An illustrative patch doing this
> > change is included.
>
> Makes sense to me.
>
> > **However**, this is where I've seemingly hit another bug in Erc.
> > While setting 'mouse-face should - in theory - work, and cause the
> > propertized text to get revealed on mouse hover, in practice, it does
> > not. Some part of Erc's formatting machinery seems to strip away the
> > 'mouse-face property off the text, so it does seem like the
> > `put-text-property' call in `erc-controls-propertize' has never really
> > worked for quite some time.
>
> Your suspicions are likely spot on. Sad as it is, I don't think this
> "feature" has _ever_ worked, especially if the unit test is anything to
> go by. Basically, if I remove a lazy contrivance from the test
> environment so it better reflects reality, the thing fails with exactly
> the behavior you describe. FWIW, I've attached an improved version that
> no longer suffers from this problem.
>
> > Or at least, this is what I observe on my
> > own Emacs setup – would be helpful if others can confirm this
> > behavior.
> >
> > Unfortunately, I haven't managed to find exactly where there the
> > 'mouse-face property is removed, which is why I've termed the attached
> > patch "illustrative", aka. it does not quite resolve the issue fully.
> > Some help here would be appreciated.
>
> Ugh, sorry to have put you through all that. I've gone ahead and
> attached a preliminary proposal for addressing the situation. If it
> seems rather roundabout, it definitely is. Basically, we can't really
> responsibly move `erc-controls-highlight' after `erc-button-add-buttons'
> in `erc-insert-modify-hook' without causing general mayhem. So, absent a
> smarter way to reconcile various interests (many of them legacy)
> contending for the same real estate (e.g., `mouse-face'), we'll likely
> have little choice but to settle for something in the vicinity of where
> I've ended up (although I'd love to be wrong about that).
>
> >
> > Cheers,
> > FM.
> [...]
> >>
> >
> > From 06e008d1de8a85c9e6b9a5a83f5ec5aefeb446c3 Mon Sep 17 00:00:00 2001
> > From: "F. Moukayed" <smfadi+emacs <at> gmail.com>
> > Date: Fri, 8 Mar 2024 08:39:03 +0000
> > Subject: [PATCH] * lisp/erc/erc-goodies.el: redefine & rework
> >  `erc-spoilers-face' to indicate revealed text
> >
> > ---
> >  lisp/erc/erc-goodies.el | 15 +++++----------
> >  1 file changed, 5 insertions(+), 10 deletions(-)
> >
> > diff --git a/lisp/erc/erc-goodies.el b/lisp/erc/erc-goodies.el
> > index 7e30b10..12f7f3c 100644
> > --- a/lisp/erc/erc-goodies.el
> > +++ b/lisp/erc/erc-goodies.el
> > @@ -665,9 +665,7 @@ The value `erc-interpret-controls-p' must also be t for this to work."
> >    "ERC inverse face."
> >    :group 'erc-faces)
> >
> > -(defface erc-spoiler-face
> > -  '((((background light)) :foreground "DimGray" :background "DimGray")
> > -    (((background dark)) :foreground "LightGray" :background "LightGray"))
> > +(defface erc-spoiler-face '((t :inherit (erc-inverse-face)))
> >    "ERC spoiler face."
> >    :group 'erc-faces)
> >
> > @@ -968,13 +966,10 @@ Also see `erc-interpret-controls-p' and `erc-interpret-mirc-color'."
> >    "Prepend properties from IRC control characters between FROM and TO.
> >  If optional argument STR is provided, apply to STR, otherwise prepend properties
> >  to a region in the current buffer."
> > -  (if (and fg bg (equal fg bg))
> > -      (progn
> > -        (setq fg 'erc-spoiler-face
> > -              bg nil)
> > -        (put-text-property from to 'mouse-face 'erc-inverse-face str))
> > -    (when fg (setq fg (erc-get-fg-color-face fg)))
> > -    (when bg (setq bg (erc-get-bg-color-face bg))))
> > +  (when (and fg bg (equal fg bg))
> > +    (put-text-property from to 'mouse-face 'erc-spoiler-face str))
>
> Here's how I envision this working. So, in addition to your
> `put-text-property' above, you'd have something like this:
>
>   (erc--reserve-important-text-props from to
>                                      '( mouse-face erc-spoiler-face
>                                         cursor-face erc-spoiler-face))
>
> If you want, you can add `cursor-face' as well, so people without mice
> can optionally use the feature:
>
>   (add-text-properties from to '( mouse-face erc-spoiler-face
>                                   cursor-face erc-spoiler-face)))
>
> Please take a look at and (if possible) try the changes when you have a
> chance. Happy to explain whatever doesn't make sense. And, obviously, if
> you have any improvements or a superior solution, please don't hesitate.
>
> Many thanks, as always.
>
> > +  (when fg (setq fg (erc-get-fg-color-face fg)))
> > +  (when bg (setq bg (erc-get-bg-color-face bg)))
> >    (font-lock-prepend-text-property
> >     from
> >     to
>
[0001-erc-redefine-rework-erc-spoilers.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69597; Package emacs. (Sat, 09 Mar 2024 04:44:02 GMT) Full text and rfc822 format available.

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

From: "J.P." <jp <at> neverwas.me>
To: Fadi Moukayed <smfadi <at> gmail.com>
Cc: emacs-erc <at> gnu.org, 69597 <at> debbugs.gnu.org
Subject: Re: bug#69597: 29.2; ERC 5.6-git: Add a new customizable variable
 controlling how Erc displays spoilers
Date: Fri, 08 Mar 2024 20:43:05 -0800
[Message part 1 (text/plain, inline)]
Fadi Moukayed <smfadi <at> gmail.com> writes:

> The only issue I noticed after applying the patches, is that the
> following warning is emitted on the *Messages* buffer – (Note that I
> have native compilation enabled):
>
>> ⛔ Warning (comp): erc-button.el:532:4: Warning: the function
>> ‘erc--restore-important-text-props’ is not known to be defined.
>
> I assume this is a compilation order issue? Note that this only
> happens with a clean ELN cache (The following Emacs loads are fine),
> so not sure how significant it is.

Hm, unless I messed something up (definitely possible), that shouldn't
happen . For ERC, I typically remove all the lisp/erc/*.elc files after
every change and before rerunning Make, regardless of whether native
comp is enabled (though removing native-lisp/30.0.50-deadbeef/*.eln
isn't usually necessary, AFAICT). Sometimes, though, I also have to
remove lisp/loaddefs.* and others. In case you weren't aware, there are
recipes for regenerating various autoloads and Custom business in
lisp/Makefile, but I usually just delete all stale assets completely.

>>Happy to explain whatever doesn't make sense
>
> One question regarding "FIXME use a region instead of point-min/max"
> in patch #0002, is there a reason why (region-beginning) /
> (region-end) is indeed not used instead? Just mentioning that because
> IIRC, point-{min,max} is a range over the entire (narrowed) buffer,
> including the (buttonized) nick, message text, possible timestamp (if
> activated) as well. I checked the properties on the whole message line
> while testing and it doesn't seem to have any negative side-effects,
> aside from the fact that it operates on more text than it has to – I
> believe it need only be applied to the message text.

Unfortunately, insertion-hook members lack a means for detecting message
boundaries unequivocally, although they can obviously keep track of
their own modifications and make assertions accordingly. So I agree that
allowing the caller to specify BEG and END in cases where they're
already known makes sense. For example, if they've already scanned for
the end of the speaker name to accomplish some other task or happen to
know the start of the right stamp, it's worth passing that knowledge
along. But computing a sub-region specially, beforehand, just to call
this function is likely less efficient (not that you were suggesting
that). And, of course, accepting BEG/END parameters make it easier to
protect areas of the exposed buffer from props restoration, if ever
there's a need.

>> > From 06e008d1de8a85c9e6b9a5a83f5ec5aefeb446c3 Mon Sep 17 00:00:00 2001
>> > From: "F. Moukayed" <smfadi+emacs <at> gmail.com>
>> > Date: Fri, 8 Mar 2024 08:39:03 +0000
>> > Subject: [PATCH] * lisp/erc/erc-goodies.el: redefine & rework
>> >  `erc-spoilers-face' to indicate revealed text

This is news to me, but apparently there's a Git hook that complains
about overlong subject lines. After running git-am(1) to apply your
latest patch, I saw:

  Line longer than 78 characters in commit message
  Commit aborted; please see the file CONTRIBUTE

So I adjusted the message to conform to this requirement. If the
attached changes look all right to you, then I'll install them (or
something similar) in the coming days.

Thanks,
J.P.
[0000-v1-v2.diff (text/x-patch, attachment)]
[0001-5.6-Fix-misleading-test-in-erc-goodies.patch (text/x-patch, attachment)]
[0002-5.6-Make-important-text-props-more-resilient-in-ERC.patch (text/x-patch, attachment)]
[0003-5.6-Redefine-erc-spoilers-face-to-indicate-revealed-.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69597; Package emacs. (Sat, 09 Mar 2024 14:32:01 GMT) Full text and rfc822 format available.

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

From: Fadi Moukayed <smfadi <at> gmail.com>
To: "J.P." <jp <at> neverwas.me>
Cc: emacs-erc <at> gnu.org, 69597 <at> debbugs.gnu.org
Subject: Re: bug#69597: 29.2; ERC 5.6-git: Add a new customizable variable
 controlling how Erc displays spoilers
Date: Sat, 9 Mar 2024 11:34:58 +0100
>I typically remove all the lisp/erc/*.elc files

Thanks, this is what I missed. Deleting all *.elc files solves the
warning issue.

>So I adjusted the message to conform to this requirement. If the
>attached changes look all right to you, then I'll install them (or
>something similar) in the coming days.

Much appreciated. Didn't notice/expect a client-side hook running when
committing.

That said, I'd like to +1 the changeset and confirm that the proposed
changes apply cleanly, and yield the desired result. I tested inputs
of the form ^CX,X<text>^C on a temporary private channel – spoilers
are formatted and revealed as intended, and no regressions were
observed. Very nice. Would be a neat addition/fix for the next Erc
release.

Cheers,
FM

Am Sa., 9. März 2024 um 05:43 Uhr schrieb J.P. <jp <at> neverwas.me>:
>
> Fadi Moukayed <smfadi <at> gmail.com> writes:
>
> > The only issue I noticed after applying the patches, is that the
> > following warning is emitted on the *Messages* buffer – (Note that I
> > have native compilation enabled):
> >
> >> ⛔ Warning (comp): erc-button.el:532:4: Warning: the function
> >> ‘erc--restore-important-text-props’ is not known to be defined.
> >
> > I assume this is a compilation order issue? Note that this only
> > happens with a clean ELN cache (The following Emacs loads are fine),
> > so not sure how significant it is.
>
> Hm, unless I messed something up (definitely possible), that shouldn't
> happen . For ERC, I typically remove all the lisp/erc/*.elc files after
> every change and before rerunning Make, regardless of whether native
> comp is enabled (though removing native-lisp/30.0.50-deadbeef/*.eln
> isn't usually necessary, AFAICT). Sometimes, though, I also have to
> remove lisp/loaddefs.* and others. In case you weren't aware, there are
> recipes for regenerating various autoloads and Custom business in
> lisp/Makefile, but I usually just delete all stale assets completely.
>
> >>Happy to explain whatever doesn't make sense
> >
> > One question regarding "FIXME use a region instead of point-min/max"
> > in patch #0002, is there a reason why (region-beginning) /
> > (region-end) is indeed not used instead? Just mentioning that because
> > IIRC, point-{min,max} is a range over the entire (narrowed) buffer,
> > including the (buttonized) nick, message text, possible timestamp (if
> > activated) as well. I checked the properties on the whole message line
> > while testing and it doesn't seem to have any negative side-effects,
> > aside from the fact that it operates on more text than it has to – I
> > believe it need only be applied to the message text.
>
> Unfortunately, insertion-hook members lack a means for detecting message
> boundaries unequivocally, although they can obviously keep track of
> their own modifications and make assertions accordingly. So I agree that
> allowing the caller to specify BEG and END in cases where they're
> already known makes sense. For example, if they've already scanned for
> the end of the speaker name to accomplish some other task or happen to
> know the start of the right stamp, it's worth passing that knowledge
> along. But computing a sub-region specially, beforehand, just to call
> this function is likely less efficient (not that you were suggesting
> that). And, of course, accepting BEG/END parameters make it easier to
> protect areas of the exposed buffer from props restoration, if ever
> there's a need.
>
> >> > From 06e008d1de8a85c9e6b9a5a83f5ec5aefeb446c3 Mon Sep 17 00:00:00 2001
> >> > From: "F. Moukayed" <smfadi+emacs <at> gmail.com>
> >> > Date: Fri, 8 Mar 2024 08:39:03 +0000
> >> > Subject: [PATCH] * lisp/erc/erc-goodies.el: redefine & rework
> >> >  `erc-spoilers-face' to indicate revealed text
>
> This is news to me, but apparently there's a Git hook that complains
> about overlong subject lines. After running git-am(1) to apply your
> latest patch, I saw:
>
>   Line longer than 78 characters in commit message
>   Commit aborted; please see the file CONTRIBUTE
>
> So I adjusted the message to conform to this requirement. If the
> attached changes look all right to you, then I'll install them (or
> something similar) in the coming days.
>
> Thanks,
> J.P.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69597; Package emacs. (Sat, 09 Mar 2024 16:08:02 GMT) Full text and rfc822 format available.

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

From: "J.P." <jp <at> neverwas.me>
To: Fadi Moukayed <smfadi <at> gmail.com>
Cc: emacs-erc <at> gnu.org, 69597 <at> debbugs.gnu.org
Subject: Re: bug#69597: 29.2; ERC 5.6-git: Add a new customizable variable
 controlling how Erc displays spoilers
Date: Sat, 09 Mar 2024 08:06:15 -0800
[Message part 1 (text/plain, inline)]
Fadi Moukayed <smfadi <at> gmail.com> writes:

> That said, I'd like to +1 the changeset and confirm that the proposed
> changes apply cleanly, and yield the desired result. I tested inputs
> of the form ^CX,X<text>^C on a temporary private channel – spoilers
> are formatted and revealed as intended, and no regressions were
> observed. Very nice. Would be a neat addition/fix for the next Erc
> release.

Really appreciate the thorough testing -- and your patience even more so
because as much as I'd like to put a bow on this, it turns out (sigh)
there's one lingering matter yet unresolved.

Alas, looking more closely at how `erc-controls-propertize' treats
`erc-inverse-face' (crucially, as a modifying toggle [1]), I've quickly
come to rue the day I ever thought to suggest otherwise, especially in
drawing misguided associations with `erc-spoiler-face'. (Indeed, my
quasi-conflating the two was what led us astray to begin with.) So, if
not already clear, I now believe we should just treat `erc-spoiler-face'
as its own concern entirely and not have it inherit from
`erc-inverse-face'. All this to say: yet another revision attached.

Thanks, and apologies for the head fake.

[1] https://modern.ircdocs.horse/formatting#reverse-color


[0000-v2-v3.diff (text/x-patch, attachment)]
[0001-5.6-Leverage-inverse-video-for-erc-inverse-face.patch (text/x-patch, attachment)]
[0002-5.6-Make-important-text-props-more-resilient-in-ERC.patch (text/x-patch, attachment)]
[0003-5.6-Redefine-erc-spoiler-face-to-indicate-revealed-t.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69597; Package emacs. (Sat, 09 Mar 2024 20:40:01 GMT) Full text and rfc822 format available.

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

From: Fadi Moukayed <smfadi <at> gmail.com>
To: "J.P." <jp <at> neverwas.me>
Cc: emacs-erc <at> gnu.org, 69597 <at> debbugs.gnu.org
Subject: Re: bug#69597: 29.2; ERC 5.6-git: Add a new customizable variable
 controlling how Erc displays spoilers
Date: Sat, 9 Mar 2024 18:19:32 +0100
[Message part 1 (text/plain, inline)]
>Really appreciate the thorough testing -- and your patience even more so
>because as much as I'd like to put a bow on this, it turns out (sigh)
>there's one lingering matter yet unresolved.

*Ahem* I apologize for opening this can of worms really. Didn't quite
expect a simple question regarding spoiler rendering to result in this
amount of Erc hackery :)

> [1] https://modern.ircdocs.horse/formatting#reverse-color

Good call. I am aware of this reference (as any IRC tinkerer should
be), and indeed, Erc should handle ^V and ^C99,99 [1], even though
both are "Not universally supported".

I gave the patches another look and did another clean re-apply for an
additional test. All OK (results seen in the attached screenshot).
Implementing ^V using :inverse-video should be okay, although I'm not
sure about the implications on platforms that do not support that (do
any exist? documentation here is sparse, unfortunately). I assume it
will fall back to the hard-coded white-on-black color specification in
that case, which is probably an acceptable compromise.

Cheers,
FM

[1] subtle edge case which shouldn't be formatted as a spoiler, even
though X=Y=99 in ^CX,Y (Again, good catch, thanks for that)

Am Sa., 9. März 2024 um 17:06 Uhr schrieb J.P. <jp <at> neverwas.me>:
>
> Fadi Moukayed <smfadi <at> gmail.com> writes:
>
> > That said, I'd like to +1 the changeset and confirm that the proposed
> > changes apply cleanly, and yield the desired result. I tested inputs
> > of the form ^CX,X<text>^C on a temporary private channel – spoilers
> > are formatted and revealed as intended, and no regressions were
> > observed. Very nice. Would be a neat addition/fix for the next Erc
> > release.
>
> Really appreciate the thorough testing -- and your patience even more so
> because as much as I'd like to put a bow on this, it turns out (sigh)
> there's one lingering matter yet unresolved.
>
> Alas, looking more closely at how `erc-controls-propertize' treats
> `erc-inverse-face' (crucially, as a modifying toggle [1]), I've quickly
> come to rue the day I ever thought to suggest otherwise, especially in
> drawing misguided associations with `erc-spoiler-face'. (Indeed, my
> quasi-conflating the two was what led us astray to begin with.) So, if
> not already clear, I now believe we should just treat `erc-spoiler-face'
> as its own concern entirely and not have it inherit from
> `erc-inverse-face'. All this to say: yet another revision attached.
>
> Thanks, and apologies for the head fake.
>
> [1] https://modern.ircdocs.horse/formatting#reverse-color
>
>
[erc-spoilers.png (image/png, attachment)]

Reply sent to "J.P." <jp <at> neverwas.me>:
You have taken responsibility. (Thu, 14 Mar 2024 02:19:02 GMT) Full text and rfc822 format available.

Notification sent to Fadi Moukayed <smfadi <at> gmail.com>:
bug acknowledged by developer. (Thu, 14 Mar 2024 02:19:02 GMT) Full text and rfc822 format available.

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

From: "J.P." <jp <at> neverwas.me>
To: 69597-done <at> debbugs.gnu.org
Cc: Fadi Moukayed <smfadi <at> gmail.com>, emacs-erc <at> gnu.org
Subject: Re: bug#69597: 29.2; ERC 5.6-git: Add a new customizable variable
 controlling how Erc displays spoilers
Date: Wed, 13 Mar 2024 19:17:43 -0700
Fadi Moukayed <smfadi <at> gmail.com> writes:

> I gave the patches another look and did another clean re-apply for an
> additional test. All OK (results seen in the attached screenshot).
> Implementing ^V using :inverse-video should be okay, although I'm not
> sure about the implications on platforms that do not support that (do
> any exist? documentation here is sparse, unfortunately). I assume it
> will fall back to the hard-coded white-on-black color specification in
> that case, which is probably an acceptable compromise.

This has been installed as

  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=166c8a98

Thanks and closing.

                              * * *

A tangentially related note regarding this area of the code base: As
mentioned at least once in code comments, we should probably eventually
begin phasing out the existing face names in favor of ones that abide by
Emacs naming conventions:

- Like all identifiers, they should be properly namespaced, so no
  leading "fg:" and "bg:"

- They should _not_ end in "-face" (info "(elisp) Defining Faces")

I don't really have the bandwidth for dealing with this anytime soon,
even though it's a relatively straightforward, mostly mechanical change.
Just thought I'd note it here for the record in case someone reading
this takes a fancy.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Thu, 11 Apr 2024 11:24:44 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 67 days ago.

Previous Next


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