Replace Date, Time, DateTime with their counterparts from gleam_time. Closes #8

[ibarchenkov]

I'm a Gleam newbie, so please review carefully. Let me know if there’s any room for improvement or if you spot any obvious mistakes.

Closes #8

[lpil]

Fantastic, thank you! Looks great.

I'm not sure about the conversion into Timestamp here. It is the type we want to encourage people to use, but unfortunately TOML supports ambiguous dates without a timezone, so we can't do this without discarding that information about whether the offset is local or not.

Here's the part from the spec:

If you omit the offset from an RFC 3339 formatted date-time, it will represent the given date-time without any relation to an offset or timezone. It cannot be converted to an instant in time without additional information. Conversion to an instant, if required, is implementation-specific.

Do you have any thoughts on this?

[lpil]

Only one test case per test please 🙏

[ibarchenkov]

I'm not sure about the conversion into Timestamp here. It is the type we want to encourage people to use, but unfortunately TOML supports ambiguous dates without a timezone, so we can't do this without discarding that information about whether the offset is local or not.

Good point, I completely overlooked that. 🤦

Two options come to my mind:

pub type DateTime {
  // Represents a local/ambiguous/naive datetime, e.g., "1979-05-27T07:32:00"
  LocalDateTime(date: calendar.Date, time: calendar.TimeOfDay)
  // Represents a specific, unambiguous point in time,
  // taking the offset into account, e.g., "1979-05-27T07:32:00Z", "1979-05-27T00:32:00-07:00"
  Timestamp(timestamp.Timestamp)
}

pub type DateTime {
  DateTimeValue(date: calendar.Date, time: calendar.TimeOfDay, offset: Offset)
}

pub type Offset {
  Local
  Offset(duration.Duration)
}

What do you think?

[lpil]

Although we want to promote the use of timestamps as much as possible I think it might make sense to use calendar time here, since TOML doesn't permit you to always use them? Did you have any thoughts?

[ibarchenkov]

There seems to be a clear distinction in the spec between Offset Date-Time and Local Date-Time.
Can we represent these as individual variants in the Toml type?

pub type Toml {
  OffsetDateTime(timestamp.Timestamp)
  LocalDateTime(LocalDateTime)
  ... 
}

And replace as_date_time function with:

pub fn as_offset_date_time(toml: Toml) -> Result(timestamp.Timestamp, GetError) {
  case toml {
    OffsetDateTime(timestamp) -> Ok(timestamp)
    other -> Error(WrongType([], "OffsetDateTime", classify(other)))
  }
}

pub fn as_local_date_time(toml: Toml) -> Result(LocalDateTime, GetError) {
  case toml {
    LocalDateTime(local_datetime) -> Ok(local_datetime)
    other -> Error(WrongType([], "LocalDateTime", classify(other)))
  }
}

As a side note, it might be useful to introduce a type similar to Elixir's NaiveDateTime in gleam_time, so it can be reused here and in other libraries, for example in pog to represent Postgres' timestamp without time zone.

[lpil]

That design would discard the offset information, which is encoded in the TOML.

Let's store it as calendar time in the data structure, and then we can make getter functions for both calendar and timestamp time

[ibarchenkov]

Let's store it as calendar time in the data structure

Could you please take a look at my last commit? Do you think using Duration for the Offset makes sense, or would you prefer representing it explicitly as hours and minutes?

[lpil]

Thank you!

[lpil]

Fantastic!! Thank you very much!

I've fixed CI and renamed some of the already existing functions.