tom icon indicating copy to clipboard operation
tom copied to clipboard

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

Open ibarchenkov opened this issue 7 months ago • 3 comments

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

ibarchenkov avatar May 19 '25 16:05 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?

ibarchenkov avatar May 20 '25 20:05 ibarchenkov

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?

lpil avatar May 21 '25 14:05 lpil

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.

ibarchenkov avatar May 21 '25 21:05 ibarchenkov

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

lpil avatar May 28 '25 19:05 lpil

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?

ibarchenkov avatar May 31 '25 07:05 ibarchenkov