Skip to content

Capital letter detection for multibyte strings doesn't work correctly #855

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

Closed
wants to merge 1 commit into from
Closed

Capital letter detection for multibyte strings doesn't work correctly #855

wants to merge 1 commit into from

Conversation

DaazKu
Copy link

@DaazKu DaazKu commented Jan 8, 2016

Accessing string as array actually cut multibyte characters.

That description would pass the test but it shouldn't.

/**
 * étude des ...
 */

Rewrite the regexes and compare the entire string.

@DaazKu DaazKu closed this Jan 8, 2016
@aik099
Copy link
Contributor

aik099 commented Jan 9, 2016

Why you've closed this?

@gsherwood
Copy link
Member

Why you've closed this?

Maybe because it breaks the unit tests.

The original issue is caused by preg_match returning false, although I don't know what the error is.

@gsherwood
Copy link
Member

although I don't know what the error is.

Probably because it's only getting the first byte of the multibyte character rather than a regex issue.

@gsherwood
Copy link
Member

I think I've found the original problem and a possible fix. I'll just turn this into a bug report.

@gsherwood gsherwood changed the title Fix comments capital letter detection for multibyte string. Capital letter detection for multibyte strings doesn't work correctly Jan 11, 2016
@DaazKu
Copy link
Author

DaazKu commented Jan 11, 2016

Ya sorry my fixe broke unit test I have have no time to investigate further.

I mostly changed preg_match('/\p{Lu}|\P{L}/u', $string[0]) to preg_match('/^(\p{Lu}|\P{L})/u', $string) as you can see in files changed.

@gsherwood
Copy link
Member

@DaazKu Thanks. Your PR was very helpful in finding the problem.

gsherwood added a commit that referenced this pull request Jan 11, 2016
jrfnl added a commit to jrfnl/WordPress-Coding-Standards that referenced this pull request May 2, 2016
A bugfix upstream now causes `Doc comment short description must start with a capital letter` errors for `/* translators: ... */` comments.

Excluding the offending rule to continue to allow these type of comments.

Ref: squizlabs/PHP_CodeSniffer#855
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.

3 participants