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

[String] Add support for emoji in AsciiSlugger #47264

Merged

nicolas-grekas merged 1 commit into symfony:6.2 from lyrixx:string-slug-emoji

Aug 17, 2022

Member

lyrixx commented Aug 12, 2022 •

edited by nicolas-grekas

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 added Status: Needs Review Feature String labels

carsonbot added this to the 6.2 milestone

lyrixx force-pushed the string-slug-emoji branch from bd775ba to 9e01528 Compare

August 12, 2022 13:12

welcoMattic reviewed

View reviewed changes

src/Symfony/Component/String/Slugger/AsciiSlugger.php Outdated Show resolved Hide resolved

src/Symfony/Component/String/Tests/Slugger/AsciiSluggerTest.php Outdated Show resolved Hide resolved

src/Symfony/Component/String/Tests/Slugger/AsciiSluggerTest.php Outdated Show resolved Hide resolved

lyrixx force-pushed the string-slug-emoji branch from 9e01528 to f78a0a4 Compare

August 13, 2022 08:41

Member Author

lyrixx commented Aug 13, 2022

@welcoMattic Thanks for the review, I addressed your comments

derrabus approved these changes

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels

stof reviewed

View reviewed changes

src/Symfony/Component/String/Slugger/AsciiSlugger.php Outdated Show resolved Hide resolved

94noni reviewed

View reviewed changes

src/Symfony/Component/String/Tests/Slugger/AsciiSluggerTest.php Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

gharlan reviewed

View reviewed changes

src/Symfony/Component/String/Slugger/AsciiSlugger.php Outdated Show resolved Hide resolved

lyrixx force-pushed the string-slug-emoji branch from f78a0a4 to dc48d45 Compare

August 16, 2022 08:30

Member Author

lyrixx commented Aug 16, 2022

Hi, I addressed all comments. Thanks for the review

lyrixx commented

View reviewed changes

src/Symfony/Component/String/Slugger/AsciiSlugger.php Outdated Show resolved Hide resolved

lyrixx force-pushed the string-slug-emoji branch from dc48d45 to a1ad331 Compare

August 17, 2022 08:27

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.

Member Author

lyrixx commented Aug 17, 2022

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

nicolas-grekas reviewed

View reviewed changes

src/Symfony/Component/String/Slugger/AsciiSlugger.php Outdated Show resolved Hide resolved

src/Symfony/Component/String/Slugger/AsciiSlugger.php Outdated Show resolved Hide resolved

src/Symfony/Component/String/Tests/Slugger/AsciiSluggerTest.php Outdated Show resolved Hide resolved

lyrixx force-pushed the string-slug-emoji branch from a1ad331 to 06a91c6 Compare

August 17, 2022 10:02

Member Author

lyrixx commented Aug 17, 2022

@nicolas-grekas I addressed your comments

lyrixx force-pushed the string-slug-emoji branch from 06a91c6 to 9c88d02 Compare

August 17, 2022 10:05

nicolas-grekas approved these changes

View reviewed changes

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?

Member

nicolas-grekas commented Aug 17, 2022

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

Member

nicolas-grekas commented Aug 17, 2022

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

lyrixx force-pushed the string-slug-emoji branch from 9c88d02 to d7b891a Compare

August 17, 2022 12:18

lyrixx mentioned this pull request

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

Merged

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 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


          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


          [String] Add support for emoji in AsciiSlugger

ab20db7

lyrixx force-pushed the string-slug-emoji branch from 4009405 to ab20db7 Compare

August 17, 2022 12:38

Member

nicolas-grekas commented Aug 17, 2022

Thank you @lyrixx.

nicolas-grekas merged commit 1931e87 into symfony:6.2

lyrixx deleted the string-slug-emoji branch

August 17, 2022 13:04

nicolas-grekas mentioned this pull request

[String] Add support for emoji in AsciiSlugger symfony/symfony-docs#17193

Closed

fabpot mentioned this pull request

Release v6.2.0-BETA1 #47972

Merged

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

Reviewers

fabpot fabpot left review comments

gharlan gharlan left review comments

stof stof left review comments

welcoMattic welcoMattic left review comments

ro0NL ro0NL left review comments

94noni 94noni left review comments

nicolas-grekas nicolas-grekas approved these changes

derrabus derrabus approved these changes