Skip to content

Avoid changing the mbstring encoding when useless in the Parser#321

Merged
stof merged 1 commit intoscssphp:masterfrom
stof:improve_parser_check
Mar 13, 2021
Merged

Avoid changing the mbstring encoding when useless in the Parser#321
stof merged 1 commit intoscssphp:masterfrom
stof:improve_parser_check

Conversation

@stof
Copy link
Copy Markdown
Member

@stof stof commented Mar 12, 2021

The Parser does not use mbstring itself. The only reason to change the encoding is to ensure compatibility with the mbstring.func_overload feature of PHP, as we rely on the fact that standard PHP functions are dealing with bytes, not with multibyte chars (which would not be compatible with preg_match offsets which are not overloaded).
There is no need to change that global state if that obsolete PHP feature is not enabled.

The Parser does not use mbstring itself. The only reason to change the
encoding is to ensure compatibility with the mbstring.func_overload
feature of PHP, as we rely on the fact that standard PHP functions are
dealing with bytes, not with multibyte chars (which would not be
compatible with preg_match offsets which are not overloaded).
There is no need to change that global state if that obsolete PHP
feature is not enabled.
private function saveEncoding()
{
if (\extension_loaded('mbstring')) {
if (\PHP_VERSION_ID < 80000 && \extension_loaded('mbstring') && (2 & (int) ini_get('mbstring.func_overload')) > 0) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the PHP version check is because the feature is gone entirely in PHP 8.0 (it is deprecated since PHP 7.2)

@stof stof added this to the 1.5 milestone Mar 12, 2021
@stof stof merged commit 4d44139 into scssphp:master Mar 13, 2021
@stof stof deleted the improve_parser_check branch March 13, 2021 08:34
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.

1 participant