Skip to content

fix: parsing underscores in the email of user mention #548

Merged
johnmlee101 merged 1 commit into
Expensify:mainfrom
eh2077:19957-fix-parsing-underscores-in-mention
Jun 13, 2023
Merged

fix: parsing underscores in the email of user mention #548
johnmlee101 merged 1 commit into
Expensify:mainfrom
eh2077:19957-fix-parsing-underscores-in-mention

Conversation

@eh2077

@eh2077 eh2077 commented Jun 13, 2023

Copy link
Copy Markdown
Contributor

Fixed Issues

$ Expensify/App#19957

Tests

  1. Go to a chat and add a comment @_concierge_@expensify.com
  2. Verify that App doesn’t crash and _concierge_ is not rendered as italic

QA

Same as test

@eh2077 eh2077 requested a review from a team as a code owner June 13, 2023 09:26
@melvin-bot melvin-bot Bot requested review from dangrous and removed request for a team June 13, 2023 09:27
@allroundexperts

Copy link
Copy Markdown
Contributor

@eh2077 Can we add this for other cases such as bold text?

@eh2077

eh2077 commented Jun 13, 2023

Copy link
Copy Markdown
Contributor Author

@eh2077 Can we add this for other cases such as bold text?

It's a good point. I also dug this but I found that this issue happens only on underscore _ because underscore _ is treated as word character in JavaScript regex but * and ~ are not word characters.

* and ~ are not valid characters for the email of user mention. For example, input like @*concierge*@expensify.com won’t be parsed as user mention because *concierge*@expensify.com is not a valid email.

So, I think we don't need to change the regex for bold and strikethrough rule.

@allroundexperts allroundexperts left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.
cc @dangrous

@dangrous

Copy link
Copy Markdown
Contributor

I'm happy to review this but it looks like it should be @johnmlee101 based on the linked issue?

@allroundexperts

Copy link
Copy Markdown
Contributor

I'm happy to review this but it looks like it should be @johnmlee101 based on the linked issue?

Correct @dangrous. If you can assign him, then that would be awesome!

@dangrous dangrous requested review from johnmlee101 and removed request for dangrous June 13, 2023 17:02
@johnmlee101 johnmlee101 merged commit 9dfabe1 into Expensify:main Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants