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

<chrono>: Fix hh_mm_ss subsecond formatting for floats #1866

Merged
merged 2 commits into from
Apr 20, 2021

Conversation

eldakesh-ms
Copy link
Contributor

Before, the same formatting string was used for floats and integrals.
This meant that large floats were formatted using exponent notaion and
small floats were not, and it also meant there was an extra period in a
time, as the subseconds could be fractions of a subsecond (say .4
nanoseconds). Now if the subseconds are floats, we force fixed
formatting to get the right number of leading zeroes and a precision of
0 to round off fractions of subseconds.

Before, the same formatting string was used for floats and integrals.
This meant that large floats were formatted using exponent notaion and
small floats were not, and it also meant there was an extra period in a
time, as the subseconds could be fractions of a subsecond (say .4
nanoseconds). Now if the subseconds are floats, we force fixed
formatting to get the right number of leading zeroes and a precision of
0 to round off fractions of subseconds.
@eldakesh-ms eldakesh-ms requested a review from a team as a code owner April 20, 2021 16:08
@eldakesh-ms
Copy link
Contributor Author

Per discord discussion with @statementreply: Currently this rounds, but after some deliberation truncation probably makes more sense. The spec is a bit ambiguous on this. If we do rounding, we need to pull the subsecond into the seconds (if it rounds up to a whole second), and this can mean that a second becomes a minute and a minute becomes an hour. I think truncation is the easiest solution, but maybe I a missing something.

@mnatsuhara mnatsuhara added the chrono C++20 chrono label Apr 20, 2021
@eldakesh-ms eldakesh-ms merged commit cc2651d into microsoft:chronat2 Apr 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chrono C++20 chrono
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants