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

Time parse error in master #6093

Closed
kostya opened this issue May 12, 2018 · 6 comments
Closed

Time parse error in master #6093

kostya opened this issue May 12, 2018 · 6 comments

Comments

@kostya
Copy link
Contributor

kostya commented May 12, 2018

this works in 0.24.2 and not work in master:

p Time.parse("2011-08-15 12:00", "%Y-%m-%d %H:%M")
Unhandled exception: Time format did not include time zone and no default location provided (Time::Format::Error)
  from src/time/format/parser.cr:362:9 in 'raise'
  from src/time/format/parser.cr:30:9 in 'time'
  from src/time/format.cr:77:5 in 'parse'
  from src/time/format.cr:74:3 in 'parse'
  from src/time.cr:542:5 in 'parse'
  from src/time.cr:541:3 in 'parse'
  from 1.cr:3:3 in '__crystal_main'
  from src/crystal/main.cr:104:5 in 'main_user_code'
  from src/crystal/main.cr:93:7 in 'main'
  from src/crystal/main.cr:133:3 in 'main'
@kostya
Copy link
Contributor Author

kostya commented May 12, 2018

i find that now need to set location:

Time.parse("2011-08-15 12:00", "%Y-%m-%d %H:%M", Time::Location.local)

why not set it by default?

@RX14
Copy link
Member

RX14 commented May 14, 2018

Because the correct default massively depends on usecase.

@RX14 RX14 closed this as completed May 14, 2018
@w-A-L-L-e
Copy link

w-A-L-L-e commented Aug 22, 2018

Well the local default seems easier and makes sense in most cases.
Also doing Time.new just gives back current local time.
Example how it is now:

puts Time.parse("2012-11-01 22:08:12", "%F %T", Time::Location.local)
puts Time.parse("Fri Oct 31 23:00:24 2014", "%c", Time::Location.local)
puts Time.parse("Fri Oct 31 23:00:24 2014", "%c", Time::Location::UTC)
puts Time.parse("20150624", "%Y%m%d", Time::Location.local)
puts Time.new

with output:

2012-11-01 22:08:12 +01:00
2014-10-31 23:00:24 +01:00
2014-10-31 23:00:24 UTC
2015-06-24 00:00:00 +02:00
2018-08-22 21:00:05 +02:00

With the default suggested the code would be (basically if the user is concerned with timezones he'll add the argument, if not the sane/default is fine, being local time):

puts Time.parse("2012-11-01 22:08:12", "%F %T")
puts Time.parse("Fri Oct 31 23:00:24 2014", "%c")
puts Time.parse("Fri Oct 31 23:00:24 2014", "%c", Time::Location::UTC)
puts Time.parse("20150624", "%Y%m%d")
puts Time.new

So for me also +1 for the defaulting to local time. If you want UTC or something else you can always give it. Also to drive it further down to having defaults. The documentation on the crystal lang site is not updated and does say you have to give a time location as being local for .parse to work:

https://crystal-lang.org/api/0.20.0/Time.html
https://crystal-lang.org/api/0.20.0/Time.html#parse%28time%3AString%2Cpattern%3AString%2Ckind%3DTime%3A%3AKind%3A%3AUnspecified%29%3Aself-class-method

Other than that, keep up the great work!
Crystal's performance is great. Been playing a few days with this language benchmarking and doing some mvc with amber and liking it a lot ;)

@Sija
Copy link
Contributor

Sija commented Aug 22, 2018

There's also Time.parse! accepting %z

@straight-shoota
Copy link
Member

@w-A-L-L-e Seems you've been looking at outdated docs from 0.20.0 judging by the links you posted. The most recent API documentation is here: https://crystal-lang.org/api/latest/Time.html

There is now Time#parse_local which behaves like you described, see #6369 for details on this topic.

@w-A-L-L-e
Copy link

w-A-L-L-e commented Aug 23, 2018

Thanks, parse_local and parse_utc work indeed for a short/concise way of creating times... great ;).

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

No branches or pull requests

5 participants