Provide a way to influence highlighting decisions in rooms

[sergiodj]

OS/platform

GNU/Linux

Emacs version and provenance

30.0.50, compiled manually.

Emacs command

emacs server + client

Emacs frame type

A mix of both (mainly GUI, though)

Ement package version and provenance

0.15.1, from ELPA

Actions taken

I use Heisenbridge to connect to my ZNC bouncer, which gives me a nice way to be natively on IRC while at the same time having access to everything via Matrix too.

Heisenbridge works by creating Matrix users for each IRC user it sees. These Matrix user IDs have the form of @hbirc_libera.znc_NICK:sergiodj.net.

Observed results

I run my own Matrix server, and my domain name is the same as my Matrix user (sergiodj.net for the domain, and sergiodj for the user). Because the Heisenbridge user IDs contain my domain name in them, and because IRC messages look like the below excerpt on Matrix:

<a href=\"https://matrix.to/#/@hbirc_libera.znc_NICKNAME:sergiodj.net\">NICKNAME</a>: This is an example message mentioning a user whose nick is NICKNAME.

Ement mistakenly thinks that I'm being mentioned whenever someone mentions anyone else on IRC.

This causes many problems. Namely, all IRC messages which mention some user:

• Generate a desktop notification (fixed; see below).
• End up in the *Ement Mentions* buffer (fixed; see below).
• Cause the message to be highlighted in the room (this is what this bug is about).

I fixed the first problem by adding the following function to ement-notify-ignore-predicates:

  (defun sdj/ement-notify--event-from-other-heisenbridge-user (event room session)
    "Return non-nil if EVENT in ROOM comes from another heisenbridge user."
    (pcase-let* (((cl-struct ement-session user) session)
		 ((cl-struct ement-event sender) event))
      (unless (equal (ement-user-id user) (ement-user-id sender))
	(pcase-let* (((cl-struct ement-event content) event)
		     ((map body formatted_body) content)
		     (body (or formatted_body body)))
	  (when body
	    (save-match-data
	      (let ((idx 0)
		    (found-me nil))
		(while (string-match
			(rx "<a href=\"https://matrix.to/#/@hbirc_"
			    (minimal-match (1+ (not ">"))) ">"
			    (group (minimal-match (1+ (not "<"))))
			    "</a>")
			body idx)
		  (setq found-me (or found-me
				     (string= (match-string 1 body) "sergiodj")))
		  (setq idx (match-end 1)))
		(not found-me))))))))

The second problem was fixed by removing ement-notify--event-mentions-session-user-p from ement-notify-mention-predicates and adding the following function instead:

  (defun sdj/ement-notify--event-doesnt-mention-heisenbridge (event room session)
    (pcase-let* (((cl-struct ement-session user) session)
		 ((cl-struct ement-event sender) event))
      (and (not (equal (ement-user-id user) (ement-user-id sender)))
	   (ement-room--event-mentions-user-p event user room)
	   (not (sdj/ement-notify--event-from-other-heisenbridge-user event room session)))))

Expected results

It would be great to have a way to control the events which make highlighting happen in the room. Currently I can't hook into the function that decides to do that.

Backtrace

No response

Etc.

No response

[alphapapa]

Hi Sergio,

Thanks for the well-written report.

FWIW I think you could solve two of these problems by applying advice to the function ement-notify--event-mentions-session-user-p, i.e. wrap it in the equivalent of your sdj/ement-notify--event-doesnt-mention-heisenbridge function.

In the meantime, the idea to have more control over what highlights a message is not a bad one, so it will go on the to-do list. Thanks.

[sergiodj]

Thanks for the well-written report.

Thanks for the quick reply!

FWIW I think you could solve two of these problems by applying advice to the function ement-notify--event-mentions-session-user-p, i.e. wrap it in the equivalent of your sdj/ement-notify--event-doesnt-mention-heisenbridge function.

