Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Intl] Add a special locale to strip emojis easily with EmojiTransliterator #48396

Merged
merged 1 commit into from Dec 22, 2022

Conversation

fancyweb
Copy link
Contributor

@fancyweb fancyweb commented Nov 29, 2022

Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets -
License MIT
Doc PR -

I'd like to add a special locale strip to the emoji transliterator to easily remove all emojis from a string. It would only work with valid input/emojis.

Removing emojis from a string is not easy. Regexes are complicated and most of the time not up-to-date with the latest Unicode versions. I thought we could leverage the new transliterator 😄

Copy link
Member

@lyrixx lyrixx left a comment

Choose a reason for hiding this comment

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

I like it

@javiereguiluz
Copy link
Member

I like this feature (and I've needed it myself more than once ... I agree with what you said about regexes).

What I don't like is "hacking" this feature as a special non-real locale. This is a convention that developers must learn, so it makes it a bit more difficult. I wonder if we could add a removeEmojis() method in String component or any other method somewhere. Thanks.

@OskarStark OskarStark changed the title [Intl] Add a special locale to strip emojis easily with EmojiTransliterator [Intl] Add a special locale to strip emojis easily with EmojiTransliterator Dec 2, 2022
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

LGTM.

This is not really a locale but a transliteration rule. This belongs to the EmojiTransliterator to me.
(the patch demonstrates it quite strongly: there is no new code here, just a new map.)

@ro0NL
Copy link
Contributor

ro0NL commented Dec 18, 2022

i'd still be in favor of String integration, u($s)->stripEmoji()->toString()

@nicolas-grekas
Copy link
Member

i'd still be in favor of String integration, u($s)->stripEmoji()->toString()

That'd be 👎 on my side. I don't see why this specific translit rule would deserve such an important place.

@ro0NL
Copy link
Contributor

ro0NL commented Dec 18, 2022

i assumed our removeEmoji() removed emoji, but in fact it transliterates them to codes 🤦 (based on https://github.com/aaronpk/emoji-detector-php)

nevertheless i still think controlling the emoji at String level is nice to have

@nicolas-grekas
Copy link
Member

Thank you @fancyweb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants