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 #3148

Conversation

SappyJoy
Copy link
Contributor

@SappyJoy SappyJoy commented Oct 19, 2022

Continuation of #3122

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("{:.6%Y-%m-%d %H:%M:%S %Z}", datetime), "2022-09-28 12:14:25.123456 MSK");

So precision can be applied not only for %Q spec, but for %S too.

@vitaut
Copy link
Contributor

vitaut commented Oct 22, 2022

Could you copy the summary (updating it if needed) from #3122?

return;
}

FMT_ASSERT(std::is_floating_point<typename Duration::rep>::value, "");
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add an alias for typename Duration::rep and use it here and in several places below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@@ -1056,31 +1056,91 @@ void write_fractional_seconds(OutputIt& out, Duration d) {
}
}

template <typename Char, typename OutputIt, typename Duration>
void write_fractional_seconds(OutputIt& out, Duration d, int precision) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's merge this overload with the one above to reduce code duplication.

: std::chrono::duration_cast<subsecond_precision>(fractional)
.count();
uint32_or_64_or_128_t<long long> n =
to_unsigned(to_nonnegative_int(subseconds, max_value<long long>()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can subseconds be negative?

Copy link
Contributor Author

@SappyJoy SappyJoy Nov 2, 2022

Choose a reason for hiding this comment

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

It's nonnegative because fractional is nonnegative:

const auto fractional = detail::abs(d) - std::chrono::duration_cast<std::chrono::seconds>(d);

But subseconds can be signed or unsigned and to_unsigned raises error if argument is unsigned, so i'll make static_cast.

// Format subseconds which are given as a floating point type with an appropiate
// number of digits. We cannot pass the Duration here, as we explicitly need to
// pass the Rep value in the chrono_formatter.
template <typename Duration>
void write_floating_seconds(memory_buffer& buf, Duration duration) {
FMT_ASSERT(std::is_floating_point<typename Duration::rep>::value, "");
using Rep = typename Duration::rep;
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Rep -> rep to follow naming conventions

Comment on lines 1124 to 1125
void write_floating_seconds(memory_buffer& buf, Duration duration,
int precision) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, let's merge the two write_floating_seconds overloads.

Comment on lines 1581 to 1582
bool is_floating_point = false;
bool has_precision = false;
Copy link
Contributor

Choose a reason for hiding this comment

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

We only need one flag for the condition has_precision && !is_floating_point, something like has_precision_integral.


const auto fractional =
detail::abs(d) - std::chrono::duration_cast<std::chrono::seconds>(d);
const auto& subseconds =
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: & is not needed here

Comment on lines 1863 to 1869
if (precision >= 0) {
write_fractional_seconds<char_type>(
out, std::chrono::duration<rep, Period>(val), precision);
} else {
write_fractional_seconds<char_type>(
out, std::chrono::duration<rep, Period>(val));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that write_fractional_seconds overloads have been merged, the if statement is no longer needed. You can just call

write_fractional_seconds<char_type>(
              out, std::chrono::duration<rep, Period>(val), precision);

@vitaut vitaut merged commit 795ed8a into fmtlib:master Nov 2, 2022
@vitaut
Copy link
Contributor

vitaut commented Nov 2, 2022

Thanks!

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