| 1 May 2025 |
 @joepie91:pixie.town | if you are assuming that a mention contains an @, which that code does, then it should have done an additional check for the @ actually being attached to something instead of just counting @s | 15:42:53 |
| @joepie91:pixie.town | and this is like, table stakes string matching stuff, this is not something obscure or complicated, this is pretty much the answer you end up at after a few minutes of thinking about it | 15:43:20 |
| @joepie91:pixie.town | it's the lowest of low-hanging fruit for preventing false positives, and it clearly wasn't done | 15:43:34 |
 @emma:rory.gay | i mean, i would assume that it is? i dont see what wrong with the current code | 15:43:44 |
| @emma:rory.gay | anything more than this would completely miss the actual behavior of legacy mentions on matrix, which is just a string.contains() | 15:44:15 |
| * @joepie91:pixie.town sighs | 15:44:25 |
| @joepie91:pixie.town | string.contains does not involve @s at any point and so is in no way relevant to the point I am making here | 15:44:50 |
| @joepie91:pixie.town | I am running out of ways to say this | 15:45:10 |
| @emma:rory.gay | unless you meant to say that this would be right solution | 15:45:14 |
| @joepie91:pixie.town | there is the @ case and the non-@ case | 15:45:17 |
| @joepie91:pixie.town | I am talking about the @ case, not about the non-@ case | 15:45:23 |
| @joepie91:pixie.town | string.contains is the non-@ case | 15:45:48 |
| @joepie91:pixie.town | there is presumably an @ case because otherwise this @-counting code would not exist | 15:46:03 |
| @emma:rory.gay | the @ case isnt really a real case either | 15:46:18 |
| @joepie91:pixie.town | and the @ case is implemented wrong in a very trivial-to-improve way that has nevertheless not been done | 15:46:23 |
| @joepie91:pixie.town | so then why is there @-counting code at all? | 15:46:33 |
| @emma:rory.gay | the only part where the @ case is relevant, is if they print the MXID (which was the first iteration of ping spam), which pings user because mxid.contains(user.localpart) == true | 15:46:59 |
| @joepie91:pixie.town | and so there would be something attached to the @ and so you should be checking for that, not just counting isolated @s | 15:47:42 |
| @emma:rory.gay | ideally, you'd check for just localparts and displaynames, yes | 15:48:01 |
| @joepie91:pixie.town | no, that is not what I said | 15:48:09 |
| @emma:rory.gay | oh, did you mean matching @.*:.*? | 15:48:35 |
| @emma:rory.gay | aka "is it a valid mxid" | 15:48:42 |
| @emma:rory.gay | * oh, did you mean matching /@.*:.*/? | 15:49:03 |
| @joepie91:pixie.town | yes, that is the kind of thing I am talking about. and if it's not certain that an @ is always followed by an mxid, then it could even be @\B (ie. @ followed by a non-boundary character) | 15:49:10 |
| @emma:rory.gay | i feel like this would be the most reliable way to handle it, tbh | 15:49:53 |
| @emma:rory.gay | unless you have a bunch of users that just have singular english words as localpart/displayname... | 15:50:11 |
| @emma:rory.gay | at which point, you've been scuntorphe'd | 15:50:31 |
| @joepie91:pixie.town | it would be, but the point I was trying to make above isn't that it's suboptimal, because that can easily be responded to with "well I didn't have the time to get it right". the point I was trying to make is that it would've taken like 5 more minutes and zero structural code changes to deal with the low-hanging fruit only by doing a simple regex check, and that even that wasn't done, and that this is a recurring problem with element/matrix core code | 15:51:34 |
| @joepie91:pixie.town | checking against a list of known users would be ideal but even without the resources to implement that (as it involves state management and so is a bit more work), there is still no excuse for the implementation as it exists today | 15:52:05 |
| @emma:rory.gay | tbh, this is the kind of thing they would definitely accept a PR for | 15:52:36 |
