chronotope / chrono-tz

TimeZone implementations for rust-chrono from the IANA database
Other
229 stars 55 forks source link

Export TzOffset #117

Closed mjdwitt closed 3 months ago

mjdwitt commented 1 year ago

This type is included in the Tz's implementation of TimeZone and not exporting it makes it difficult to work with Tz in wrapper types. Are there changes planned for this type that preclude it from being exported or is this okay?

mjdwitt commented 1 year ago

This question was brought up before in #35 but the asker closed it there after finding a workaround for their use-case.

djc commented 1 year ago

I became the maintainer for this crate a few months ago and haven't had time to dig in all that much beyond some version bumps. Thus, I don't have any context in my head around TzOffset and I'm wary of exposing more API surface. However, if you can give more context on your use case and on the role you think TzOffset plays in this crate we could discuss a way forward.

@LucioFranco any opinions?

mjdwitt commented 1 year ago

@djc Your wariness makes sense. I don't personally have any rush for this as I'm happy with using my branch of chrono_tz for now.

My specific use case is building a wrapper type that unifies Tz with a "floating" time zone for an implementation of iCal. It's not on GitHub, but it looks something like this:

/// iCal times and datetimes can either have a specific time zone or be considered "floating", i.e.
/// assumed to be in the current local timezone. Specific time zones can be either UTC or some
/// named time zone such as "America/New York". This enum models specific time zones as a wrapped
/// [`chrono_tz::Tz`] and floating time zones are just a [`chrono::Local`] under the hood.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub enum Zone {
    Floating,
    Named(chrono_tz::Tz),
}

This enum type is fine but the trick is in implementing chrono::TimeZone:

impl TimeZone for Zone {
    type Offset = ZoneOffset;
    // ...
}

#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub enum ZoneOffset {
    Floating(chrono::FixedOffset),
    Named(chrono_tz::TzOffset),
}

impl Offset for ZoneOffset {
    fn fix(&self) -> chrono::FixedOffset {
        match self {
            Self::Floating(offset) => *offset,
            Self::Named(tzoffset) => tzoffset.fix(),
        }
    }
}

Using my branch via a [patch.crates-io] directive in Cargo.toml lets me build this wrapper type by simply delegating to the underlying types for each variation of the wrapper.

pitdicker commented 3 months ago

Thank you @mjdwitt!