Re: bug#59805: 28.2; erc-track: handle faces modified with erc-button-add-face

[J.P.]

Nacho Barrientos <nacho.barrientos@cern.ch> writes:

> Hi J.P.,

Hi Nacho,

>> Looks like the default value of `erc-track-faces-priority-list' contains
>> some composite faces, such as
>>
>>   (erc-nick-default-face erc-current-nick-face)
>>
>> So whatever happens, I think we'll have to preserve compatibility for
>> people already used to searching for such composites.
>
> I was totally unaware that "face composites" were a thing, so
> perhaps my proposal to flatten the return value of `erc-faces-in' does
> not make sense anymore.

FWIW, "composite" was just some term I conjured up to refer to what's
described in the third `face' bullet point of "(elisp) Special
Properties":

  A list of faces.  Each list element should be either a face
  name or an anonymous face.  This specifies a face which is an
  aggregate of the attributes of each of the listed faces.
  Faces occurring earlier in the list have higher priority.

While perhaps less common than lone face names, such values do appear in
the wild, for example, when `whitespace-mode' is active.

> For 2) I don't have an elegant/generic solution to propose, but adding
>
>   (erc-hl-nicks-nick-nacho-face erc-current-nick-face)
>
> to `erc-track-faces-priority-list' can definitely work for me. My
> nickname is stable across networks, however I think that the rather
> valid use case of being notified no matter what your nick name is cannot
> be honoured elegantly when using erc-hl-nicks.

>From what I can gather, you still believe this despite the more recent
findings in your followup just below.

> Hi again,
>
> [...]
>
> Actually there's something else that can be done from the erc-hl-nicks
> side, which is:
>
>   (setq erc-hl-nicks-skip-nicks '("nacho"))

Good find!

> Which comes again with the problematic of having to hardcode known nicks
> but it does the job. If that variable is set as indicated above,
> erc-hl-nicks will never create the composite face on the mention and
> hence:
>
>   λ> (erc-faces-in (buffer-substring (point) (+ 1 (point))))
>   (erc-current-nick-face)
>
> (in the example above, `point' is on a mention to the current nick)
>
> Making erc-track's matching work. The current nickname is coloured
> anyway by ERC so if erc-hl-nicks does not act on it should be okay.

Re the "hardcode problem": I'm not sure it's incumbent upon ERC to offer
a solution uniquely tailored to updating `erc-hl-nicks-skip-nicks'
specifically (not that you were suggesting such a thing). However, I do
think the current offering of `erc-nick-changed-functions' has proven
insufficient for servicing a variety of legitimate needs. It only runs
under really specific circumstances, such as when a server-mandated nick
change occurs or a confirmation arrives following a NickServ IDENTIFY
(or similar action). However, it does *not* run on confirmation of a
client-initiated NICK command, whether manually issued by a user or
executed as library code.

Unfortunately, for compatibility reasons, we can't just repurpose
`erc-nick-changed-functions' willy-nilly. Thus, I think introducing a
*new* abnormal hook to be run by `erc-set-current-nick' might be
justified, likely defined as compatible with functions taking a new nick
and giving nothing in return. Users and downstream packages could then
do whatever needs doing in reaction to nick changes, for example:

  (add-hook 'erc-set-current-nick-functions
            (lambda (n)
              (customize-set-variable
               'erc-hl-nicks-skip-nicks
               (cl-pushnew n erc-hl-nicks-skip-nicks :test #'equal))))

> This approach is good enough for me so feel free to close the bug if you
> don't want to go down this rabbit hole further :)

Regardless of this bug's original title, the mission of improving the
user experience remains paramount, wherever that may lead. If you're up
for it, I for one would welcome a patch adding a hook like the one
described above (whenever time allows). Otherwise, we could always just
kick the can down the road and rename this bug to something like "ERC
needs a smarter way to run user code on nick changes" in hopes some
enterprising future person will find it. Either way, I appreciate your
willingness to contribute to ERC.

J.P.