Indeed, I thought about using advices, too. I like being able to cleanly modify a function's behaviour using hooks or other mechanisms, so that's why I chose the "hard" way of reimplementing things and using hooks instead, but I will revisit the code and see if I can make it simpler :-).

In the meantime, the idea to have more control over what highlights a message is not a bad one, so it will go on the to-do list. Thanks.

Thanks for considering the idea.

Do you think it's also possible to use an advice to solve this problem, BTW?

[alphapapa]

Do you think it's also possible to use an advice to solve this problem, BTW?

I don't see why not. Advice allows you to replace a function with your own code, which can do anything.

[viiru-]

Wouldn't the simple solution be just ignoring the sender field in ement-room--event-mentions-user-p and/or ement-notify--event-mentions-session-user-p? It doesn't seem like matching on that would be correct in any situation.

[alphapapa]

Wouldn't the simple solution be just ignoring the sender field in ement-room--event-mentions-user-p and/or ement-notify--event-mentions-session-user-p? It doesn't seem like matching on that would be correct in any situation.

That's used to negate matches, i.e. if the user types his own name or MXID, we don't want to act as if he mentioned himself.

[viiru-]

ement-notify--event-mentions-session-user-p has a separate check for that, (unless (equal (ement-user-id user) (ement-user-id sender)). The problem is indeed that ement-room--event-mentions-user-p doesn't check for that, and acts as if the user mentioned themselves.

[alphapapa]

ement-notify--event-mentions-session-user-p has a separate check for that, (unless (equal (ement-user-id user) (ement-user-id sender)). The problem is indeed that ement-room--event-mentions-user-p doesn't check for that, and acts as if the user mentioned themselves.

The lines in question are here:

https://github.com/alphapapa/ement.el/blob/9f6cbb8b406160c950d8932d66f3629aba091ec8/ement-room.el#L4139C13-L4141

The problem is how to distinguish sergiodj in a message from, I guess, @hbirc_libera.znc_NICKNAME:sergiodj.net. You can try to wrap the regexp in various metacharacters, like word-start/end, but we also have to allow for, e.g. punctuation around it (e.g. if someone writes, Hello, sergiodj, how are you?, or, He said, "sergiodj said..."). How do we allow for those without also matching foobar:sergiodj.net or other potential superstrings of the user's displayname?

[alphapapa]

The best idea I have is to allow the user to specify an "anti-mention" regexp, which would nullify any apparent mention. In this case, the user could set it to something like (rx "@hbirc_libera.znc_" (1+ (not blank)) ":sergiodj.net"). Even then, I don't know if it would be perfect, because a message could contain both a string matching that, and a legitimate mention of sergiodj; so to correctly filter those would probably require comparing the "anti-mention" regexp against the string that appeared to indicate a legitimate mention.

[viiru-]

Excluding the sender field from the matching wouldn't have any of the mentioned issues.

[alphapapa]

Excluding the sender field from the matching wouldn't have any of the mentioned issues.

I think we must be miscommunicating. If the message body has one of these bridged MXIDs in it, that is causing this false mention. Unless I'm just totally misunderstanding what's going on here...

[viiru-]

I think we must be miscommunicating. If the message body has one of these bridged MXIDs in it, that is causing this false mention. Unless I'm just totally misunderstanding what's going on here...

You are entirely correct. I went through this in my head a few more times last night and finally it clicked. The match isn't on sender, but on body contents.

The way I've been avoiding this in practice is by using ement-notify--room-unread-p instead of ement-room--event-mentions-user-p. That has the problematic side-effect of notifying for every subsequent message after a mention.

[sergiodj]

Do you think it's also possible to use an advice to solve this
problem, BTW?

I don't see why not. Advice allows you to replace a function with
your own code, which can do anything.

Sorry about the delay in replying, and I see now that my question was
poorly worded. I meant to ask if you'd know which function should be
advised in order to perform implement the behaviour I wanted, but I've
already found it. For the record, here's the simplified solution I'm
using which solves all 3 scenarios I described:

(cl-defun sdj/ement-room--event-doesnt-mention-other-heisenbridge-user-p (event user &optional _ignore)
  "Verify whether USER is a Heisenbridge user.  If it is, and if it isn't me, return nil.  Otherwise, return t."
  (pcase-let* (((cl-struct ement-event content) event)
  	 ((map body formatted_body) content)
  	 (body (or formatted_body body)))
    (when body
  (cond
   ((string-match-p (regexp-quote "@hbirc_") body)
    ;; This comes from Heisenbridge.
    (let ((idx 0)
  	(found-me nil))
      (save-match-data
        (while (and (not found-me)
  		  (string-match
  		   (rx "<a href=\"https://matrix.to/#/@hbirc_"
  		       (minimal-match (1+ (not ">"))) ">"
  		       (group (minimal-match (1+ (not "<"))))
  		       "</a>")
  		   body idx))
  	(setq found-me (string= (match-string 1 body) "sergiodj"))
  	(setq idx (match-end 1))))
      found-me))
   ((string-match-p (regexp-quote "sergiodj") body)
    ;; This is likely a "regular" Matrix message with my nick in it.
    t)
   (t nil)))))

(defun sdj/ement-notify--event-from-other-heisenbridge-user-p (event room session)
  "Return non-nil if EVENT in ROOM comes from another heisenbridge user."
  (pcase-let* (((cl-struct ement-session user) session)
  	 ((cl-struct ement-event sender) event))
    (unless (equal (ement-user-id user) (ement-user-id sender))
  (not (sdj/ement-room--event-doesnt-mention-other-heisenbridge-user-p event user)))))


(advice-add #'ement-room--event-mentions-user-p
        :before-while #'sdj/ement-room--event-doesnt-mention-other-heisenbridge-user-p)
(add-to-list 'ement-notify-ignore-predicates
         #'sdj/ement-notify--event-from-other-heisenbridge-user-p
         t)

[sergiodj]

The best idea I have is to allow the user to specify an "anti-mention"
regexp, which would nullify any apparent mention. In this case, the
user could set it to something like (rx "@hbirc_libera.znc_" (1+ (not blank)) ":sergiodj.net"). Even then, I don't know if it would be
perfect, because a message could contain both a string matching that,
and a legitimate mention of sergiodj; so to correctly filter those
would probably require comparing the "anti-mention" regexp against the
string that appeared to indicate a legitimate mention.

I think it's obvious by looking at the code I posted, but the way I'm
doing it is by looking at the entire message body and seeing whether
my nickname appears to be mentioned (either by a Heisenbridge ID or by
a regular MXID). Arguably this is a heuristic, but I'd rather have
false positives than to miss real mentions.

IOW, I like the idea of having an "anti-mention" regexp; I think it's
a good first step towards solving this problem.

[alphapapa]

The way I've been avoiding this in practice is by using ement-notify--room-unread-p instead of ement-room--event-mentions-user-p. That has the problematic side-effect of notifying for every subsequent message after a mention.

Yeah, I keep meaning to fix that. It annoys me too.

[alphapapa]

I think it's obvious by looking at the code I posted, but the way I'm
doing it is by looking at the entire message body and seeing whether
my nickname appears to be mentioned (either by a Heisenbridge ID or by
a regular MXID). Arguably this is a heuristic, but I'd rather have
false positives than to miss real mentions.

Well, it's not quite obvious to me; whether it's due to the docstring or the formatting, I'm not sure. ;) Also, it's a bit unusual to write a "doesn't" predicate function; it would probably be easier to understand if you wrote it in positive terms and then negate the result where you use it.

Anyway, what you describe now is what the existing code already does: looks for your displayname in the message body. So I'm not sure how your advice function is solving the problem.

FWIW, I've never seen :before-while advice actually used, so your use of it is probably either a very elegant solution or a very unusual hack. ;)

[sergiodj]

I think it's obvious by looking at the code I posted, but the way
I'm doing it is by looking at the entire message body and seeing
whether my nickname appears to be mentioned (either by a
Heisenbridge ID or by a regular MXID). Arguably this is a heuristic,
but I'd rather have false positives than to miss real mentions.

Well, it's not quite obvious to me; whether it's due to the docstring
or the formatting, I'm not sure. ;) Also, it's a bit unusual to write
a "doesn't" predicate function; it would probably be easier to
understand if you wrote it in positive terms and then negate the
result where you use it.

Ah, the docstring got messed up when I pasted the code, sorry about
that. It's fixed now.

In a way, the function already works in "positive terms" as you put
it; it's just its name that's reversed. You can see that I'm already
negating its result when I call it directly inside
sdj/ement-notify--event-from-other-heisenbridge-user-p. But I like
your suggestion; I'll rename the function to make it clear that it's
trying to find my nickname in the message.

Anyway, what you describe now is what the existing code already does:
looks for your displayname in the message body. So I'm not sure how
your advice function is solving the problem.

My advice function extends the existing check by supporting matching
Heisenbridge-style IDs. IIUC your idea, I would be able to hook my
logic into this "anti-matching" regexp in order to filter out unwanted
Heisenbridge events.

FWIW, I've never seen :before-while advice actually used, so your
use of it is probably either a very elegant solution or a very unusual
hack. ;)

Hah, it's probably the latter. But it made sense to use it because I
want ement-room--event-mentions-user-p to run only if my advice was
successful. Since my function isn't trying to be an all-in-one
solution, it still relies on the smarter logic inside
ement-room--event-mentions-user-p to make sure that my nickname is
indeed present.

[alphapapa]

Ah, the docstring got messed up when I pasted the code, sorry about that. It's fixed now.

In a way, the function already works in "positive terms" as you put it; it's just its name that's reversed. You can see that I'm already negating its result when I call it directly inside sdj/ement-notify--event-from-other-heisenbridge-user-p. But I like your suggestion; I'll rename the function to make it clear that it's trying to find my nickname in the message.

It's probably just me, but I still find the first function confusing. I'd suggest 1) writing the docstring in terms of, "Return non-nil if..." (this is standard Emacs convention; and there's no need to say that it returns nil otherwise); 2) narrow its scope: as is, it seems less like a predicate function, because it seems to test two different things. It might be best split into two functions.

OTOH if this is just for your config and it works for you, then that's all that matters.

[sergiodj]

It's probably just me, but I still find the first function confusing.
I'd suggest 1) writing the docstring in terms of, "Return non-nil
if..." (this is standard Emacs convention; and there's no need to say
that it returns nil otherwise);

Ah, good data point. I've rewritten the docstring now.

2. narrow its scope: as is, it seems
   less like a predicate function, because it seems to test two different
   things. It might be best split into two functions.

Would you care expanding on this point? I see how I could split this
function in two (one for checking Heisenbridge-style usernames, and
the other for regular MXIDs), but IMHO this function is already a
predicate in that it returns either nil or t depending on whether
my username appears in the message.

OTOH if this is just for your config and it works for you, then that's
all that matters.

Yeah, this is just for my personal config, but I'm also interested in
discussing these things with a more seasoned elisp hacker :-).
Apologies if this is out of scope; feel free to move this discussion
somewhere else if you'd like.

[alphapapa]

@sergiodj It seems that I was looking at #281 (comment) while you were updating the code in #281 (comment).

[sergiodj]

@alphapapa, nope, I was updating the code on comment #281 (comment). That's pretty much the last version of the code I have here.

[alphapapa]

@sergiodj I'm not sure what you mean; #281 (comment) has no code in it.

[sergiodj]

@alphapapa sorry, I should have said #281 (comment)