search

WPTT / WPThemeReview

PHP_CodeSniffer rules (sniffs) to enforce WordPress theme review coding conventions
MIT License
209 stars 37 forks source link

NonPrintableCheck sniff #103

Closed selul closed 6 years ago

selul commented 7 years ago

Non printable chars sniffs for css,js and php files, implementing the themecheck plugin rule.

Reference #94

selul commented 7 years ago

@grappler @jrfnl I have added the requested changes. The build fails on PHP 7 as the T_BAD_CHARACTER is no longer available.

jrfnl commented 7 years ago

@selul Thanks for the update. I'll try and do another review run through tomorrow.

grappler commented 7 years ago

@jrfnl I checked with the Theme Check plugin and I am getting the same result which makes sense as the regex is the same but the in the theme check this is only a info, so it is not required.

It would be good to do some research on the subject. @selul would you be willing to help out.

Do symbols like °, ′ and ″ count as non-printable characters and what negative affect can they have?

grappler commented 7 years ago

I spoke to Otto and the reason this was added in the Theme Check was to check for Byte Order Mark. Explanation. This is now being checked with Generic.Files.ByteOrderMark

The check was made a bit looser to try to catch other issues too.

I did a bit of research, non printable characters are things like line breaks. In order for this sniff to work we would need to think of places where it does not make sense to have non printable characters. I am not sure how many places there are.

More of the ASCII codes can be found in the links 1-31 and 127-159 are the invisible characters. http://ascii.cl/htmlcodes.htm http://www.ascii-code.com/

jrfnl commented 6 years ago

Closing this PR in favour of continuing the discussion in the original issue #94.

Once that discussion has run its course, we can always reopen.