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

[RFC] Implement mb_str_pad() #11284

Merged
merged 1 commit into from
Jun 20, 2023
Merged

[RFC] Implement mb_str_pad() #11284

merged 1 commit into from
Jun 20, 2023

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos requested a review from alexdowad as a code owner May 20, 2023 12:46
@nielsdos nielsdos added the RFC label May 20, 2023
@nielsdos nielsdos linked an issue May 20, 2023 that may be closed by this pull request
@nielsdos
Copy link
Member Author

@alexdowad The RFC is accepted, and I have rebased this PR onto the latest master branch. This PR is now ready for review.
I would be grateful if you could review this sometime in the near future. Thanks :)

@alexdowad
Copy link
Contributor

@nielsdos Congrats on successfully going through the RFC process.

@alexdowad
Copy link
Contributor

@nielsdos Just read through the code. I'm not sure about the spec for the new function, but I trust that was already fully discussed during the RFC process and the new tests reflect whatever was agreed. Aside from that, the implementation looks great. You write beautiful code. It reads very, very well.

@alexdowad
Copy link
Contributor

CI failure on MacOS is spurious.

@nielsdos
Copy link
Member Author

Thank you!

In general, the behaviour is the same as with str_pad, but instead of working on bytes it works on character codepoints like the other mbstring functions.
The behaviour regarding edge cases, uneven pad lengths, exceptions, ... are all the same as for str_pad.

@sneycampos
Copy link

@nielsdos sorry for tagging you but i have a question: when i try to use mb_str_pad with a null value it throws an error, since the str_pad doesn't thrown an error when a null value is passed as value, this is expected or may be considered breaking change?

E.g:

$value = null;
str_pad(null, 4, '0', STR_PAD_LEFT)
// returns 0000

$value = null;
mb_str_pad(0, 4, '0', STR_PAD_LEFT);
//TypeError mb_str_pad(): Argument #1 ($string) must be of type string, null given

@nielsdos
Copy link
Member Author

nielsdos commented Dec 2, 2023

@sneycampos I'm not sure I understand you code sample, you pass 0 as first argument of mb_str_pad, but even if I pass null it just throws a deprecation: https://3v4l.org/vYaqp This is the same behaviour as for str_pad.
Under strict mode both str_pad and mb_str_pad will throw an exception: https://3v4l.org/i6Fkl

@sneycampos
Copy link

Sorry, the zero was a typo. And i just noticed right now that i am not using php 8.3 for this tests, i'm using php 8.2 in a Laravel Project and this function call comes from symfony's polyfill 80, not from php 8.3 sc.

Thanks anyway and sorry for bothering you.

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.

New function mb_str_pad (multibyte str_pad)
3 participants