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

"Mongolian Vowel Separator" is not considered whitespace after Ruby 2.2 #9

Merged
merged 3 commits into from
Aug 3, 2015

Conversation

tjschuck
Copy link
Contributor

Ruby Issue 9092 updated Ruby from Unicode 6.1 to Unicode 7.0 (commit here). This patch was included in Ruby 2.2.

From the Unicode 6.3.0 release notes:

The General_Category property value of U+180E MONGOLIAN VOWEL SEPARATOR has been changed from Zs to Cf. The values of other related properties such as Bidi_Class, White_Space, and Other_Default_Ignorable_Code_Point have been updated accordingly.

After 2.2 shipped, Ruby's [[:space:]] Regexp expression no longer matches U+180E.

This updates fast_blank to treat U+180E properly in blank_as?.

Prior to this change, the tests fail on 2.2.2:

1) String provides a parity with active support function
   Failure/Error: expect("#{i.to_s(16)} #{c.blank_as?}").to eq("#{i.to_s(16)} #{c.blank2?}")

     expected: "180e false"
          got: "180e true"

     (compared using ==)
   # ./spec/fast_blank_spec.rb:22:in `block (3 levels) in <top (required)>'
   # ./spec/fast_blank_spec.rb:19:in `times'
   # ./spec/fast_blank_spec.rb:19:in `block (2 levels) in <top (required)>'

It now passes on all the Rubies in the Travis matrix (RBX, 1.9, 2.0, 2.1, and 2.2).

tjschuck added 2 commits July 31, 2015 16:32
Compiling generates "lib/fast_blank.bundle", which should not be checked in.
@tjschuck
Copy link
Contributor Author

Ugh, apparently I lied about Rubinius. I assume it doesn't like the RUBY_API_VERSION_* MRI internals. I'll look into it...

[Ruby Issue 9092](https://bugs.ruby-lang.org/issues/9092) updated Ruby from Unicode 6.1 to Unicode 7.0 ([commit here](ruby/ruby@64c81e4#diff-67d181a69374a75e4b8f706fa9b81fbc)).  This patch was included in Ruby 2.2.

From the [Unicode 6.3.0 release notes](http://www.unicode.org/versions/Unicode6.3.0/):

    The General_Category property value of U+180E MONGOLIAN VOWEL SEPARATOR has been changed from Zs to Cf. The values of other related properties such as Bidi_Class, White_Space, and Other_Default_Ignorable_Code_Point have been updated accordingly.

After 2.2 shipped, Ruby's [[:space:]] Regexp expression [no longer matches U+180E](https://github.com/ruby/ruby/blob/9fefa6063797f94704c09663db13cea9e390eaba/enc/unicode/name2ctype.h#L2740-L2753).

This updates fast_blank to treat U+180E properly in blank_as?

Prior to this change, the tests fail on 2.2.2:

    1) String provides a parity with active support function
       Failure/Error: expect("#{i.to_s(16)} #{c.blank_as?}").to eq("#{i.to_s(16)} #{c.blank2?}")
    -
         expected: "180e false"
              got: "180e true"
    -
         (compared using ==)
       # ./spec/fast_blank_spec.rb:22:in `block (3 levels) in <top (required)>'
       # ./spec/fast_blank_spec.rb:19:in `times'
       # ./spec/fast_blank_spec.rb:19:in `block (2 levels) in <top (required)>'

It now passes on all the Rubies in the Travis matrix (RBX, 1.9, 2.0, 2.1, and 2.2).
@tjschuck tjschuck force-pushed the mongolian_vowel_separator branch from 3e71e6b to 9cf94b4 Compare August 2, 2015 04:24
@tjschuck
Copy link
Contributor Author

tjschuck commented Aug 2, 2015

Okay, after some rejiggering, this now also works on Rubinius.

In hindsight, I suppose a better title for this PR is "Adds Support for Ruby 2.2.2". But now you know all about Mongolian vowel separators ¯\_(ツ)_/¯

@SamSaffron
Copy link
Owner

awesome, thanks for your hard work!

SamSaffron added a commit that referenced this pull request Aug 3, 2015
"Mongolian Vowel Separator" is not considered whitespace after Ruby 2.2
@SamSaffron SamSaffron merged commit 90a826b into SamSaffron:master Aug 3, 2015
@SamSaffron
Copy link
Owner

pushed a version 1.0.0 of fast_blank ... it deserves stable been in production for so long :)

@tjschuck tjschuck mentioned this pull request Aug 11, 2015
@pusewicz
Copy link

Woot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants