Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: Adjust formatting for local datetime objects #621

Closed
wants to merge 1 commit into from

Conversation

mjip
Copy link

@mjip mjip commented Oct 6, 2021

Partially fixes #613
Fixes tests TestTOMLTest_Valid_Datetime_Local and TestTOMLTest_Valid_Datetime_Milliseconds.

Issue: #613

Explanation of what this pull request does.
Adjusts the string formatting of LocalTime structs to comply with test output (uses a T as a date/time separator, trims trailing zeros in nanosecond string).

@moorereason
Copy link
Contributor

moorereason commented Oct 7, 2021

Do we want to preserve nanosecond precision while decoding?

I have a patch locally that fixed this by preserving the precision of the nanosecond value. For example, 10:32:00.000 and 10:32:00.500 would both be preserved with 3-digit precision.

The only argument I have for it is for roundtripping.

It doesn't look like BurntSushi preserves precision (10:32:00.50010:32:00.5). The toml-test suite doesn't test for this case.

@pelletier
Copy link
Owner

Sorry for the late reply. I think that if there is no usability difference between the two method we should benchmark and take the fastest. @moorereason if you push that patch you have I'm happy to bench it!

@moorereason
Copy link
Contributor

@pelletier, I've pushed up #626 for consideration.

@pelletier
Copy link
Owner

Thank you!

@mjip mjip closed this Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants