Reviewed & tested by the community
Project:
Phone
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
12 Oct 2011 at 08:26 UTC
Updated:
28 Jul 2016 at 10:52 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
g1smd commentedThe code is inefficient.
For example, [0]{1}[6]{1} simplies to 06.
A character group is not needed.
# Phonenumber with 4 digit area code
- ([0]{1}[1-9]{1}[0-9]{2}[-\s]+[1-9]{1}[\s]*([0-9]{1}[\s]*){5})
+ ((?:\+31[ ]?|[0]{1}){1}[1-9]{1}[0-9]{2}[-\s]*[1-9]{1}[\s]*([0-9]{1}[\s]*){5})
All of the {1} counts can be removed; one is the default.
The [ ]? simplifies to \s?
Single character or digit in a group doesn't need to be in a group, e.g. [0] should be 0 and [\s] should be \s.
Comment #2
roderikThat patch did not properly match (bracket) things. It probably didn't spot that the pairs of two regexes it deleted from the original code, are slightly different.
It also didn't spot phone numbers that are too long, but just truncated them.
Here's an updated patch. It also matches bracketed area codes, like (010) 1234567. In order to support that, I had to pull the regex apart into 3 pieces, otherwise I got extra empty matches (I don't know why).
This is now the official notation (See the 2nd table on https://nl.wikipedia.org/wiki/Telefoonnummer) - but I did not dare to change the output, because maybe existing users of the module will not expect that. So the output is still 010-1234567.
Comment #3
roderikThe regex didn't match the dash in '+31-' yet, even though that's what the code outputs. Fixing.
Comment #4
BarisW commentedPatch applies cleanly and fixes all issues I had with the phone field. Great work Roderik!
Comment #5
idebr commentedThanks Roderik, #3++
Comment #6
absoludo commentedPatch still applies cleanly.
Can it be scheduled for the next release?