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

Add formatters for chrono::time_point<system_clock> #1837

Merged
merged 3 commits into from
Aug 28, 2020
Merged

Add formatters for chrono::time_point<system_clock> #1837

merged 3 commits into from
Aug 28, 2020

Conversation

adamburgess
Copy link
Contributor

@adamburgess adamburgess commented Aug 25, 2020

Fixes #1819

Makes dates a little easier to format:

auto the_time = std::chrono::system_clock::now();
fmt::print("{}", the_time); // the_time is converted using fmt::localtime, which then uses the normal tm formatter
fmt::print("UTC {}", fmt::gmtime(the_time)); // prints UTC time using a new overload on gmtime/localtime

I wanted to test localtime so I did that using setenv/tzset, though I'm not sure if that's the best way.

C++20 does include a formatter for sys_time -- in #985 it was discussed that printing a time_point was not in the standard library, but it seems with time (hah) it has now been added.

I agree that my contributions are licensed under the {fmt} license, and agree to future changes to the licensing.

Copy link
Contributor

@vitaut vitaut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. Mostly looks good but the unit tests shouldn't manipulate environment variables. Instead I suggest comparing the output to the result of formatting time_t with system functions.

@adamburgess
Copy link
Contributor Author

My thoughts were to confirm that passing a time_point would be formatting with the local system time instead of UTC, and that's hard to test without changing the timezone (also: travis etc would be using UTC as well). It's probably up to the stdlib to make sure localtime works ;)

The test now does this: check if time_point formatting is equal to strftime(localtime(to_time_t(now())))

@vitaut vitaut merged commit f39e6fb into fmtlib:master Aug 28, 2020
@LarsGullik
Copy link

It is a bit bad that the formatters for chrono clocks are not compatible with C++20. In particular that %S in C++20 also prints out subseconds. Going via tm, truncates this to seconds.

@vitaut
Copy link
Contributor

vitaut commented Oct 5, 2020

A PR to improve compatibility with C++20 is welcome.

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.

Feature Request: Print std::chrono::time_point
3 participants