Skip to content
This repository

small fix for south korean numbers #29

Merged
merged 1 commit into from over 2 years ago

2 participants

Andrew Vilcsak Florian R. Hanke
Andrew Vilcsak

fixes the case where the mobile prefix combined with the first digit of the mobile number causes the number to be identified as a 'service' number. for example, 821087971234 was being identified as 82 108 797 123 (a service number with a 108 prefix) rather than 82 10 8797 1234 (a mobile number with a 10 prefix)

Florian R. Hanke floere merged commit 2f972a4 into from
Florian R. Hanke floere closed this
Florian R. Hanke
Owner

Thank you very much! The bug is that service numbers are now matched as a whole. The service number is actually only "108", not a prefix. My bad.
I'm going to pull it in, distinguish between prefixes and actual service (short) numbers and release very soon.

Andrew Vilcsak

oh, my misunderstanding as to what a service number was. thanks again!

Florian R. Hanke
Owner

Same problem here! I mixed "service" and "special" numbers.

1.5.1 is released, with your fix. Many thanks!

Florian R. Hanke
Owner

I've just opened an issue regarding Phony being more forgiving in these cases, so that at least someone can see the whole number, even if it's formatted incorrectly.
#30
I don't feel fine when users of web applications notice at "call time" (phone call time) that a digit is missing. So I think the way to go is make phony more forgiving, even when that means we might not notice formatting errors anymore, as not many people will complain of bad formatting. (But I'd rather have less really bad cases – not being able to contact someone versus not so nice looking)
Do you have an opinion on this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
4  lib/phony/countries/south_korea.rb
@@ -9,7 +9,7 @@
9 9
 mobile  = ('10'..'19').to_a
10 10
 
11 11
 Phony.define do
12  
-  country '82', one_of(*service)              >> split(3,3) |
13  
-                one_of(*mobile)               >> split(4,4) |
  12
+  country '82', one_of(*mobile)               >> split(4,4) |
  13
+                one_of(*service)              >> split(3,3) |
14 14
                 one_of('2', :max_length => 2) >> split(4,4) # Seoul
15 15
 end
1  spec/lib/phony/countries_spec.rb
@@ -162,6 +162,7 @@
162 162
       Phony.split('82212345678').should == ['82', '2', '1234', '5678']   # Seoul
163 163
       Phony.split('825112345678').should == ['82', '51', '1234', '5678'] # Busan
164 164
       Phony.split('821027975588').should == ['82', '10', '2797', '5588'] # mobile
  165
+      Phony.split('821087971234').should == ['82', '10', '8797', '1234'] # mobile
165 166
     end
166 167
     it 'handles thai numbers' do
167 168
       Phony.split('6621231234').should == ['66', '2', '123', '1234'] # Bangkok
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.