-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
DateTime.iso8601 fails with an error if a second fraction is present #2941
Conversation
Keep in mind that |
@BanzaiMan Yes, absolutely. Please correct me if I'm wrong, but MRI seems to do a similar thing: https://github.com/ruby/ruby/blob/trunk/ext/date/date_parse.c#L2363 |
@BanzaiMan in this case it is acceptable as date.rb has gone native somewhere along MRI 1.9.3 + this is a follow-up on a previous date/format.rb patch: b2e1dc1?diff=unified @azolotko test/spec will be fine under regressions ... just be aware to filter it out in 1.8 mode (still tested against under jruby-1_7) |
There seems to be some difference in meaning of |
Having the spec in spec/ruby (RubySpec) would be useful as well. |
@@ -937,7 +937,7 @@ def self._iso8601(str) # :nodoc: | |||
h[:sec] = i sec if sec | |||
end | |||
|
|||
h[:sec_fraction] = sec_fraction if sec_fraction | |||
h[:sec_fraction] = Rational(sec_fraction.to_i, 10**sec_fraction.size) if sec_fraction |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about Float("0#{sec_fraction}")
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cause of MRI compatibility:
1.9.3-p551 :002 > date = DateTime.iso8601('2014-07-08T17:51:36.013Z')
=> #<DateTime: 2014-07-08T17:51:36+00:00 ((2456847j,64296s,13000000n),+0s,2299161j)>
1.9.3-p551 :003 > date.sec_fraction
=> (13/1000)
1.9.3-p551 :004 > date.sec_fraction.class
=> Rational
1.9.3-p551 :005 > date.second_fraction
=> (13/1000)
cherry-picked commit on jruby-1_7 as 4e822a1 ... great work Alex, esp. that you matched MRI ... thanks. |
Fixes #2883
I would be happy to add a test, if I only knew where to put it.
Thanks!