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

[UtilitiesBundle] Feature transliterator #1666

Merged
merged 2 commits into from Nov 29, 2017

Conversation

cv65kr
Copy link
Contributor

@cv65kr cv65kr commented Nov 20, 2017

Q A
Bug fix? no
New feature? improvment
BC breaks? yes
Deprecations? no
Fixed tickets no

I think we should use Behat/Transliterator is more accurate. It's made some BC breaks, but the second argument in slugify function never been used.

@cv65kr cv65kr changed the title [UtilitiesBundle][WIP] Feature transliterator [UtilitiesBundle] Feature transliterator Nov 20, 2017
*
* @return string
*/
public function slugify($text, $default = '', $replace = array("'"), $delimiter = '-')
public function slugify($text)
Copy link
Contributor

Choose a reason for hiding this comment

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

We can keep the delimiter here. The transliterate function https://github.com/Behat/Transliterator/blob/master/src/Behat/Transliterator/Transliterator.php#L431 also has a seperator as argument.

* @return mixed
*/
public function slugify($text, $default = 'n-a', $replace = array("'"), $delimiter = '-');
public function slugify($text);
Copy link
Contributor

Choose a reason for hiding this comment

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

Keep delimiter here to

@cv65kr
Copy link
Contributor Author

cv65kr commented Nov 24, 2017

@sandergo90 done.

@sandergo90 sandergo90 merged commit f02e368 into Kunstmaan:5.0 Nov 29, 2017
@krewetka
Copy link
Contributor

krewetka commented Aug 11, 2020

I just wanted to let you know that replacing previous slugifer using https://www.php.net/manual/en/transliterator.transliterate.php with behat caused arabic transliteration to change.

From what I see previous one was correct and new one is not.

Example:

الستائر - earlier was transliterated to alstayr not is transliterated to lstyr and in general all texts with this character https://www.fileformat.info/info/unicode/char/0627/index.htm

https://translate.google.com translated it to alsatayir ( even ore more a) but in general this character I mentioned (0627) should be probably be transliterated to A and behat transliterate replace it with empty character

@acrobat
Copy link
Member

acrobat commented Aug 11, 2020

Hi @krewetka, can you create a new issue for this bug and reference this PR? It makes it easier to keep track of the bug!

Can you also report this issue to behat/transliterator so it possibly can be fixed upstream? Thanks!

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

4 participants