Skip to content

[String] Add support for emoji in AsciiSlugger #47264

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 17, 2022
Merged

[String] Add support for emoji in AsciiSlugger #47264

merged 1 commit into from
Aug 17, 2022

Conversation

lyrixx
Copy link
Member

@lyrixx lyrixx commented Aug 12, 2022 •
edited by nicolas-grekas
Loading

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

Usage

use Symfony\Component\String\Slugger\AsciiSlugger;

$slugger = new AsciiSlugger();
$slugger = $slugger->withEmoji();

$slugger->slug('a 😺, 🐈‍⬛, and a 🦁 go to 🏞️... 😍 🎉 💛', '-', 'en');
// a-grinning-cat-black-cat-and-a-lion-go-to-national-park-smiling-face-with-heart-eyes-party-popper-yellow-heart

$slugger->slug('un 😺, 🐈‍⬛, et un 🦁 vont au 🏞️');
// un-chat-qui-sourit-chat-noir-et-un-tete-de-lion-vont-au-parc-national

@carsonbot carsonbot added this to the 6.2 milestone Aug 12, 2022
@lyrixx lyrixx force-pushed the string-slug-emoji branch from bd775ba to 9e01528 Compare August 12, 2022 13:12
@lyrixx lyrixx force-pushed the string-slug-emoji branch from 9e01528 to f78a0a4 Compare August 13, 2022 08:41
@lyrixx
Copy link
Member Author

lyrixx commented Aug 13, 2022

@welcoMattic Thanks for the review, I addressed your comments

@nicolas-grekas

This comment was marked as resolved.

Sign in to view
@lyrixx lyrixx force-pushed the string-slug-emoji branch from f78a0a4 to dc48d45 Compare August 16, 2022 08:30
@lyrixx
Copy link
Member Author

lyrixx commented Aug 16, 2022

Hi, I addressed all comments. Thanks for the review

@lyrixx lyrixx force-pushed the string-slug-emoji branch from dc48d45 to a1ad331 Compare August 17, 2022 08:27
@ro0NL
Copy link
Contributor

ro0NL commented Aug 17, 2022

Related to #47263, does this mean we need to provision slugger with "en_github/slack" locale? Im not sure it's legit.

@lyrixx
Copy link
Member Author

lyrixx commented Aug 17, 2022

It's not legit IMHO, since the output of the transliterator may contains :

@lyrixx lyrixx force-pushed the string-slug-emoji branch from a1ad331 to 06a91c6 Compare August 17, 2022 10:02
@lyrixx
Copy link
Member Author

lyrixx commented Aug 17, 2022

@nicolas-grekas I addressed your comments

@lyrixx lyrixx force-pushed the string-slug-emoji branch from 06a91c6 to 9c88d02 Compare August 17, 2022 10:05
@ro0NL
Copy link
Contributor

ro0NL commented Aug 17, 2022

should we update

symfony/src/Symfony/Component/Intl/Transliterator/EmojiTransliterator.php

Line 52 in 68fe153

public static function create(string $id, int $direction = self::FORWARD): ?\Transliterator
with soft @return to convey it's not a nullable?

@nicolas-grekas
Copy link
Member

We can remove the ? actually. Up for a PR @ro0NL?

@nicolas-grekas
Copy link
Member

(deps=low need a bump for intl in composer.json)

@lyrixx lyrixx force-pushed the string-slug-emoji branch from 9c88d02 to d7b891a Compare August 17, 2022 12:18
@lyrixx lyrixx mentioned this pull request Aug 17, 2022
Merged
@lyrixx
Copy link
Member Author

lyrixx commented Aug 17, 2022

I fixed requirements for this PR, and also the return type of EmojiTransliterator while I had the code open / cc @ro0NL

@lyrixx lyrixx force-pushed the string-slug-emoji branch from d7b891a to 4009405 Compare August 17, 2022 12:23
nicolas-grekas added a commit that referenced this pull request Aug 17, 2022
@nicolas-grekas
minor #47297 [Intl] Fixed return type of EmojiTransliterator::create(…
6aed124 
…) and some others methods (lyrixx)

This PR was squashed before being merged into the 6.2 branch.

Discussion
----------

[Intl] Fixed return type of EmojiTransliterator::create() and some others methods

| Q             | A
| ------------- | ---
| Branch?       | 6.2
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | #47264 (comment)
| License       | MIT
| Doc PR        |

Commits
-------

770d590 [Intl] Fixed return type of EmojiTransliterator::create() and some others methods
@lyrixx lyrixx force-pushed the string-slug-emoji branch from 4009405 to ab20db7 Compare August 17, 2022 12:38
@nicolas-grekas
Copy link
Member

Thank you @lyrixx.

@nicolas-grekas nicolas-grekas merged commit 1931e87 into symfony:6.2 Aug 17, 2022
@lyrixx lyrixx deleted the string-slug-emoji branch August 17, 2022 13:04
@nicolas-grekas nicolas-grekas mentioned this pull request Aug 17, 2022
Closed
@fabpot fabpot mentioned this pull request Oct 24, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot left review comments

@gharlan gharlan gharlan left review comments

@stof stof stof left review comments

@welcoMattic welcoMattic welcoMattic left review comments

@ro0NL ro0NL ro0NL left review comments

@94noni 94noni 94noni left review comments

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@derrabus derrabus derrabus approved these changes

Assignees
No one assigned
Labels
Feature Status: Reviewed String
Projects
None yet
Milestone
6.2
Development

Successfully merging this pull request may close these issues.
10 participants
@lyrixx @nicolas-grekas @ro0NL @fabpot @gharlan @stof @welcoMattic @94noni @derrabus @carsonbot