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

fixed #85

Closed
wants to merge 1 commit into from
Closed

fixed #85

wants to merge 1 commit into from

Conversation

zlx
Copy link

@zlx zlx commented Jan 23, 2013

modify x4e.yml ~ x9f.yml and xf9.yml ~ xfa.yml
reference: 匹配中文的正则表达片段

@rsl
Copy link
Owner

rsl commented Jan 23, 2013

would you mind writing some tests? i'm really reluctant to merge a commit that touches so many files with no tests. i'm also concerned about the possibility of this having more spaces than desired in transliterations that don't contain english strings. also... [heh] i see some strings like "如果你想寻找 Ruby 的三方库" where there are spaces around the non-chinese phrases but some that don't have them. are both valid? i don't know anything about chinese [sorry] just curious on the last one. it looks like it shouldn't be a problem. i think line 73 of string_extensions_test.rb is chinese but that's a short phrase. if you could make that a little longer [with something valid, hehe i'm sure mine is utter crap] and an example with embedded english this looks good tbh.

@zlx
Copy link
Author

zlx commented Jan 24, 2013

  • Your are right. It's dangerous to merge a commit with many files modify without tests.
  1. In this commit, I mainly add a space before every chinese word in x4e.yml ~ x9f.yml and xf9.yml ~ xfa.yml .
  2. The affected methods are String#to_url and Unidecoder::decode, And The affected tests are line 83, 91, 93 of string_extensions_test.rb and line 45, 47 of unidecoder_test.rb which contain chinese words.
  3. I fixed the failed test and add test on line 75 of string_extensions_test.rb point the issue Issue #84: puzzle for mixture of Chinese and English
  4. I think "如果你想寻找 Ruby 的三方库" is a good style. and "如果你想寻找Ruby的三方库" is valid too. so each of them call to_url should return ru-guo-ni-xiang-xun-zhao-ruby-de-san-fang-ku. do you think so?
  5. may be call squish at last of Unidecoder::decode is a good way to remove extra spaces I know little about languages except english and chinese and don't know if it may bring new issue after that.[sorry]

I'm happy to discuss with you.

@rsl
Copy link
Owner

rsl commented Jan 24, 2013

i'd say squish on to_url seems like a safe thing. i can't imagine a language where the number of spaces makes a linguistic change. heh

can you tell if those changes to the tests in the chinese [and japanese] are cases where i was missing spaces before or it's inserting them in a weird place? i have to admit i cargo-culted those phrases and transliterations.

@zlx
Copy link
Author

zlx commented Jan 25, 2013

I agree with you. space can be ignored in language

In chinese, a word is Independent

I ask my co-worker who know japanese, the chinese words in japanese are different with them in chinese. so 私はガラスを食べられます。それは私を傷つけません。 => si-ha-garasu-wo-shi-beraremasu-soreha-si-wo-shang-tukemasen may be not a good result

@zlx
Copy link
Author

zlx commented Jan 31, 2013

Do you consider merge the patch or any idea to fix the bug by other way?

@rsl
Copy link
Owner

rsl commented Jan 31, 2013

i'm nervous about the changes in the japanese. need to find someone who knows more about that transliteration than i do however i have a lot of work on my plate so i'm not able to find anyone currently so this isn't going to be merged any time soon. if you think there's a problem with that, you might try some other approaches that don't change as many of the other language transliterations. not sure.

@rsl
Copy link
Owner

rsl commented Mar 15, 2013

@lassebunk added localization implementation that rocks. i thnk this would be better implemented there. would you want to do a go-round on a chinese localization? closing this for functionality but welcome to continue discussion about chinese localization here.

@rsl rsl closed this Mar 15, 2013
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

2 participants