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

Not compatable with Laravel 9 #69

Closed
emedchill opened this issue Apr 25, 2022 · 7 comments · Fixed by #70
Closed

Not compatable with Laravel 9 #69

emedchill opened this issue Apr 25, 2022 · 7 comments · Fixed by #70

Comments

@emedchill
Copy link

Since Laravel 9 is requiring voku/portable-ascii:^2.0 and this repo is requiring voku/portable-ascii:^1.4 it causes a conflict when trying to update composer.

@lux
Copy link
Collaborator

lux commented Apr 25, 2022

I've started a branch to fix this but looks like the update breaks some test fixtures so I'll have to go through and fix those before merging.

@madman-81
Copy link
Contributor

@lux, do you have an update for us? Really looking forward to Laravel 9 support! Anything we can do to help?

@lux
Copy link
Collaborator

lux commented Jun 7, 2022

@madman-81 I updated the requirement to voku/portable-ascii:^2.0 which you can see in this broken PR but a bunch of test fixtures broke and I haven't had time to figure out which ones need updating. I have half a mind to remove most of them if they're already covered in that underlying library but also haven't had time to look into that possibility.

Any help on getting those tests passing would be great since I'd love to get back to compatible with Laravel and the latest Voku dependencies 😄

@madman-81
Copy link
Contributor

@lux, I'm not familiar with the portable-ascii package, but from what I see in the code and the change log, they changed the way Russian characters are converted by replacing "GOST 7.79-2000(B)" with "Passport (2013), ICAO", amongst others. This also has an effect on other, similar, languages like Bulgarian. Not sure if this is expected behaviour, but since there is no 'source language' specified when strings are converted, this might indeed happen (that Russian characters are used when a Bulgarian sentence is converted).

If you look at this Bulgarian sentence "Bulgarian: Жълтата дюля беше щастлива, че пухът, който цъфна, замръзна като гьон", it was converted to "bulgarian-zltata-dyulya-bese-shhastliva-ce-puxt-koito-cfna-zamrzna-kato-gyon" with the 1.6 version, but with version 2.0 it would become "bulgarian-zieltata-diulia-bese-shhastliva-ce-puxiet-koito-ciefna-zamriezna-kato-gyon".

In a bit more detail: the "ъ" character is now converted to "ie" (used to be "l"). This matches with the GOST/Passport change:
src/voku/helper/data/ascii_by_languages.php on line 1223 in the new version (the link isn't fully working some how).

So the first question that needs to be answered is: Is it OK that the new version of voku/portable-ascii will change the output that is generated?

@madman-81
Copy link
Contributor

@lux Any thoughts on this?

@jbroadway
Copy link
Owner

@madman-81 I think it's okay for the output to change since the best practice would be to generate the slug one time and store it somewhere instead of generating it fresh for every request. That wouldn't be great for performance reasons. So I think we're safe to update things to match the underlying voku/portable-ascii changes.

@madman-81
Copy link
Contributor

@lux, I've updated the expected output. See PR #71

@lux lux closed this as completed in #70 Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants