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

Remove intl platform dependency #99

Merged
merged 1 commit into from
Jan 23, 2021
Merged

Remove intl platform dependency #99

merged 1 commit into from
Jan 23, 2021

Conversation

pookmish
Copy link
Contributor

@pookmish pookmish commented Jan 21, 2021

Is the intl extension required for anything? I see it was added in this commit but I don't see any function calls on the intl package.

I'm asking because when using this library on a Drupal 9 site, our hosting provide does not have the intl extension available and they are unable to enable it.

@seboettg
Copy link
Owner

Hi @pookmish!
Thank you for this PR. I guess we need "intl" for "symfony/polyfill-mbstring" which is a fallback for PHP's multibyte extension mbstring. When both intl and mbstring are missing, I'm not sure if citeproc-php still works properly with languages that need more characters from UTF-8 than the ASCII part.

@pookmish
Copy link
Contributor Author

Hmm... that does make sense if both are missing. FWIW, Drupal requires mbstring as a platform dependency already.

I see that symfony/polyfill-mbstring doesn't require intl but it does make a suggestion for mbstring. Is that based on a previous version of the symfony library?

Would it work to just move intl to the suggested part of composer instead?

@seboettg seboettg merged commit 469ac46 into seboettg:master Jan 23, 2021
@seboettg
Copy link
Owner

I merged your PR and have moved ext-intl to the suggest section of the composer file. I hope everything still works properly for all other citeproc-php users...

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 this pull request may close these issues.

None yet

2 participants