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

Formatting a std::chrono::system_clock renders in local time, not in UTC #3199

Closed
ned14 opened this issue Nov 21, 2022 · 5 comments
Closed

Comments

@ned14
Copy link
Contributor

ned14 commented Nov 21, 2022

For some time now we've had this in our cmake, so I figured time to upstream this:

  message(STATUS "NOTE: Patching '${VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/include/fmt/chrono.h' to fix a formatting bug ...")
  file(READ "${VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/include/fmt/chrono.h" fmt_chrono_contents)
  string(REPLACE "  auto format(std::chrono::time_point<std::chrono::system_clock> val," "  auto format(std::chrono::time_point<std::chrono::system_clock, Duration> val," fmt_chrono_contents "${fmt_chrono_contents}")
  string(REPLACE "    return formatter<std::tm, Char>::format(localtime(val), ctx);" "    return formatter<std::tm, Char>::format(gmtime(std::chrono::time_point_cast<std::chrono::system_clock::time_point::duration>(val)), ctx);" fmt_chrono_contents "${fmt_chrono_contents}")
  file(WRITE "${VCPKG_INSTALLED_DIR}/${VCPKG_TARGET_TRIPLET}/include/fmt/chrono.h" "${fmt_chrono_contents}")

Two things are changed here:

  1. The Duration template parameter isn't passed through, breaking formatting time points with non-default duration types.

  2. Formatting system clock time points is to UTC, not to localtime, as system clock specifically represents UTC not local time (if you want a local time point, use chrono::local_t). Microsoft's implementation of std::format agrees with this formatting interpretation i.e. system clocks format to UTC strings, local clocks format to local strings.

@ned14 ned14 changed the title Formatting a std::chrono::system_block renders in local time, not in UTC Formatting a std::chrono::system_clock renders in local time, not in UTC Nov 21, 2022
@vitaut
Copy link
Contributor

vitaut commented Nov 27, 2022

The first change has already been applied. Could you by any chance submit the second one as a PR?

@ned14
Copy link
Contributor Author

ned14 commented Dec 16, 2022

Last day of work for this year, so had a few spare minutes to get this done finally.

@vitaut vitaut closed this as completed Dec 21, 2022
@ned14
Copy link
Contributor Author

ned14 commented Dec 22, 2022

Apologies for the unprompted reply. Currently on holidays in the United States without any ability to write code, which is deliberate. Glad you figured out some way to merge it, will look into your remaining ask when I return to Europe in January.

@Jan-Otto-Bakkland-Volue

Converting to local time not working as specified:
auto localTime = std::chrono::localtime(first_time)
image

or this:
auto localTime = std::localtime(first_time);
image

@vitaut
Copy link
Contributor

vitaut commented Nov 13, 2023

@Jan-Otto-Bakkland-Volue, this doesn't look related to {fmt}. Please check cppreference or ask on StackOverflow.

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

No branches or pull requests

3 participants