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

Implement %j specifier for std::chrono::duration #3732

Merged
merged 3 commits into from
Dec 3, 2023

Conversation

intelfx
Copy link
Contributor

@intelfx intelfx commented Dec 1, 2023

This adds support for %j presentation type for duration types:

"If the type being formatted is a specialization of duration, the decimal number of days without padding."

Fixes #3643.


I assume it isn't needed to handle padding or alternative numeric systems here as neither is mentioned in the specification, thus the implementation is basically one line. (Please correct me if I'm wrong.)

@intelfx intelfx changed the title Implement %j specifier for std::chrono::duration Implement %j specifier for std::chrono::duration Dec 1, 2023
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!

Comment on lines 1893 to 1894
if (handle_nan_inf()) return;
return write(days(), 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

This can be simplified to

if (!handle_nan_inf()) write(days(), 0);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did it this way to keep the code stylistically similar to the rest of the handlers which all have the same guard statement in the beginning of the function body. I can change it if you insist though :-)

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, but at the very least there shouldn't be return before write because this is a void function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, yes you are right of course, not sure where did I get this idea from.

test/chrono-test.cc Outdated Show resolved Hide resolved
@intelfx intelfx force-pushed the work/duration-on-day-of-year branch 2 times, most recently from 4b4f224 to 26ae9fb Compare December 2, 2023 16:05
test/chrono-test.cc Outdated Show resolved Hide resolved
This adds support for `%j` presentation type for duration types:

> "If the type being formatted is a specialization of duration, the decimal
number of days without padding."

Fixes fmtlib#3643.
@intelfx intelfx force-pushed the work/duration-on-day-of-year branch from 26ae9fb to 78a6117 Compare December 2, 2023 16:14
@vitaut vitaut merged commit 71bd51e into fmtlib:master Dec 3, 2023
40 checks passed
@vitaut
Copy link
Contributor

vitaut commented Dec 3, 2023

Merged, thanks!

happymonkey1 pushed a commit to happymonkey1/fmt that referenced this pull request Apr 7, 2024
This adds support for `%j` presentation type for duration types:

> "If the type being formatted is a specialization of duration, the decimal
number of days without padding."

Fixes fmtlib#3643.
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.

%j doesn't seem to work with a duration object
2 participants