From d45544d14e8c033179f23f460037f93c75c91bb3 Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Wed, 27 Sep 2017 19:04:15 -0700 Subject: [PATCH] Fix width handling in dynamic formatting --- fmt/format.h | 83 ++++++++++++++++++++++++++++----------------- test/format-test.cc | 1 + 2 files changed, 52 insertions(+), 32 deletions(-) diff --git a/fmt/format.h b/fmt/format.h index 98f33e13..0035fa4e 100644 --- a/fmt/format.h +++ b/fmt/format.h @@ -1875,36 +1875,70 @@ class arg_formatter_base { } }; +// Parsing context representing a format string range being parsed and an +// argument counter for automatic indexing. template class parse_context { private: basic_string_view format_str_; + int next_arg_index_; + + protected: + bool check_no_auto_index(const char *&error) { + if (next_arg_index_ > 0) { + error = "cannot switch from automatic to manual argument indexing"; + return false; + } + next_arg_index_ = -1; + return true; + } public: using char_type = Char; + using iterator = const Char*; explicit parse_context(basic_string_view format_str) - : format_str_(format_str) {} + : format_str_(format_str), next_arg_index_(0) {} - const Char *begin() const { return format_str_.begin(); } - const Char *end() const { return format_str_.end(); } + // Returns an iterator to the beginning of the format string range being + // parsed. + iterator begin() const { return format_str_.begin(); } - void advance_to(const Char *p) { - format_str_.remove_prefix(p - begin()); + // Returns an iterator past the end of the format string range being parsed. + iterator end() const { return format_str_.end(); } + + // Advances the begin iterator to ``it``. + void advance_to(iterator it) { + format_str_.remove_prefix(it - begin()); } + + // Returns the next argument index. + unsigned next_arg_index(const char *&error) { + if (next_arg_index_ >= 0) + return internal::to_unsigned(next_arg_index_++); + error = "cannot switch from manual to automatic argument indexing"; + return 0; + } + + void check_arg_id(unsigned index) { + const char *error = 0; + if (!check_no_auto_index(error)) + FMT_THROW(format_error(error)); + } + + void check_arg_id(basic_string_view) {} }; template class context_base : public parse_context{ private: basic_args args_; - int next_arg_index_; protected: typedef basic_arg format_arg; context_base(basic_string_view format_str, basic_args args) - : parse_context(format_str), args_(args), next_arg_index_(0) {} + : parse_context(format_str), args_(args) {} ~context_base() {} basic_args args() const { return args_; } @@ -1924,23 +1958,6 @@ class context_base : public parse_context{ this->do_get_arg(arg_index, error) : format_arg(); } - // Returns the next argument index. - unsigned next_arg_index(const char *&error) { - if (next_arg_index_ >= 0) - return internal::to_unsigned(next_arg_index_++); - error = "cannot switch from manual to automatic argument indexing"; - return 0; - } - - bool check_no_auto_index(const char *&error) { - if (next_arg_index_ > 0) { - error = "cannot switch from automatic to manual argument indexing"; - return false; - } - next_arg_index_ = -1; - return true; - } - public: parse_context &get_parse_context() { return *this; } }; @@ -2018,7 +2035,6 @@ class basic_context : format_arg get_arg(unsigned arg_index) { const char *error = 0; - this->check_no_auto_index(error); format_arg arg = this->do_get_arg(arg_index, error); if (error) FMT_THROW(format_error(error)); @@ -3221,6 +3237,7 @@ class specs_handler: public specs_setter { template basic_arg get_arg(Id arg_id) { + context_.check_arg_id(arg_id); return context_.get_arg(arg_id); } @@ -3254,8 +3271,6 @@ struct dynamic_format_specs : basic_format_specs { // Format spec handler that saves references to arguments representing dynamic // width and precision to be resolved at formatting time. -// ParseContext: parsing context representing a sequence of format string -// characters and an argument counter for automatic indexing. template class dynamic_specs_handler : public specs_setter { @@ -3285,8 +3300,11 @@ class dynamic_specs_handler : } arg_ref_type make_arg_ref(auto_id) { - // TODO: get next index from context - return arg_ref_type(arg_ref_type::NONE); + const char *error = 0; + auto index = context_.next_arg_index(error); + if (error) + FMT_THROW(format_error(error)); + return arg_ref_type(index); } dynamic_format_specs &specs_; @@ -3477,7 +3495,6 @@ static void handle_dynamic_spec( Spec &value, arg_ref ref, basic_context &ctx) { switch (ref.kind) { case arg_ref::NONE: - // Do nothing. break; case arg_ref::INDEX: internal::set_dynamic_spec(value, ctx.get_arg(ref.index)); @@ -3485,7 +3502,6 @@ static void handle_dynamic_spec( case arg_ref::NAME: internal::set_dynamic_spec(value, ctx.get_arg(ref.name)); break; - // TODO: handle automatic numbering } } } // namespace internal @@ -3635,7 +3651,10 @@ void vformat_to(basic_buffer &buffer, basic_string_view format_str, id_handler(Context &c, basic_arg &a): context(c), arg(a) {} void operator()() { arg = context.next_arg(); } - void operator()(unsigned id) { arg = context.get_arg(id); } + void operator()(unsigned id) { + context.check_arg_id(id); + arg = context.get_arg(id); + } void operator()(basic_string_view id) { arg = context.get_arg(id); } diff --git a/test/format-test.cc b/test/format-test.cc index 7ceb4d4b..f50f1b3d 100644 --- a/test/format-test.cc +++ b/test/format-test.cc @@ -1561,6 +1561,7 @@ TEST(FormatTest, DynamicFormatter) { auto str = variant("foo"); EXPECT_EQ("42", format("{:d}", num)); EXPECT_EQ("foo", format("{:s}", str)); + EXPECT_EQ(" 42 foo ", format("{:{}} {:{}}", num, 3, str, 4)); EXPECT_THROW_MSG(format("{:=}", str), format_error, "format specifier '=' requires numeric argument"); EXPECT_THROW_MSG(format("{:+}", str),