floraison / et-orbi

Time zones for fugit and rufus-scheduler. Urbi et Orbi.
MIT License
24 stars 11 forks source link

EtOrbi.parse unable to parse some timezones that Time.parse can #30

Closed jsmartt closed 3 years ago

jsmartt commented 3 years ago

I'm trying to parse user-given times for scheduled events, and there are certain timezones parsable by Time and DateTime that are ignored by EtOrbi.

EtOrbi.parse('7AM PST').to_s    # "2021-02-19 07:00:00 +0000"
DateTime.parse('7AM PST').to_s  # "2021-02-19 07:00:00 -0800"
Time.parse('7AM PST').to_s      # "2021-02-19 07:00:00 -0800"

EtOrbi.parse('7AM MDT').to_s    # "2021-02-19 07:00:00 +0000"
Time.parse('7AM MDT').to_s      # "2021-02-19 07:00:00 -0600"

I've done a bit of research and have a suggestion to offer. Perhaps EtOrbi.extract_zone can use Date._parse to determine if the string given has timezone info provided, and if so, use that:

def extract_zone(str)

  s = str.dup

  d = Date._parse(str)
  if d && d[:offset]
    offset = "UTC#{d[:offset] / 3600}"
    s.gsub!(d[:zone], offset) if ZONES_OLSON.include?(offset)
  end

  # Rest of original code from method
end

This allows the ZoneOffsets parsable by Time.parse to be recognized here too.

I recognize that there are a small number of timezones affected here, mainly US and military timezones, so maybe it's not worth it? I'd definitely like to hear a maintainer's thoughts.

Thanks!

jmettraux commented 3 years ago

Hello,

@jsmartt wrote:

if there is anything et-orbi can do to persist user-provided timezone info?

I would prefer et-orbi not storing anything, just making the best possible decision given context and input.

I will look at your proposition, it looks straightforward.

Give me a bit of time, thanks for the feedback!

jmettraux commented 3 years ago

@jsmartt

By the way, in which context are you using et-orbi? Via fugit or via rufus-scheduler?

jmettraux commented 3 years ago

OK, it seems that the promise of Ruby dealing with timezones is here for good, gh-16

jsmartt commented 3 years ago

Thanks for looking into it @jmettraux. I'm using etorbi in the context of fugit. And I've updated the description; I left in a blurb about tzinfo and timezones that was from a previous draft, and is irrelevant; sorry for the confusion.

jsmartt commented 3 years ago

I'm actually just now realizing some other behavior that makes this suggested change a bit useless for me. It doesn't look like EtOrbi.get_tzone knows how to handle some of the special timezones supported by EtOrbi.extract_zone, and visa-versa. This causes failures in EtOrbi.parse. For example:

EtOrbi.extract_zone('7AM -07:00') # ["7AM -07:00", nil]
EtOrbi.get_tzone('-07:00')        # <TZInfo::DataTimezone: -7:00>
EtOrbi.parse('7AM -07:00')        # <EtOrbi::EoTime:0x00007f7c172c96c0 @seconds=1613977200.0, @time=nil, @zone=#<TZInfo::DataTimezone: Etc/UTC>>

EtOrbi.extract_zone('7AM UTC-7') # ["7AM", "UTC-7"]
EtOrbi.get_tzone('UTC-7')        # Error
EtOrbi.parse('7AM UTC-7')        # Error

EtOrbi.extract_zone('7AM GMT-7') # ["7AM -7", "GMT"]
EtOrbi.get_tzone('GMT-7')        # <TZInfo::DataTimezone: GMT>
EtOrbi.parse('7AM GMT-7')        # <EtOrbi::EoTime:0x00007f7c21f00f10 @seconds=1614063600.0, @time=nil, @zone=#<TZInfo::DataTimezone: GMT>>

# These all work as expected:
EtOrbi.extract_zone('7AM Etc/GMT-7') # ["7AM", "Etc/GMT-7"]
EtOrbi.get_tzone('Etc/GMT-7')        # <TZInfo::DataTimezone: Etc/GMT-7>
EtOrbi.parse('7AM Etc/GMT-7')        # <EtOrbi::EoTime:0x00007f7c2020aa28 @seconds=1614038400.0, @time=nil, @zone=#<TZInfo::DataTimezone: Etc/GMT-7>>

There is quite a mismatch in terms of which zones are supported by these different methods. Is that expected? It would be nice if the zones like UTC-8, GMT-8, & -08:00 were fully supported across the board if they are in 1 place.

jmettraux commented 3 years ago

Hello,

actually, what I am expecting for fugit is proper TZ database names like "Africa/Abidjan". TZ abbreviations are ambiguous and then there are instances like CET (Central European Time) vs CEST (Central European Summer Time).

TZ database names are less ambiguous and with proper tzinfo management, they are more resilient to daylight saving policies and other political changes.

I will still look at your examples and let et-orbi and fugit do their best.

I will also explain TZ abbreviation vs TZ database names in fugit's readme.

jsmartt commented 3 years ago

Ok. The TZ database names definitely seem like the best option for now, so I'll make sure my users know to use those. 👍 Users are inputing these time strings, which is why I wanted etorbi/fugit to be able to parse a variety of additional formats, but I totally understand that not all arbitrary formats can or should be supported. Thanks for the clarification on all this!

jmettraux commented 3 years ago

Since you probably stand between your user input and passing it to fugit and friends, you could translate "PST" and "PDT" to "America/Los_Angeles" before passing it. You know your users timezones better than fugit and et-orbi do.