fmtlib / fmt

A modern formatting library
https://fmt.dev
Other
19.84k stars 2.42k forks source link

Add glibc ext for day of month and week of year #3976

Closed ZaheenJ closed 1 month ago

ZaheenJ commented 1 month ago

Allows for "-", "_", and "0" glibc padding specifiers to be used with day of the month (%d) and week of the year (%W,%U,%V) specifiers. I quickly drafted this based on #3271 since I needed this for my Waybar, so tell me if something is wrong. Additionally, trivially support "%k" and "%l", which strftime supports as equivalents to "%_H" and "%_I" respectively.

ZaheenJ commented 1 month ago

For the failing lint checks, I can just fix that with clang-format. However I don't know how to fix the other failing checks. I had to change these lines to include the additional pad parameter for it to compile on my system, but it seems to make CI upset only for the C++20 mac and linux builds because it thinks the parameter is unused. https://github.com/fmtlib/fmt/blob/2627fa5870bb2f7ec973a4f16b2c3552bf7dca2d/include/fmt/chrono.h#L987-L991 https://github.com/fmtlib/fmt/blob/2627fa5870bb2f7ec973a4f16b2c3552bf7dca2d/include/fmt/chrono.h#L1936-L1939

tchaikov commented 1 month ago

@ZaheenJ in that particular case, you might need to take closer look at the proposed patch:

--- a/include/fmt/chrono.h
+++ b/include/fmt/chrono.h
@@ -984,11 +984,19 @@ template <typename Derived> struct null_chrono_spec_handler {
   FMT_CONSTEXPR void on_abbr_month() { unsupported(); }
   FMT_CONSTEXPR void on_full_month() { unsupported(); }
   FMT_CONSTEXPR void on_dec_month(numeric_system) { unsupported(); }
-  FMT_CONSTEXPR void on_dec0_week_of_year(numeric_system, pad_type pad) { unsupported(); }
-  FMT_CONSTEXPR void on_dec1_week_of_year(numeric_system, pad_type pad) { unsupported(); }
-  FMT_CONSTEXPR void on_iso_week_of_year(numeric_system, pad_type pad) { unsupported(); }
+  FMT_CONSTEXPR void on_dec0_week_of_year(numeric_system, pad_type pad) {
+    unsupported();
+  }
+  FMT_CONSTEXPR void on_dec1_week_of_year(numeric_system, pad_type pad) {
+    unsupported();
+  }
+  FMT_CONSTEXPR void on_iso_week_of_year(numeric_system, pad_type pad) {
+    unsupported();
+  }
   FMT_CONSTEXPR void on_day_of_year() { unsupported(); }
-  FMT_CONSTEXPR void on_day_of_month(numeric_system, pad_type pad) { unsupported(); }
+  FMT_CONSTEXPR void on_day_of_month(numeric_system, pad_type pad) {
+    unsupported();
+  }
   FMT_CONSTEXPR void on_24_hour(numeric_system) { unsupported(); }
   FMT_CONSTEXPR void on_12_hour(numeric_system) { unsupported(); }
   FMT_CONSTEXPR void on_minute(numeric_system) { unsupported(); }

the linter actually suggests just to wrap the line.

ZaheenJ commented 1 month ago

I didn't realize that the documentation for the "_", "-" and "0" padding modifiers was not there, I thought it would've been added in the original glibc extension PR. I added that based off the strftime man page if that's okay. I also will open a separate PR for the new specifiers.

vitaut commented 1 month ago

LGTM but please rebase and note that the syntax doc has been converted to Markdown (you can use pandoc to convert your changes).

vitaut commented 1 month ago

Thank you!