Skip to content

Commit

Permalink
Small API cleanup and a couple dev docs updates (#190)
Browse files Browse the repository at this point in the history
This moves some lingering `as_ixdtf_string` methods to
`to_ixdtf_string`. Changes `Instant::from_millisecond` from taking a
`i128` to `i64` as that should be more than sufficient for representing
an epoch millisecond value in the supported range.

---------

Co-authored-by: José Julián Espina <[email protected]>
  • Loading branch information
nekevss and jedel1043 authored Feb 4, 2025
1 parent aa1db99 commit f7d0500
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 23 deletions.
18 changes: 7 additions & 11 deletions docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,23 +26,19 @@ The primary date & time builtins/components are located in the
These builtins are then reexported from `lib.rs` to be available from
`temporal_rs`'s root module.

### Core vs. Native
### Core vs. Compiled

`temporal_rs`'s builtins are split in two distinct directories `core`
and `native`. The core implementation contains the core implementation
of the Temporal builtins; meanwhile, the `native` implementation is a
Rust wrapper around the `core` implementation that simplifies some
"lower" level date/time API that may not be necessary for a general use
case.
and `compiled`. The core implementation contains the core implementation
of the Temporal builtins; meanwhile, the `compiled` implementation provides
method wrappers around the `core` methods that simplify some "lower" level
date/time APIs that may not be necessary for a general use case.

### Core implementation

The core implementation is always publicly available, but may not be
available to import from the `temporal_rs`'s root.

The core implementation can be made available from the root by providing
the `--no-default-features` flag.

The core implementation exposes the Provider API that allows the user to
supply a "provider", or any type that implements the `TimeZoneProvider`
trait, for time zone data that the library can use to complete it's
Expand All @@ -61,9 +57,9 @@ impl ZonedDateTime {
}
```

### Native implementation
### Compiled implementation

The native implementation is only available via the "full" default
The native implementation is only available via the "compiled" default
feature flag.

For the same reason that the Provider API is useful for language
Expand Down
4 changes: 2 additions & 2 deletions src/builtins/compiled/instant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ impl Instant {
/// provided options
///
/// Enable with the `compiled_data` feature flag.
pub fn as_ixdtf_string(
pub fn to_ixdtf_string(
&self,
timezone: Option<&TimeZone>,
options: ToStringRoundingOptions,
Expand All @@ -18,6 +18,6 @@ impl Instant {
.lock()
.map_err(|_| TemporalError::general("Unable to acquire lock"))?;

self.as_ixdtf_string_with_provider(timezone, options, &*provider)
self.to_ixdtf_string_with_provider(timezone, options, &*provider)
}
}
16 changes: 8 additions & 8 deletions src/builtins/core/instant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,10 @@ impl Instant {
}

/// Creates a new `Instant` from the provided Epoch millisecond value.
pub fn from_epoch_milliseconds(epoch_milliseconds: i128) -> TemporalResult<Self> {
let epoch_nanos = epoch_milliseconds
.checked_mul(1_000_000)
.unwrap_or(i128::MAX);
pub fn from_epoch_milliseconds(epoch_milliseconds: i64) -> TemporalResult<Self> {
// Input at most is `i64::MAX`. This means guarantees that the
// transition into nanoseconds MUST be in range of `i128`
let epoch_nanos = (epoch_milliseconds as i128) * 1_000_000;
Self::try_new(epoch_nanos)
}

Expand Down Expand Up @@ -238,7 +238,7 @@ impl Instant {
// ==== Instant Provider API ====

impl Instant {
pub fn as_ixdtf_string_with_provider(
pub fn to_ixdtf_string_with_provider(
&self,
timezone: Option<&TimeZone>,
options: ToStringRoundingOptions,
Expand Down Expand Up @@ -629,13 +629,13 @@ mod tests {
// Assert the to_string is valid.
let provider = &FsTzdbProvider::default();
let inst_string = instant
.as_ixdtf_string_with_provider(None, ToStringRoundingOptions::default(), provider)
.to_ixdtf_string_with_provider(None, ToStringRoundingOptions::default(), provider)
.unwrap();
let one_string = one
.as_ixdtf_string_with_provider(None, ToStringRoundingOptions::default(), provider)
.to_ixdtf_string_with_provider(None, ToStringRoundingOptions::default(), provider)
.unwrap();
let two_string = two
.as_ixdtf_string_with_provider(None, ToStringRoundingOptions::default(), provider)
.to_ixdtf_string_with_provider(None, ToStringRoundingOptions::default(), provider)
.unwrap();

assert_eq!(&inst_string, "1969-12-25T12:23:45.678901234Z");
Expand Down
2 changes: 1 addition & 1 deletion src/builtins/core/time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ impl PlainTime {
Ok(Self::new_unchecked(result))
}

pub fn as_ixdtf_string(&self, options: ToStringRoundingOptions) -> TemporalResult<String> {
pub fn to_ixdtf_string(&self, options: ToStringRoundingOptions) -> TemporalResult<String> {
let resolved = options.resolve()?;
let (_, result) = self
.iso
Expand Down
2 changes: 1 addition & 1 deletion temporal_capi/src/plain_time.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ pub mod ffi {
write: &mut DiplomatWrite,
) -> Result<(), TemporalError> {
// TODO this double-allocates, an API returning a Writeable or impl Write would be better
let string = self.0.as_ixdtf_string(options.into())?;
let string = self.0.to_ixdtf_string(options.into())?;
// throw away the error, the write itself should always succeed
let _ = write.write_str(&string);

Expand Down

0 comments on commit f7d0500

Please sign in to comment.