chronotope / chrono

Date and time library for Rust
Other
3.3k stars 523 forks source link

Add `MappedLocalTime::try_map` #1533

Closed pitdicker closed 6 months ago

pitdicker commented 6 months ago

Passing on one error in unix::Cache::offset requires a match that is very similar to the one in TimeZone::from_local_datetime. I added MappedLocalTime::try_map that returns MappedLocalTime::None that can be used in both places.

For the 0.5.x branch it will be easy to convert this method to return a Result instead.

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 75.00000% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 91.82%. Comparing base (3adfd88) to head (e8db5cd).

Files Patch % Lines
src/offset/mod.rs 73.33% 4 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1533 +/- ## ======================================= Coverage 91.82% 91.82% ======================================= Files 40 40 Lines 18345 18345 ======================================= Hits 16846 16846 Misses 1499 1499 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pitdicker commented 6 months ago

This PR on its own doesn't look well-motivated...

Not sure what you mean. Maybe as another motivation: to implement addition and substraction for a CalendarDuration (#1282) months and days should be added in localtime with a MappedLocalTime as result. Then seconds should be added using a method such as this so the type remains a MappedLocalTime.

But I have to admit I'm not sure if the current two uses in this PR for the 0.4 versions will remain useful for 0.5 with one of the plans in 1448#issuecomment-1987220921. So the method is pub(crate) for now.

djc commented 6 months ago

Ah, sorry, I missed that there are sort of two users for the new method. Looks a little surprising because one of the callers did not previously have the extensive logic implemented in the method.

pitdicker commented 6 months ago

Yes, that was the issue I set out to fix :smile:. Sorry for not saying so better.