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

[J.P.]

Hi Fadi,

Fadi Moukayed <smfadi@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@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.