Subject: Re: bug#64301: 30.0.50; ERC 5.6: Make speaker labels easier to work with

From: "J.P." <[email-masked]>
Date: Thu, 13 Jul 2023 19:20:08 -0700

"J.P." <[email-masked]> writes:

> v2. [...] Combine faces in `erc-display-message' when called with a
> list `type' arg.
> [...]
>
> This iteration includes a number of changes, the most important being
> one that looks severe but is largely mechanical. It concerns a bug
> that's been around forever and, as such, is now part of the foundation.
> Basically, the `type' parameter of the function `erc-display-message'
> determines a special handler that applies styling to a message. When the
> parameter is a list, the function calls each handler in turn, left to
> right, which clobbers existing faces instead of combines them. The net
> effect (in most cases) is identical to calling the function with the
> last member alone. Despite this, it would seem ending up with multiple,
> "layered" faces was the authors' original intent based on the ubiquity
> of call sites featuring the list variant.
>
> Authoritative intent aside, in the intervening decades, other code was
> written that expects the current clobbering behavior. An example of this
> exists in the default value of `erc-track-faces-priority-list', which
> contains `erc-error-face' alone rather than paired with
> `erc-notice-face'. There may indeed be others that will go undetected
> should we make this "fix," which I'm still in favor of despite the
> attendant risk.

Actually, I decided against doing this after all [1]. There was simply no
telling what kind of damage might result from these changes rippling
through two decades of third-party code.

> Though improved call-site readability is one upside, I'm more
> interested in the UX possibilities such layering opens up. Subtle
> benefits can already be seen after applying these changes, for
> example, in text inserted for outgoing /me commands, which now sport a
> combination of `erc-input-face' and `erc-action-face'.
>
> In a bid to mitigate potential breakage, at least internally, I've gone
> ahead and mass-replaced all instances of

>   (erc-display-message parsed '(notice error) ...)

> with

>   (erc-display-message parsed 'error ...)

> which amounts to 38 incisions in total. Once again, if we leave things
> as is, users who've customized `erc-track-faces-priority-list' won't
> ever see error-colored text in their mode line, which is a deal breaker.

Instead of all that, I've implemented an uglier and less transparent
(but also non-disruptive) way for `erc-display-message' to perform such
layering. Basically, callers can request the effect by specifying a
slight variant of its list `type' parameter, namely, one headed by the
symbol t, e.g,

  (erc-display-message parsed '(t notice error) ...)

Without the t, this function acts as it always has. With it, the
function composes faces atop one another, left to right. Existing call
sites obviously won't benefit, but at least they'll be shielded from
harm. Hope that's an agreeable compromise.

Thanks and closing.

[1] https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=d45770e8