Skip to content

Commit

Permalink
Move offset logic to UtcOffset (#5681)
Browse files Browse the repository at this point in the history
  • Loading branch information
@robertbastian
robertbastian authored Oct 14, 2024
1 parent e506244 commit 0732198
Show file tree
Hide file tree
Showing 15 changed files with 207 additions and 141 deletions.
40 changes: 13 additions & 27 deletions components/datetime/src/time_zone.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -389,43 +389,29 @@ impl FormatOffset for LocalizedOffsetFormat {

self.fdf
.format(
&FixedDecimal::from(
(self.offset.offset_seconds() / 3600).unsigned_abs(),
)
.padded_start(
if self.length == FieldLength::Wide {
&FixedDecimal::from(self.offset.hours_part().unsigned_abs())
.padded_start(if self.length == FieldLength::Wide {
2
} else {
0
},
),
}),
)
.write_to(sink)?;

if self.length == FieldLength::Wide
|| self.offset.has_minutes()
|| self.offset.has_seconds()
|| self.offset.minutes_part() != 0
|| self.offset.seconds_part() != 0
{
sink.write_char(self.separator)?;
self.fdf
.format(
&FixedDecimal::from(
(self.offset.offset_seconds() / 60 % 60).unsigned_abs(),
)
.padded_start(2),
)
.format(&FixedDecimal::from(self.offset.minutes_part()).padded_start(2))
.write_to(sink)?;
}

if self.offset.has_seconds() {
if self.offset.seconds_part() != 0 {
sink.write_char(self.separator)?;
self.fdf
.format(
&FixedDecimal::from(
(self.offset.offset_seconds() % 60).unsigned_abs(),
)
.padded_start(2),
)
.format(&FixedDecimal::from(self.offset.seconds_part()).padded_start(2))
.write_to(sink)?;
}

Expand Down Expand Up @@ -647,22 +633,22 @@ impl Iso8601Format {

sink.write_char(if offset.is_positive() { '+' } else { '-' })?;

format_time_segment(sink, (offset.offset_seconds() / 3600).unsigned_abs())?;
format_time_segment(sink, offset.hours_part().unsigned_abs())?;

if self.minutes == IsoMinutes::Required
|| (self.minutes == IsoMinutes::Optional && offset.has_minutes())
|| (self.minutes == IsoMinutes::Optional && offset.minutes_part() != 0)
{
if extended_format {
sink.write_char(':')?;
}
format_time_segment(sink, (offset.offset_seconds() % 3600 / 60).unsigned_abs())?;
format_time_segment(sink, offset.minutes_part())?;
}

if self.seconds == IsoSeconds::Optional && offset.has_seconds() {
if self.seconds == IsoSeconds::Optional && offset.seconds_part() != 0 {
if extended_format {
sink.write_char(':')?;
}
format_time_segment(sink, (offset.offset_seconds() % 3600 % 60).unsigned_abs())?;
format_time_segment(sink, offset.seconds_part())?;
}

Ok(())
Expand Down
3 changes: 1 addition & 2 deletions components/datetime/tests/datetime.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,8 +479,7 @@ fn test_time_zone_format_offset_seconds() {
neo_marker::NeoTimeZoneOffsetMarker, neo_skeleton::NeoSkeletonLength, NeverCalendar,
};

let time_zone =
CustomTimeZone::new_with_offset(UtcOffset::try_from_offset_seconds(12).unwrap());
let time_zone = CustomTimeZone::new_with_offset(UtcOffset::try_from_seconds(12).unwrap());
let tzf = TypedNeoFormatter::<NeverCalendar, _>::try_new(
&locale!("en").into(),
NeoTimeZoneOffsetMarker::with_length(NeoSkeletonLength::Medium),
Expand Down
16 changes: 8 additions & 8 deletions components/timezone/src/ixdtf.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl UtcOffset {
fn try_from_utc_offset_record(record: &UTCOffsetRecord) -> Result<Self, ParseError> {
let hour_seconds = i32::from(record.hour) * 3600;
let minute_seconds = i32::from(record.minute) * 60;
Self::try_from_offset_seconds(
Self::try_from_seconds(
i32::from(record.sign as i8)
* (hour_seconds + minute_seconds + i32::from(record.second)),
)
Expand Down Expand Up @@ -191,7 +191,7 @@ impl CustomZonedDateTime<Iso> {
/// assert_eq!(zoneddatetime.time.second.number(), 19);
/// assert_eq!(zoneddatetime.time.nanosecond.number(), 0);
/// assert_eq!(zoneddatetime.zone, CustomTimeZone {
/// offset: Some(UtcOffset::try_from_offset_seconds(-18000).unwrap()),
/// offset: Some(UtcOffset::try_from_seconds(-18000).unwrap()),
/// time_zone_id: Some(TimeZoneBcp47Id(tinystr!(8, "uschi"))),
/// metazone_id: Some(MetazoneId(tinystr!(4, "amce"))),
/// zone_variant: Some(ZoneVariant::daylight()),
Expand Down Expand Up @@ -277,7 +277,7 @@ impl CustomZonedDateTime<AnyCalendar> {
/// assert_eq!(zoneddatetime.time.second.number(), 19);
/// assert_eq!(zoneddatetime.time.nanosecond.number(), 0);
/// assert_eq!(zoneddatetime.zone, CustomTimeZone {
/// offset: Some(UtcOffset::try_from_offset_seconds(-18000).unwrap()),
/// offset: Some(UtcOffset::try_from_seconds(-18000).unwrap()),
/// time_zone_id: Some(TimeZoneBcp47Id(tinystr!(8, "uschi"))),
/// metazone_id: Some(MetazoneId(tinystr!(4, "amce"))),
/// zone_variant: Some(ZoneVariant::daylight()),
Expand All @@ -300,7 +300,7 @@ impl CustomZonedDateTime<AnyCalendar> {
/// let tz_from_offset = CustomZonedDateTime::try_from_str("2024-08-08T12:08:19-05:00").unwrap();
///
/// assert_eq!(tz_from_offset.zone, CustomTimeZone {
/// offset: Some(UtcOffset::try_from_offset_seconds(-18000).unwrap()),
/// offset: Some(UtcOffset::try_from_seconds(-18000).unwrap()),
/// time_zone_id: None,
/// metazone_id: None,
/// zone_variant: None,
Expand All @@ -320,7 +320,7 @@ impl CustomZonedDateTime<AnyCalendar> {
/// let tz_from_iana_annotation = CustomZonedDateTime::try_from_str("2024-08-08T12:08:19[America/Chicago]").unwrap();
///
/// assert_eq!(tz_from_offset_annotation.zone, CustomTimeZone {
/// offset: Some(UtcOffset::try_from_offset_seconds(-18000).unwrap()),
/// offset: Some(UtcOffset::try_from_seconds(-18000).unwrap()),
/// time_zone_id: None,
/// metazone_id: None,
/// zone_variant: None,
Expand Down Expand Up @@ -352,7 +352,7 @@ impl CustomZonedDateTime<AnyCalendar> {
/// let consistent_tz_from_both = CustomZonedDateTime::try_from_str("2024-08-08T12:08:19-05:00[America/Chicago]").unwrap();
///
/// assert_eq!(consistent_tz_from_both.zone, CustomTimeZone {
/// offset: Some(UtcOffset::try_from_offset_seconds(-18000).unwrap()),
/// offset: Some(UtcOffset::try_from_seconds(-18000).unwrap()),
/// time_zone_id: Some(TimeZoneBcp47Id(tinystr!(8, "uschi"))),
/// metazone_id: Some(MetazoneId(tinystr!(4, "amce"))),
/// zone_variant: Some(ZoneVariant::daylight()),
Expand All @@ -361,7 +361,7 @@ impl CustomZonedDateTime<AnyCalendar> {
/// let inconsistent_tz_from_both = CustomZonedDateTime::try_from_str("2024-08-08T12:08:19-05:00[America/Los_Angeles]").unwrap();
///
/// assert_eq!(inconsistent_tz_from_both.zone, CustomTimeZone {
/// offset: Some(UtcOffset::try_from_offset_seconds(-18000).unwrap()),
/// offset: Some(UtcOffset::try_from_seconds(-18000).unwrap()),
/// time_zone_id: Some(TimeZoneBcp47Id(tinystr!(8, "uslax"))),
/// metazone_id: Some(MetazoneId(tinystr!(4, "ampa"))),
/// zone_variant: None,
Expand All @@ -379,7 +379,7 @@ impl CustomZonedDateTime<AnyCalendar> {
/// let consistent_tz_from_both = CustomZonedDateTime::try_from_str("2024-08-08T12:08:19-05:00[-05:00]").unwrap();
///
/// assert_eq!(consistent_tz_from_both.zone, CustomTimeZone {
/// offset: Some(UtcOffset::try_from_offset_seconds(-18000).unwrap()),
/// offset: Some(UtcOffset::try_from_seconds(-18000).unwrap()),
/// time_zone_id: None,
/// metazone_id: None,
/// zone_variant: None,
Expand Down
12 changes: 5 additions & 7 deletions components/timezone/src/time_zone.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,7 @@ impl CustomTimeZone {
zone_variant: TinyAsciiStr<2>,
) -> Self {
Self {
offset: Some(UtcOffset::from_offset_eighths_of_hour(
offset_eighths_of_hour,
)),
offset: Some(UtcOffset::from_eighths_of_hour(offset_eighths_of_hour)),
time_zone_id: Some(TimeZoneBcp47Id(time_zone_id)),
metazone_id: Some(MetazoneId(metazone_id)),
zone_variant: Some(ZoneVariant(zone_variant)),
Expand Down Expand Up @@ -131,10 +129,10 @@ impl CustomTimeZone {
/// let tz3: CustomTimeZone = CustomTimeZone::try_from_str("+02:30")
/// .expect("Failed to parse a time zone");
///
/// assert_eq!(tz0.offset.map(UtcOffset::offset_seconds), Some(0));
/// assert_eq!(tz1.offset.map(UtcOffset::offset_seconds), Some(7200));
/// assert_eq!(tz2.offset.map(UtcOffset::offset_seconds), Some(-9000));
/// assert_eq!(tz3.offset.map(UtcOffset::offset_seconds), Some(9000));
/// assert_eq!(tz0.offset.map(UtcOffset::to_seconds), Some(0));
/// assert_eq!(tz1.offset.map(UtcOffset::to_seconds), Some(7200));
/// assert_eq!(tz2.offset.map(UtcOffset::to_seconds), Some(-9000));
/// assert_eq!(tz3.offset.map(UtcOffset::to_seconds), Some(9000));
/// ```
///
/// [`CustomZonedDateTime::try_iso_from_str`]: crate::CustomZonedDateTime::try_iso_from_str
Expand Down
49 changes: 27 additions & 22 deletions components/timezone/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,10 @@ impl Default for UtcOffset {
}
}

fn try_get_time_component([tens, ones]: [u8; 2]) -> Option<i32> {
Some(((tens as char).to_digit(10)? * 10 + (ones as char).to_digit(10)?) as i32)
}

impl UtcOffset {
/// Attempt to create a [`UtcOffset`] from a seconds input. It returns
/// [`InvalidOffsetError`] when the seconds are out of bounds.
pub fn try_from_offset_seconds(seconds: i32) -> Result<Self, InvalidOffsetError> {
pub fn try_from_seconds(seconds: i32) -> Result<Self, InvalidOffsetError> {
if seconds.unsigned_abs() > 18 * 60 * 60 {
Err(InvalidOffsetError)
} else {
Expand All @@ -45,10 +41,10 @@ impl UtcOffset {
///
/// assert_eq!(
/// UtcOffset::try_from_str("-0600").unwrap(),
/// UtcOffset::from_offset_eighths_of_hour(-6 * 8),
/// UtcOffset::from_eighths_of_hour(-6 * 8),
/// );
/// ```
pub const fn from_offset_eighths_of_hour(eighths_of_hour: i8) -> Self {
pub const fn from_eighths_of_hour(eighths_of_hour: i8) -> Self {
Self(eighths_of_hour as i32 * 450)
}

Expand Down Expand Up @@ -82,10 +78,10 @@ impl UtcOffset {
/// let offset_err1 =
/// UtcOffset::try_from_str("+05000").expect_err("Invalid input");
///
/// assert_eq!(offset0.offset_seconds(), 0);
/// assert_eq!(offset1.offset_seconds(), 18000);
/// assert_eq!(offset2.offset_seconds(), 18000);
/// assert_eq!(offset3.offset_seconds(), -18000);
/// assert_eq!(offset0.to_seconds(), 0);
/// assert_eq!(offset1.to_seconds(), 18000);
/// assert_eq!(offset2.to_seconds(), 18000);
/// assert_eq!(offset3.to_seconds(), -18000);
/// ```
#[inline]
pub fn try_from_str(s: &str) -> Result<Self, InvalidOffsetError> {
Expand All @@ -94,6 +90,10 @@ impl UtcOffset {

/// See [`Self::try_from_str`]
pub fn try_from_utf8(mut code_units: &[u8]) -> Result<Self, InvalidOffsetError> {
fn try_get_time_component([tens, ones]: [u8; 2]) -> Option<i32> {
Some(((tens as char).to_digit(10)? * 10 + (ones as char).to_digit(10)?) as i32)
}

let offset_sign = match code_units {
[b'+', rest @ ..] => {
code_units = rest;
Expand Down Expand Up @@ -129,26 +129,26 @@ impl UtcOffset {
}
.ok_or(InvalidOffsetError)?;

Self::try_from_offset_seconds(offset_sign * (hours * 60 + minutes) * 60)
Self::try_from_seconds(offset_sign * (hours * 60 + minutes) * 60)
}

/// Create a [`UtcOffset`] from a seconds input without checking bounds.
///
/// # Safety
///
/// The seconds must be a valid value as returned by [`Self::offset_seconds`].
/// The seconds must be a valid value as returned by [`Self::to_seconds`].
#[inline]
pub unsafe fn from_offset_seconds_unchecked(seconds: i32) -> Self {
pub unsafe fn from_seconds_unchecked(seconds: i32) -> Self {
Self(seconds)
}

/// Returns the raw offset value in seconds.
pub fn offset_seconds(self) -> i32 {
pub fn to_seconds(self) -> i32 {
self.0
}

/// Returns the raw offset value in eights of an hour (7.5 minute units).
pub fn offset_eighths_of_hour(self) -> i8 {
pub fn to_eighths_of_hour(self) -> i8 {
(self.0 / 450) as i8
}

Expand All @@ -162,14 +162,19 @@ impl UtcOffset {
self.0 == 0
}

/// Returns `true` if the [`UtcOffset`] has non-zero minutes, otherwise `false`.
pub fn has_minutes(self) -> bool {
self.0 % 3600 / 60 > 0
/// Returns the hours part of if the [`UtcOffset`]
pub fn hours_part(self) -> i32 {
self.0 / 3600
}

/// Returns the minutes part of if the [`UtcOffset`].
pub fn minutes_part(self) -> u32 {
(self.0 % 3600 / 60).unsigned_abs()
}

/// Returns `true` if the [`UtcOffset`] has non-zero seconds, otherwise `false`.
pub fn has_seconds(self) -> bool {
self.0 % 3600 % 60 > 0
/// Returns the seconds part of if the [`UtcOffset`].
pub fn seconds_part(self) -> u32 {
(self.0 % 60).unsigned_abs()
}
}

Expand Down
11 changes: 5 additions & 6 deletions components/timezone/src/zone_offset.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl ZoneOffsetCalculator {
/// TimeZoneBcp47Id(tinystr!(8, "usden")),
/// &DateTime::try_new_iso_datetime(2024, 1, 1, 0, 0, 0).unwrap()
/// ),
/// Some((UtcOffset::try_from_offset_seconds(-7 * 3600).unwrap(), Some(UtcOffset::try_from_offset_seconds(-6 * 3600).unwrap())))
/// Some((UtcOffset::try_from_seconds(-7 * 3600).unwrap(), Some(UtcOffset::try_from_seconds(-6 * 3600).unwrap())))
/// );
///
/// // America/Phoenix does not
Expand All @@ -87,7 +87,7 @@ impl ZoneOffsetCalculator {
/// TimeZoneBcp47Id(tinystr!(8, "usphx")),
/// &DateTime::try_new_iso_datetime(2024, 1, 1, 0, 0, 0).unwrap()
/// ),
/// Some((UtcOffset::try_from_offset_seconds(-7 * 3600).unwrap(), None))
/// Some((UtcOffset::try_from_seconds(-7 * 3600).unwrap(), None))
/// );
/// ```
pub fn compute_offsets_from_time_zone(
Expand All @@ -110,10 +110,9 @@ impl ZoneOffsetCalculator {
}
let offsets = offsets?;
Some((
UtcOffset::from_offset_eighths_of_hour(offsets.0),
(offsets.1 != 0).then_some(UtcOffset::from_offset_eighths_of_hour(
offsets.0 + offsets.1,
)),
UtcOffset::from_eighths_of_hour(offsets.0),
(offsets.1 != 0)
.then_some(UtcOffset::from_eighths_of_hour(offsets.0 + offsets.1)),
))
}
None => None,
Expand Down
11 changes: 7 additions & 4 deletions ffi/capi/bindings/c/CustomTimeZone.h
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions ffi/capi/bindings/cpp/icu4x/CustomTimeZone.d.hpp
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 0732198

Please sign in to comment.