Implement fill/align/width parsing in chrono formatter

include/fmt/chrono.h

@@ -370,12 +370,50 @@ template <> FMT_CONSTEXPR const char *get_units<std::ratio<3600>>() {
 
 template <typename Rep, typename Period, typename Char>
 struct formatter<std::chrono::duration<Rep, Period>, Char> {
+ private:
+  align_spec spec;
+  internal::arg_ref<Char> width_ref;
   mutable basic_string_view<Char> format_str;
   typedef std::chrono::duration<Rep, Period> duration;
 
+  struct spec_handler {
+    formatter &f;
+    basic_parse_context<Char> &context;
+
+    typedef internal::arg_ref<Char> arg_ref_type;
+
+    template <typename Id>
+    FMT_CONSTEXPR arg_ref_type make_arg_ref(Id arg_id) {
+      context.check_arg_id(arg_id);
+      return arg_ref_type(arg_id);
+    }
+
+    FMT_CONSTEXPR arg_ref_type make_arg_ref(internal::auto_id) {
+      return arg_ref_type(context.next_arg_id());
+    }
+
+    void on_error(const char *msg) { throw format_error(msg); }
+    void on_fill(Char fill) { f.spec.fill_ = fill; }
+    void on_align(alignment align) { f.spec.align_ = align; }
+    void on_width(unsigned width) { f.spec.width_ = width; }
+
+    template <typename Id>
+    void on_dynamic_width(Id arg_id) {
+      f.width_ref = make_arg_ref(arg_id);
+    }
+  };
+
+ public:
+  formatter() : spec() {}
+
   FMT_CONSTEXPR auto parse(basic_parse_context<Char> &ctx)
       -> decltype(ctx.begin()) {
     auto begin = ctx.begin(), end = ctx.end();
+    if (begin == end) return begin;
+    spec_handler handler{*this, ctx};
+    begin = internal::parse_align(begin, end, handler);
+    if (begin == end) return begin;
+    begin = internal::parse_width(begin, end, handler);
     end = parse_chrono_format(begin, end, internal::chrono_format_checker());
     format_str = basic_string_view<Char>(&*begin, end - begin);
     return end;
@@ -386,13 +424,21 @@ struct formatter<std::chrono::duration<Rep, Period>, Char> {
       -> decltype(ctx.out()) {
     auto begin = format_str.begin(), end = format_str.end();
     if (begin == end || *begin == '}') {
+      memory_buffer buf;
       if (const char *unit = get_units<Period>())
-        return format_to(ctx.out(), "{}{}", d.count(), unit);
+        format_to(buf, "{}{}", d.count(), unit);
-      if (Period::den == 1)
+      else if (Period::den == 1)
-        return format_to(ctx.out(), "{}[{}]s", d.count(), Period::num);
+        format_to(buf, "{}[{}]s", d.count(), Period::num);
-      return format_to(ctx.out(), "{}[{}/{}]s",
+      else
-                       d.count(), Period::num, Period::den);
+        format_to(buf, "{}[{}/{}]s", d.count(), Period::num, Period::den);
+      typedef output_range<decltype(ctx.out()), Char> range;
+      basic_writer<range> w(range(ctx.out()));
+      internal::handle_dynamic_spec<internal::width_checker>(
+        spec.width_, width_ref, ctx);
+      w.write(buf.data(), buf.size(), spec);
+      return w.out();
     }
+    // TODO: use fill and align
     internal::chrono_formatter<FormatContext> f(ctx);
     f.s = std::chrono::duration_cast<std::chrono::seconds>(d);
     f.ms = std::chrono::duration_cast<std::chrono::milliseconds>(d - f.s);

include/fmt/format.h

@@ -1342,7 +1342,7 @@ class arg_formatter_base {
 
   void write(bool value) {
     string_view sv(value ? "true" : "false");
-    specs_ ? writer_.write_str(sv, *specs_) : writer_.write(sv);
+    specs_ ? writer_.write(sv, *specs_) : writer_.write(sv);
   }
 
   void write(const char_type *value) {
@@ -1350,7 +1350,7 @@ class arg_formatter_base {
       FMT_THROW(format_error("string pointer is null"));
     auto length = std::char_traits<char_type>::length(value);
     basic_string_view<char_type> sv(value, length);
-    specs_ ? writer_.write_str(sv, *specs_) : writer_.write(sv);
+    specs_ ? writer_.write(sv, *specs_) : writer_.write(sv);
   }
 
  public:
@@ -1426,7 +1426,7 @@ class arg_formatter_base {
     if (specs_) {
       internal::check_string_type_spec(
             specs_->type, internal::error_handler());
-      writer_.write_str(value, *specs_);
+      writer_.write(value, *specs_);
     } else {
       writer_.write(value);
     }
@@ -1840,15 +1840,11 @@ struct precision_adapter {
   SpecHandler &handler;
 };
 
-// Parses standard format specifiers and sends notifications about parsed
+// Parses fill and alignment.
-// components to handler.
+template <typename Char, typename Handler>
-template <typename Char, typename SpecHandler>
+FMT_CONSTEXPR const Char *parse_align(
-FMT_CONSTEXPR const Char *parse_format_specs(
+    const Char *begin, const Char *end, Handler &&handler) {
-    const Char *begin, const Char *end, SpecHandler &&handler) {
+  FMT_ASSERT(begin != end, "");
-  if (begin == end || *begin == '}')
-    return begin;
-
-  // Parse fill and alignment.
   alignment align = ALIGN_DEFAULT;
   int i = 0;
   if (begin + 1 != end) ++i;
@@ -1876,10 +1872,39 @@ FMT_CONSTEXPR const Char *parse_format_specs(
         handler.on_fill(c);
       } else ++begin;
       handler.on_align(align);
-      if (begin == end) return begin;
       break;
     }
   } while (i-- > 0);
+  return begin;
+}
+
+template <typename Char, typename Handler>
+FMT_CONSTEXPR const Char *parse_width(
+    const Char *begin, const Char *end, Handler &&handler) {
+  FMT_ASSERT(begin != end, "");
+  if ('0' <= *begin && *begin <= '9') {
+    handler.on_width(parse_nonnegative_int(begin, end, handler));
+  } else if (*begin == '{') {
+    ++begin;
+    if (begin != end)
+      begin = parse_arg_id(begin, end, width_adapter<Handler, Char>(handler));
+    if (begin == end || *begin != '}')
+      return handler.on_error("invalid format string"), begin;
+    ++begin;
+  }
+  return begin;
+}
+
+// Parses standard format specifiers and sends notifications about parsed
+// components to handler.
+template <typename Char, typename SpecHandler>
+FMT_CONSTEXPR const Char *parse_format_specs(
+    const Char *begin, const Char *end, SpecHandler &&handler) {
+  if (begin == end || *begin == '}')
+    return begin;
+
+  begin = parse_align(begin, end, handler);
+  if (begin == end) return begin;
 
   // Parse sign.
   switch (static_cast<char>(*begin)) {
@@ -1909,20 +1934,8 @@ FMT_CONSTEXPR const Char *parse_format_specs(
     if (++begin == end) return begin;
   }
 
-  // Parse width.
+  begin = parse_width(begin, end, handler);
-  if ('0' <= *begin && *begin <= '9') {
-    handler.on_width(parse_nonnegative_int(begin, end, handler));
   if (begin == end) return begin;
-  } else if (*begin == '{') {
-    ++begin;
-    if (begin != end) {
-      begin = parse_arg_id(
-            begin, end, width_adapter<SpecHandler, Char>(handler));
-    }
-    if (begin == end || *begin != '}')
-      return handler.on_error("invalid format string"), begin;
-    if (++begin == end) return begin;
-  }
 
   // Parse precision.
   if (*begin == '.') {
@@ -2251,8 +2264,6 @@ class basic_writer {
   iterator out_;  // Output iterator.
   internal::locale_ref locale_;
 
-  iterator out() const { return out_; }
-
   // Attempts to reserve space for n extra characters in the output range.
   // Returns a pointer to the reserved range or a reference to out_.
   auto reserve(std::size_t n) -> decltype(internal::reserve(out_, n)) {
@@ -2263,7 +2274,28 @@ class basic_writer {
   //   <left-padding><value><right-padding>
   // where <value> is written by f(it).
   template <typename F>
-  void write_padded(const align_spec &spec, F &&f);
+  void write_padded(const align_spec &spec, F &&f) {
+    unsigned width = spec.width(); // User-perceived width (in code points).
+    size_t size = f.size(); // The number of code units.
+    size_t num_code_points = width != 0 ? f.width() : size;
+    if (width <= num_code_points)
+      return f(reserve(size));
+    auto &&it = reserve(width + (size - num_code_points));
+    char_type fill = static_cast<char_type>(spec.fill());
+    std::size_t padding = width - num_code_points;
+    if (spec.align() == ALIGN_RIGHT) {
+      it = std::fill_n(it, padding, fill);
+      f(it);
+    } else if (spec.align() == ALIGN_CENTER) {
+      std::size_t left_padding = padding / 2;
+      it = std::fill_n(it, left_padding, fill);
+      f(it);
+      it = std::fill_n(it, padding - left_padding, fill);
+    } else {
+      f(it);
+      it = std::fill_n(it, padding, fill);
+    }
+  }
 
   template <typename F>
   struct padded_int_writer {
@@ -2525,15 +2557,6 @@ class basic_writer {
     }
   };
 
-  // Writes a formatted string.
-  template <typename Char>
-  void write_str(const Char *s, std::size_t size, const align_spec &spec) {
-    write_padded(spec, str_writer<Char>{s, size});
-  }
-
-  template <typename Char>
-  void write_str(basic_string_view<Char> str, const format_specs &spec);
-
   template <typename Char>
   friend class internal::arg_formatter_base;
 
@@ -2543,6 +2566,8 @@ class basic_writer {
       Range out, internal::locale_ref loc = internal::locale_ref())
     : out_(out.begin()), locale_(loc) {}
 
+  iterator out() const { return out_; }
+
   void write(int value) { write_decimal(value); }
   void write(long value) { write_decimal(value); }
   void write(long long value) { write_decimal(value); }
@@ -2602,9 +2627,20 @@ class basic_writer {
     it = std::copy(value.begin(), value.end(), it);
   }
 
-  template <typename... FormatSpecs>
+  // Writes a formatted string.
-  void write(basic_string_view<char_type> str, FormatSpecs... specs) {
+  template <typename Char>
-    write_str(str, format_specs(specs...));
+  void write(const Char *s, std::size_t size, const align_spec &spec) {
+    write_padded(spec, str_writer<Char>{s, size});
+  }
+
+  template <typename Char>
+  void write(basic_string_view<Char> s,
+             const format_specs &spec = format_specs()) {
+    const Char *data = s.data();
+    std::size_t size = s.size();
+    if (spec.precision >= 0 && internal::to_unsigned(spec.precision) < size)
+      size = internal::to_unsigned(spec.precision);
+    write(data, size, spec);
   }
 
   template <typename T>
@@ -2617,42 +2653,6 @@ class basic_writer {
   }
 };
 
-template <typename Range>
-template <typename F>
-void basic_writer<Range>::write_padded(const align_spec &spec, F &&f) {
-  unsigned width = spec.width(); // User-perceived width (in code points).
-  size_t size = f.size(); // The number of code units.
-  size_t num_code_points = width != 0 ? f.width() : size;
-  if (width <= num_code_points)
-    return f(reserve(size));
-  auto &&it = reserve(width + (size - num_code_points));
-  char_type fill = static_cast<char_type>(spec.fill());
-  std::size_t padding = width - num_code_points;
-  if (spec.align() == ALIGN_RIGHT) {
-    it = std::fill_n(it, padding, fill);
-    f(it);
-  } else if (spec.align() == ALIGN_CENTER) {
-    std::size_t left_padding = padding / 2;
-    it = std::fill_n(it, left_padding, fill);
-    f(it);
-    it = std::fill_n(it, padding - left_padding, fill);
-  } else {
-    f(it);
-    it = std::fill_n(it, padding, fill);
-  }
-}
-
-template <typename Range>
-template <typename Char>
-void basic_writer<Range>::write_str(
-    basic_string_view<Char> s, const format_specs &spec) {
-  const Char *data = s.data();
-  std::size_t size = s.size();
-  if (spec.precision >= 0 && internal::to_unsigned(spec.precision) < size)
-    size = internal::to_unsigned(spec.precision);
-  write_str(data, size, spec);
-}
-
 struct float_spec_handler {
   char type;
   bool upper;

test/chrono-test.cc

@@ -91,6 +91,14 @@ TEST(ChronoTest, FormatDefault) {
                         std::chrono::duration<int, std::ratio<15, 4>>(42)));
 }
 
+TEST(ChronoTest, Align) {
+  auto s = std::chrono::seconds(42);
+  EXPECT_EQ("42s  ", fmt::format("{:5}", s));
+  EXPECT_EQ("42s  ", fmt::format("{:{}}", s, 5));
+  EXPECT_EQ("  42s", fmt::format("{:>5}", s));
+  EXPECT_EQ("**42s**", fmt::format("{:*^7}", s));
+}
+
 TEST(ChronoTest, FormatSpecs) {
   EXPECT_EQ("%", fmt::format("{:%%}", std::chrono::seconds(0)));
   EXPECT_EQ("\n", fmt::format("{:%n}", std::chrono::seconds(0)));