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

[9.x] Str::squish trim ZWNBSP #41971

Merged
merged 2 commits into from
Apr 14, 2022
Merged

[9.x] Str::squish trim ZWNBSP #41971

merged 2 commits into from
Apr 14, 2022

Conversation

allowing
Copy link
Contributor

@taylorotwell taylorotwell merged commit 7e7379c into laravel:9.x Apr 14, 2022
@nshiro
Copy link
Contributor

nshiro commented Apr 14, 2022

@allowing
Hello! I saw your comment today.

Nice PR, but I am a little confused.
I thought this squish is supposed to cut spaces from the string in the middle, too.
If so, don't we need "ZWNBSP" in the first preg_replace() too for consistency ?

#41791
#41877
#41924

@allowing
Copy link
Contributor Author

@nshiro
嗯对的,第一个也应该加上。感谢你的提醒。

其实通过枚举的方式来实现,有点Hack的味道。没办法,'\s'并不完全包含不可见字符。

Well yes, the first one should also be added. Thank you for your reminder.

In fact, it is implemented by enumeration, which is a bit like Hack. No way, '\s' doesn't exactly contain invisible characters.

@allowing
Copy link
Contributor Author

Oh!

The ZWNBSP cannot be replaced with spaces. Because it's called Zero Width No-Break SPace. It doesn't take up space.

@allowing allowing deleted the squish branch April 15, 2022 02:35
@nshiro
Copy link
Contributor

nshiro commented Apr 15, 2022

Ah, things are not so simple.. 😢

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.

4 participants