Details

No, these characters are not escaped on render. The only character which is going to be escaped is ], because there might be a chat name like this: Chat name ]] ]] and regex would not include all of closing brackets (that's why we need decodeChatMentionText and encodeChatMentionText, so when user clicks a chat in typeahead tooltip, before raw chat mention is saved, all ] characters are escaped (\])).

lib/shared/markdown.js
226 ↗	(On Diff #30070)	Regex explanation: `(?<!\\\\)` - Negative lookbehind assertion. It asserts that what precedes the current position in the string is not a backslash (\). This is used to ensure that there is no escaping backslash before the @ symbol. `(@\\[\\[([0-9,\|]+):(${validChatNameRegexString}?)(?<!\\\\)\\]\\])` - Main part of the regex. Inner negative lookbehind makes sure that if the user escapes `]` character, whole expression is not matched. For example this expression would not match: `@[[123:test\]]`.
229–231 ↗	(On Diff #30070)	Soon I will introduce `encodeChatMentionText` which will encode all `]` in chat mention text when chat candidate is clicked in typeahead tooltip.

• patryk published this revision for review.Aug 20 2023, 11:11 PM

Move regex and decode function to mention-utils.js

Harbormaster completed remote builds in B21940: Diff 30132.Aug 21 2023, 5:03 AM

Match/parse seems good, just one comment inline that I won't block this diff on.

Could you also maybe add some unit tests for the regex in markdown.test.js alongside this diff since it's pretty difficult to review? They don't need to be super complicated but since the regex itself is pretty hard to understand, it'd be a good way to make sure all cases are accounted for

lib/shared/markdown.js
229–237 ↗	(On Diff #30132)	For a relatively simple case like this, we could probably just handle it like the other simple cases directly in `web/markdown/rules.react.js` and `native/markdown/rules.react.js` using the `SimpleMarkdown.inlineRegex(SharedMarkdown.chatMentionRegex)` method Going to cc @ashoat though since this was feedback he gave to me in D5346 and I'm not sure if it's worth blocking this diff for

This revision is now accepted and ready to land.Aug 21 2023, 5:32 PM

ashoat added inline comments.Aug 21 2023, 9:18 PM

lib/shared/markdown.js
229–237 ↗	(On Diff #30132)	Ah yeah, good call there – I agree

Remove matchChatMentions

Harbormaster completed remote builds in B21974: Diff 30176.Aug 22 2023, 1:36 AM

Change id detection regex

Harbormaster completed remote builds in B22009: Diff 30220.Aug 22 2023, 7:13 AM

rohan mentioned this in D8944: [web] Modify mention typeahead regex.Aug 25 2023, 6:19 AM

Modify regex

Harbormaster completed remote builds in B22254: Diff 30551.Aug 30 2023, 7:00 AM

• patryk added a child revision: D9056: [lib] Add unit tests for chat mention regex.Aug 31 2023, 2:09 PM

Use idSchemaRegex

Harbormaster completed remote builds in B22332: Diff 30684.Sep 1 2023, 12:08 AM

• patryk mentioned this in D9061: [lib][native][web] Move validChatNameRegex to validation-utils.js.Sep 1 2023, 5:28 AM

inka requested changes to this revision.Sep 6 2023, 2:37 AM

inka added inline comments.

lib/shared/mention-utils.js
39 ↗	(On Diff #30684)	I don't think you want it to always be at the beginning of the line? If I'm right please amend D9057 to test for a mention indie of a message

This revision now requires changes to proceed.Sep 6 2023, 2:37 AM

ashoat added inline comments.Sep 11 2023, 1:12 PM

lib/shared/mention-utils.js
39 ↗	(On Diff #30684)	Not sure if this is relevant, but `simple-markdown` always parses RegExps at the "start of the line". It process tokens at the start of the text, and then removes those tokens before processing the new set of tokens

• patryk added inline comments.Sep 14 2023, 1:11 AM

lib/shared/mention-utils.js
39 ↗	(On Diff #30684)	@ashoat comment explains why this regex is checking if mention is at the beginning of the line. D9005 modifies `chatMentionRegexString` and removes `^`. We do sth similar for user mentions, see `markdownUserMentionRegex`. I can rename `chatMentionRegex` to `markdownChatMentionRegex` to distinguish "global" chat mention regex from markdown regex.