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

Added transliterator to email and username #333

Closed
wants to merge 5 commits into from
Closed

Added transliterator to email and username #333

wants to merge 5 commits into from

Conversation

fonsecas72
Copy link
Contributor

see #311
I've added transliteration to email and username.
I've added some tests that will make sure that every email and username from any provider are valid.

I've updated readme.md file to inform of the new intl and php dependency.
Should we be worried about those people that need transliteration (of any kind) but still use a lower PHP version?
For these people Faker will not work after the update as I've removed all "toAscii" methods.

@Anahkiasen
Copy link

What does this do?

@fonsecas72
Copy link
Contributor Author

@Anahkiasen This replaces "toAscii" methods by the PHP transliterator function.
See #311.

@fzaninotto
Copy link
Owner

Yes, you should provide an alternative implementation without intl, similar to what the toAscii() method did in the past. Something like:

private static function transliterate($string)
{
    if (function_exists('transliterator_transliterate')) {
        // fine, use intl
        // ...
    } else {
        // use the old special char replacement technique
        // with a unique replacement matrix for all locales
    }

    return $string;
}

That way, you can remove the part in README.md about intl being a requirement.

Also, transliterateEmail() shouldn't do an if, since the translitterate() method will always return the translitterated string.

Lastly, tests fail.

@fonsecas72
Copy link
Contributor Author

Despite the use of my best toAscii method I'm not able to make the tests go green. There are too many char to normalize.
For the record I've found some interesting things like this intl implementation by symfony
https://github.com/symfony/Intl
I don't know if we can use it though.
BTW, This site inspired my solution for this problem http://camendesign.com/code/transliteration

@fzaninotto
Copy link
Owner

Superseded by #490

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

Successfully merging this pull request may close these issues.

3 participants