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

[Form] Do not trim unassigned unicode characters#43031

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

Merged

Conversation

@simonberger
Copy link
Contributor

@simonbergersimonberger commentedSep 14, 2021•
edited
Loading

QA
Branch?4.4
Bug fix?yes
New feature?no
Deprecations?no
LicenseMIT

I faced a problem with StringUtil::trim in PHP <7.3 and added a test only (for now).
I probably should have created an issue instead but I thought it is better to look at the result of a test and make sure it actually is a problem.

It is caused by\pC from the preg_replace.
I am no expert for UTF-8 whitespace and cannot suggest a good bugfix. Are you interested to work around the buggy behavior of the older PHP versions? Otherwise I'll close this.

@carsonbotcarsonbot added this to the4.4 milestoneSep 14, 2021
@carsonbotcarsonbot changed the titleAdd StringUtil::trim test case for an emoji[Form] Add StringUtil::trim test case for an emojiSep 14, 2021
@simonbergersimonberger changed the title[Form] Add StringUtil::trim test case for an emoji[Form] Add failing StringUtil::trim test case trimming an emojiSep 14, 2021
@derrabusderrabus added the Help wantedIssues and PRs which are looking for volunteers to complete them. labelSep 14, 2021
@derrabus
Copy link
Member

Thank you for the test case. Yes, I think, we should fix that.

@nicolas-grekas
Copy link
Member

PCRE is compiled with Unicode tables, older versions have outdated tables. I don't know what we can do about this nor how we can work around it in a reasonable way...
Any idea?

@simonberger
Copy link
ContributorAuthor

As far as I understand the problem it is about removing invisible characters, where the old PCRE version probably does this for all ranges it did not know at that time.
Can we instead use some ranges of really invisible characters instead of\pC? Or do this just for PHP <7.3 if it is safe for the other versions.

@simonbergersimonberger changed the title[Form] Add failing StringUtil::trim test case trimming an emoji[Form] Do not trim unassigned unicode charactersSep 15, 2021
@simonberger
Copy link
ContributorAuthor

@nicolas-grekas I re-requested your approval because I removed the Surrogate and Private Use character categories as well. I think those should not be relevant or if they are, trimming would be more likely a wrong move.

@nicolas-grekas
Copy link
Member

Thank you@simonberger.

@nicolas-grekasnicolas-grekas merged commit6fdf0c9 intosymfony:4.4Sep 17, 2021
@fabpotfabpot mentioned this pull requestSep 28, 2021
@fabpotfabpot mentioned this pull requestSep 28, 2021
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@derrabusderrabusderrabus left review comments

@xabbuhxabbuhAwaiting requested review from xabbuhxabbuh is a code owner

@ycerutoycerutoAwaiting requested review from ycerutoyceruto is a code owner

@nicolas-grekasnicolas-grekasAwaiting requested review from nicolas-grekas

Assignees

No one assigned

Labels

BugFormHelp wantedIssues and PRs which are looking for volunteers to complete them.Status: Needs Review

Projects

None yet

Milestone

4.4

Development

Successfully merging this pull request may close these issues.

4 participants

@simonberger@derrabus@nicolas-grekas@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp