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

Preg best practices #81

Merged
merged 9 commits into from
Aug 19, 2013
Merged

Preg best practices #81

merged 9 commits into from
Aug 19, 2013

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Jul 8, 2013

Applied best practices for regular expressions:

Checked all use of PHP preg_* functions for some best practices and consistency:

  • Use "" as delimiter instead of "/" - "" is a very unlikely character to ever be used in text and therefore a good delimiter
  • Use of preg_quote() if part of the pattern is unknown (user-input/database)
  • Use of the u modifier if the pattern may contain utf-8 characters. I did notice it already being there in some instances, even when not needed. I've not removed it if it was there and not needed, just to be on the safe side.
  • Adjusted some patterns to remember only what's needed, removing superfluous braces () or adding the ?: modifier to forget the unneeded part of the match
  • Adjusted some regex for ambuiguity: "https?" - modifiers are greedy by default, so the ? in this case could extend to more than just the s, modified to: "http[s]?"
  • Only use regex when it's really needed, sometimes there are better solutions, applied a number of times.

N.B.: I couldn't find any unit tests to run on the changes - are there any available ? especially with regex, very useful to have ;-)

Open:

  1. There are a number of places where html tag attributes are checked. Not all regexes allow for the ' character to quote the attribute value. The important ones seem to do.
    Not sure if this has ever been an issue, but it might be an idea to verify/evaluate those regexes again.
  2. In a few places I wasn't sure whether adjusting the regex would lead to the same results. I've added some suggestions for improvements in the form of @todo comments with my initials [JRF].

jrfnl added a commit that referenced this pull request Aug 19, 2013
@jrfnl jrfnl merged commit d901653 into Yoast:master Aug 19, 2013
@jrfnl jrfnl deleted the preg-best-practices branch August 19, 2013 02:29
hansjovis pushed a commit that referenced this pull request Nov 21, 2022
hansjovis pushed a commit that referenced this pull request Nov 21, 2022
Add Slack sharing to customization sidebar
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.

None yet

1 participant