Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Bugfix: mbstring polyfills must not raise value errors in PHP 7#501

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
derrabus wants to merge1 commit intosymfony:1.x
base:1.x
Choose a base branch
Loading
fromderrabus:bugfix/mbstring-value-error

Conversation

@derrabus
Copy link
Member

@derrabusderrabus commentedSep 17, 2024
edited
Loading

Fixes#499.

This PR changes all new mbstring polyfills on PHP 7 so that they trigger an oldschool PHP warning instead of raising aValueError. The reason for that is that theValueError class might not be available on PHP 7 and the behavior is more consitent with the other polyfills of the mbstring extension.

The somewhat weird side-effect is that we now polyfill a behavior that was never implemented in PHP.

@derrabusderrabusforce-pushed thebugfix/mbstring-value-error branch from847748a tob2da01dCompareSeptember 17, 2024 08:43
@derrabusderrabus mentioned this pull requestSep 17, 2024
}

if (!function_exists('mb_str_pad')) {
functionmb_str_pad(string$string,int$length,string$pad_string ='',int$pad_type =STR_PAD_RIGHT, ?string$encoding =null):string {returnp\Mbstring::mb_str_pad($string,$length,$pad_string,$pad_type,$encoding); }

Choose a reason for hiding this comment

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

what about a bootstrap80.php file to add the return types when running on PHP8+?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

That file already exists. ✌🏻

nicolas-grekas reacted with hooray emoji
Copy link
Member

@nicolas-grekasnicolas-grekasSep 18, 2024
edited
Loading

Choose a reason for hiding this comment

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

ah indeed :)
there are some failures to fix ;)

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Can you help we with those? Apparently, the test listener is skipping tests at a strange point in time which conflicts with theexpectWarning() feature of PHPUnit. How do we solve that usually?

@derrabusderrabusforce-pushed thebugfix/mbstring-value-error branch fromb2da01d to94414f3CompareSeptember 18, 2024 10:19
@nicolas-grekas
Copy link
Member

Let's also address#506 here?

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

+1 more reviewer

@jdreesenjdreesenjdreesen left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ValueError class not available in PHP < 8

3 participants

@derrabus@nicolas-grekas@jdreesen

[8]ページ先頭

©2009-2025 Movatter.jp