Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Constrain the ement-room--event-mentions-user-p regexp #240

Open
phil-s opened this issue Nov 7, 2023 · 6 comments
Open

Constrain the ement-room--event-mentions-user-p regexp #240

phil-s opened this issue Nov 7, 2023 · 6 comments
Assignees
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed priority:B
Milestone

Comments

@phil-s
Copy link

phil-s commented Nov 7, 2023

At present the word uphill in a message is treated as mentioning the username phil :)

Let's at minimum wrap the pattern with word boundaries?

(Or do we then worry about usernames which do not begin/end with word characters? Either way, I'm sure we can improve this...)

@alphapapa
Copy link
Owner

Makes sense. Thanks.

@alphapapa alphapapa self-assigned this Nov 7, 2023
@alphapapa alphapapa added bug Something isn't working help wanted Extra attention is needed good first issue Good for newcomers labels Nov 7, 2023
@alphapapa alphapapa added this to the 0.14 milestone Nov 7, 2023
@phil-s
Copy link
Author

phil-s commented Nov 26, 2023

I looked to see if there were restrictions on characters used for a user's display name, and I'm not seeing anything. Searching for displayname at https://spec.matrix.org/v1.8/client-server-api/ gives lots of info, but it seems like the value may be "any string".

So we can't simply wrap the quoted display name with word boundaries, but I suspect that even if the user had non-word characters at the start or end of their display name, we could assume that any mention of their name will have non-word characters (or bol/eol) surrounding the display name. So maybe something like this:

"\\(?:^\\|[^[:word:]]\\)\\(DISPLAYNAME\\)\\(?:$\\|[^[:word:]]\\)"

Possibly in the form of a rx-based template in a new variable or user option.

@alphapapa
Copy link
Owner

It's kind of hidden here: https://spec.matrix.org/v1.8/appendices/#user-identifiers

@phil-s
Copy link
Author

phil-s commented Nov 26, 2023

Ah, yes, we have different rules for username and displayname (I was only thinking about displayname).

At present ement-room--event-mentions-user-p uses the same code for both. In fact it matches on three names...

    (or (matches-body-p (ement-user-username user))
        (matches-body-p (ement--user-displayname-in room user))
        (matches-body-p (ement-user-id user)))))))

Where username is the "Username part of user's Matrix ID." in the user struct; so I guess we apply the character constraints from your spec URL to both ement-user-username and ement-user-id, and only ement--user-displayname-in should be the more permissive one.

@Konubinix
Copy link

IIUC, doing this would fix the issue for both username and useid, right?

    (or (matches-body-p (format "\\b%s\\b" (ement-user-username user)))
        (matches-body-p (ement--user-displayname-in room user))
        (matches-body-p (format "\\b%s\\b" (ement-user-id user))))))))

Can we do this first and deal with the user displayname after?

@Konubinix
Copy link

About the displayname, it sounds like there is no silver bullet.

For example, I think the displayname ":-) me :-)" can be considered a mention in "Hello :-) me :-)" but also in "Hello:-) me :-)".

But the displayname ". Therefore" would be matched against "Bla bla. Therefore bla bla", which seems counterintuitive.

I guess that to deal with all cases, we should provide a custom fonction that the user may define to match per own displayname, and provide one by default. My opinion would be to provide one that keep the current behavior.

@alphapapa alphapapa modified the milestones: 0.14, 0.15 Jan 25, 2024
@alphapapa alphapapa modified the milestones: 0.15, 0.16 Mar 31, 2024
@alphapapa alphapapa modified the milestones: v0.16, v0.17 Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed priority:B
Projects
None yet
Development

No branches or pull requests

3 participants