elixir-cldr / cldr_dates_times

Date & times formatting functions for the Common Locale Data Repository (CLDR) package https://github.com/elixir-cldr/cldr
Other
68 stars 13 forks source link

Time Intervals over day boundaries #42

Closed larshei closed 11 months ago

larshei commented 11 months ago

If a time interval spans over midnight, the function Cldr.Time.Interval.to_string cannot be used, returning the following error:

Cldr.DateTime.DateTimeOrderError, "Start date/time must be earlier or equal to end date/time.

In my case, I would like to display the time interval for parties, which usually start late in the evening and span to the next day.

The function preventing this is Cldr.Time.Interval.from_less_than_or_equal_to/2, as it does a Time.compare, which does not check the date, resulting in today, 20:00 being considered after tomorrow, 2:00.

Is this intended behaviour?

Personally, I would love to have a display like "22:00 - 3:00" (or 10 PM - 3 AM). Would it be okay to change the check to use DateTime.compare and maybe check if the interval is smaller than 24 hours? If this approach is fine, I'd be happy to work on that and submit a PR.

kipcole9 commented 11 months ago

Lars, good call out. I'll work on this on my Sunday (UTC+8). I have another interval related issue to fix.

Very much appreciate your willingness to provide a PR but I think in this case it'll be quicker if I do it since I'll be working on interval code anyway.

larshei commented 11 months ago

Okay, awesome!

Thank you for your work and providing these really valuable packages!

kipcole9 commented 11 months ago

Thanks for the kind words @larshei, much appreciated.

I've pushed a commit to the main branch that I believe fixes this issue. For example:

iex> MyApp.Cldr.Time.Interval.to_string ~T[23:00:00.0Z], ~T[01:01:00.0Z], locale: :en
{:ok, "11:00 PM – 1:01 AM"}

iex> MyApp.Cldr.Time.Interval.to_string ~T[23:00:00.0Z], ~T[01:01:00.0Z], locale: :fr
{:ok, "23:00 – 01:01"}

Would you consider given this test in your environment and let me know before I publish to hex?

larshei commented 11 months ago

Works as expected now, thanks for the quick fix.

kipcole9 commented 11 months ago

Thanks for the feedback. I will publish to hex in an hour or so.Sent from my iPhoneOn 22 Oct 2023, at 12:11, Lars Heinrichs @.***> wrote: Works as expected now, thanks for the quick fix.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you modified the open/close state.Message ID: @.***>