Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Don't consider textual characters to be emoji #12580

Closed

Conversation

robintown
Copy link
Member

We were using emojibase-regex to match emoji within messages. However, the docs state that this regex matches both emoji and text presentation characters. This is not what we want, and will result in false positives for characters like '↔' that could turn into an emoji if paired with a variation selector. The emojibase-regex/emoji regex from the same package does what we want.

We were using emojibase-regex to match emoji within messages. However, the docs (https://emojibase.dev/docs/regex/) state that this regex matches both emoji and text presentation characters. This is not what we want, and will result in false positives for characters like '↔' that could turn into an emoji if paired with a variation selector. The emojibase-regex/emoji regex from the same package does what we want.
@robintown robintown added the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label Jun 5, 2024
@robintown robintown requested a review from a team as a code owner June 5, 2024 20:35
@robintown robintown marked this pull request as draft June 5, 2024 20:51
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

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

this would likely benefit from a linter rule to avoid the same landmine

@robintown
Copy link
Member Author

Turns out that none of Emojibase's regexes actually match all emoji, and only emoji, and neither does the other popular emoji regex JS library :( I'll be closing this for now.

@robintown robintown closed this Jun 5, 2024
@robintown
Copy link
Member Author

milesj/emojibase#174

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants