Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

add BrokenDownTime::set_offset and BrokenDownTime::set_iana #78

Closed
martintmk opened this issue Aug 5, 2024 · 1 comment · Fixed by #118
Closed

add BrokenDownTime::set_offset and BrokenDownTime::set_iana #78

martintmk opened this issue Aug 5, 2024 · 1 comment · Fixed by #118
Labels
enhancement New feature or request

Comments

@martintmk
Copy link

Parsing custom UTC timestamp format:

#[test]
fn test_parsing() {
    let time: strtime::BrokenDownTime =
        jiff::fmt::strtime::parse("%Y-%m-%d at %H:%M:%S", "1970-01-01 at 01:00:00").unwrap();

    let ts = time.to_timestamp().unwrap();
}

Fails with the following error:

parsing format did not include time zone offset directive

This surprised me as I thought that zero offset would be used by default for timestamps if time zone information is not found. The BrokenDownTime offers some modification APIs such as set_month,., set_second, however, set_offset is missing here. This would at least allow me to do:

#[test]
fn test_parsing() {
    let mut time: strtime::BrokenDownTime =
        jiff::fmt::strtime::parse("%Y-%m-%d at %H:%M:%S", "1970-01-01 at 01:00:00").unwrap();

    time.set_offset(Some(Offset::UTC));

    let ts = time.to_timestamp().unwrap();
}

Would it make sense to fallback to UTC offset here?

let offset =

If not, exposing BrokenDownTime::set_offset should suffice.

@BurntSushi
Copy link
Owner

set_offset would be the way to go. It is definitely wildly inappropriate to assume UTC in cases like this. It might be appropriate in very specific circumstances, but implicitly always assuming UTC is I believe widely considered a source of many many many bugs.

@BurntSushi BurntSushi added the enhancement New feature or request label Aug 5, 2024
@BurntSushi BurntSushi changed the title Parsing of UTC timestamps requires the time zone offset add BrokenDownTime::set_offset and BrokenDownTime::set_iana Aug 5, 2024
BurntSushi added a commit that referenced this issue Aug 28, 2024
These have been added so that they can be set explicitly, like other
fields. This also includes an accessor for the IANA time zone
identifier.

I'm not sure why I didn't add these initially since there isn't anything
subtle going on here. I think it might have just been an oversight.

Fixes #78
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants