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

Green Day Support #3758

Closed
vitaut opened this issue Dec 17, 2023 · 7 comments · Fixed by #3906
Closed

Green Day Support #3758

vitaut opened this issue Dec 17, 2023 · 7 comments · Fixed by #3906

Comments

@vitaut
Copy link
Contributor

vitaut commented Dec 17, 2023

{fmt} doesn't support printing green day at the moment:

#include <fmt/chrono.h>
#include <fmt/color.h>

int main() {
  fmt::print(fg(fmt::color::green), "{}", std::chrono::day());
}

Error:

include/fmt/core.h:1594:63: error: implicit instantiation of undefined template 'fmt::detail::type_is_unformattable_for<const std::chrono::day, char>'
    type_is_unformattable_for<T, typename Context::char_type> _;
                                                              ^
...
include/fmt/core.h:1597:3: error: static assertion failed due to requirement 'formattable': Cannot format an argument. To make type T formattable provide a formatter<T> specialization: https://fmt.dev/latest/api.html#udt
  static_assert(
  ^
2 errors generated.
@vitaut vitaut changed the title Green Day support Green Day Support Dec 17, 2023
@zivshek
Copy link
Contributor

zivshek commented Dec 20, 2023

Looking at this, not only day(), but also month() and year(). It seems they do not have a formatter.

Adding a custom formatter does fix it, but should we support it? If we want to, where is the best place to define them?

@vitaut
Copy link
Contributor Author

vitaut commented Dec 20, 2023

The C++ standard mentions these formatters (https://eel.is/c++draft/time):

  template<class charT> struct formatter<chrono::day, charT>;
  template<class charT> struct formatter<chrono::month, charT>;
  template<class charT> struct formatter<chrono::year, charT>;

@zivshek
Copy link
Contributor

zivshek commented Dec 20, 2023

in chrono.h, it defined the formatters for weekday, duration, time_point, etc., putting the defines for day, month, and year there works. I'll do some clean ups.

@zivshek
Copy link
Contributor

zivshek commented Dec 20, 2023

The C++ standard mentions these formatters (https://eel.is/c++draft/time):

  template<class charT> struct formatter<chrono::day, charT>;
  template<class charT> struct formatter<chrono::month, charT>;
  template<class charT> struct formatter<chrono::year, charT>;

you're right, std should have those formatters, but __cpp_lib_format is false, did some research, is it std::format not supported yet in MSVC?

It's interesting, as C++ 20 supports formatting all the chrono types, why you had to define the formatters for duration, weekday etc. in fmt in the first place? I think the issue is to make fmt make use the std::formatter's.

@vitaut
Copy link
Contributor Author

vitaut commented Dec 20, 2023

why you had to define the formatters for duration, weekday etc. in fmt in the first place? I think the issue is to make fmt make use the std::formatter's.

Good question. std::chrono is usually wider available than std::format and considering that day/month/year formatters should be easy to implement I think we should just do it ourselves.

@vitaut
Copy link
Contributor Author

vitaut commented Dec 21, 2023

Also year_month_day mentioned in #3772.

@razaqq
Copy link
Contributor

razaqq commented Dec 21, 2023

There are a couple more of interest.

grafik

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants