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 precision modifier for seconds in chrono format #3122

Closed
wants to merge 5 commits into from
Closed

Add precision modifier for seconds in chrono format #3122

wants to merge 5 commits into from

Conversation

SappyJoy
Copy link
Contributor

Add precision modifier for seconds in chrono format and test for it.

It's became important for custom datetime types that store date, time and subseconds. I want to use chrono format for those types, but I have no way to control subsecond precision.

Use case:

EXPECT_EQ(fmt::format("{:%Y-%m-%d %H:%M:%.6S %Z}", datetime), "2022-09-28 12:14:25.123456 MSK");

@vitaut
Copy link
Contributor

vitaut commented Sep 28, 2022

Thanks for the PR. Is this feature part of the standard?

@SappyJoy
Copy link
Contributor Author

SappyJoy commented Sep 28, 2022

No, it is not in standard. The standard only supports the precision modifier for strings and floating point numbers. Precision available only for particular argument, but not for format-spec.

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. Adding precision support to durations sounds reasonable but for consistency with the standard it should go at the beginning, not in %S (https://eel.is/c++draft/time.format), e.g.

fmt::print("{:.6%S}", std::chrono::nanoseconds{1234});

@SappyJoy
Copy link
Contributor Author

SappyJoy commented Oct 5, 2022

{:.6%S} looks like precision for argument. There is test for this:

  EXPECT_EQ("1.2 ms   ", fmt::format("{:7.1%Q %q}", dms(1.234)));

It is not obvious for user to use {:.6%S} and expect that this precision modifier belong to %S. But it can be done this way if the specifier has a higher priority on getting the modifier than the argument.

@vitaut
Copy link
Contributor

vitaut commented Oct 8, 2022

But it can be done this way if the specifier has a higher priority on getting the modifier than the argument.

We should make existing precision apply to %S and not introduce a new one to avoid diverging too much from the standard.

@SappyJoy SappyJoy closed this Oct 19, 2022
@SappyJoy SappyJoy deleted the add-precision-modifier-for-seconds-in-chrono-format branch October 19, 2022 11:19
@SappyJoy
Copy link
Contributor Author

SappyJoy commented Oct 19, 2022

Oh, I squashed last commits to resolve conflict. It led to close this PR. @vitaut, please reopen it, I made changes. Now it should work.

@vitaut
Copy link
Contributor

vitaut commented Oct 19, 2022

I am not able to reopen this diff for some reason. Please submit a new one.

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.

2 participants