| 1 May 2025 |
@emma:rory.gay | tl;dr it splits the string by litteral @ characters, and checks if the result count is above the limit | 15:38:47 |
@joepie91:pixie.town | yes, and that's a terrible way to check this... | 15:38:54 |
@emma:rory.gay | yes, the correct way would be to grab the member list, and count instances of user.displayname ?? user.mxid | 15:39:19 |
@emma:rory.gay | * yes, the correct way would be to grab the member list, and count instances of user.displayname ?? user.mxid | 15:39:22 |
@emma:rory.gay | and well, obviously thats very slow because extra api call | 15:39:36 |
@joepie91:pixie.town | like, this feels really illustrative for the problem with a lot of element/matrix code, actually. someone implemented the absolute most naive implementation of the idea, even though with barely any extra effort they could've substantially improved the reliability, but they just didn't do that step at all | 15:40:18 |
@joepie91:pixie.town | I've been seeing this in so many places | 15:40:32 |
@emma:rory.gay | well, originally they didnt use displayname so it was actually sensible | 15:40:41 |
@joepie91:pixie.town | no it wasn't | 15:40:47 |
@joepie91:pixie.town | there are no circumstances under which this check is a reasonable implementation | 15:40:56 |
@joepie91:pixie.town | I literally provided the improved version above | 15:41:05 |
@emma:rory.gay | besides, friendly reminder that you dont need a word boundary | 15:41:18 |
@emma:rory.gay | joepie91 🏳️🌈Yorusaka Miyabi [DO NOT DM]WeetHet | 15:41:45 |
@joepie91:pixie.town | I feel like you keep missing my point here | 15:41:49 |
@emma:rory.gay | maybe i am, but i dont see a point besides "you should add word boundary checks" | 15:42:34 |
@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 |