Full Timezone support when parsing ical #408
Conversation
Change offset without changing time
like PDT, PST, etc.
not TimeWithZone, while when parsing a timezone it is a TimeWithZone
|
@avit do you think we should move ical parsing out of |
lib/ice_cube/parsers/ical_parser.rb
Outdated
| def parse_in_tzid(value, tzid) | ||
| time_parser = Time | ||
| if tzid | ||
| time_parser = ActiveSupport::TimeZone.new(tzid.split('=')[1]) || Time |
There was a problem hiding this comment.
Use rescue Time here if ActiveSupport is not loaded? Maybe something like IcalParser.time_parser can be determined statically for performance.
|
I like the idea... are you thinking of a separate gem, or maybe just an optional require? |
|
I think the optional require makes sense, but separate gem also sounds like a decent idea to me. I just dislike that we're potentially having two different behaviors in the ical parser dependending if |
|
We have a dependency on ActiveSupport here. Maybe this can work with |
|
@avit Ah I like that idea - will update this soon |
|
Any update on this? It's really a bummer that this has been roadblocked for years at this point, even though the current behavior which is completely broken w.r.t. daylight savings time, just to avoid the dependency on ActiveSupport. With all due respect, what's more important - having a library that works and solves people's problems, or keeping the dependencies down? |
|
What do people think of the compromise of just making this method raise if |
|
No worries, thanks for your continuing work maintaining this library. Hopefully it's close now! +1 from me on raising if ActiveSupport isn't available. |
|
Seems like a good idea. |
This PR builds on the great work from #335 (thanks @dcosson)
I'll be removing the need for
ActiveSupporthere.Also, added support for parsing
RDATEwhile I'm in here.Closes #335