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.

Sign up for GitHub

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

Restructure printf_arg_formatter to make it customizable #1093

Merged
merged 1 commit into from
Apr 15, 2019
Merged

Restructure printf_arg_formatter to make it customizable #1093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 30 additions & 17 deletions include/fmt/printf.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,7 @@ using internal::printf; // For printing into memory_buffer.

template <typename Range> class printf_arg_formatter;

template <typename OutputIt, typename Char,
typename ArgFormatter =
printf_arg_formatter<back_insert_range<internal::buffer<Char>>>>
template <typename OutputIt, typename Char>
class basic_printf_context;

/**
Expand All @@ -212,12 +210,12 @@ class printf_arg_formatter
: public internal::function<
typename internal::arg_formatter_base<Range>::iterator>,
public internal::arg_formatter_base<Range> {
public:
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think these should be public because the children won't be able to use these types directly anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. But I have to keep the iterator public (like in the arg_formatter) to get my approach working.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, making iterator public makes sense.

typedef decltype(internal::declval<Range>().begin()) iterator;
private:
typedef typename Range::value_type char_type;
typedef decltype(internal::declval<Range>().begin()) iterator;
typedef internal::arg_formatter_base<Range> base;
typedef basic_printf_context<iterator, char_type, printf_arg_formatter>
context_type;
typedef basic_printf_context<iterator, char_type> context_type;

context_type& context_;

Expand Down Expand Up @@ -328,7 +326,7 @@ template <typename T> struct printf_formatter {
};

/** This template formats data and writes the output to a writer. */
template <typename OutputIt, typename Char, typename ArgFormatter>
template <typename OutputIt, typename Char>
class basic_printf_context {
public:
/** The character type for the output. */
Expand Down Expand Up @@ -379,11 +377,12 @@ class basic_printf_context {
}

/** Formats stored arguments and writes the output to the range. */
template<typename ArgFormatter = printf_arg_formatter<back_insert_range<internal::buffer<Char>>>>
OutputIt format();
};

template <typename OutputIt, typename Char, typename AF>
void basic_printf_context<OutputIt, Char, AF>::parse_flags(format_specs& spec,
template <typename OutputIt, typename Char>
void basic_printf_context<OutputIt, Char>::parse_flags(format_specs& spec,
const Char*& it,
const Char* end) {
for (; it != end; ++it) {
Expand All @@ -409,18 +408,18 @@ void basic_printf_context<OutputIt, Char, AF>::parse_flags(format_specs& spec,
}
}

template <typename OutputIt, typename Char, typename AF>
typename basic_printf_context<OutputIt, Char, AF>::format_arg
basic_printf_context<OutputIt, Char, AF>::get_arg(unsigned arg_index) {
template <typename OutputIt, typename Char>
typename basic_printf_context<OutputIt, Char>::format_arg
basic_printf_context<OutputIt, Char>::get_arg(unsigned arg_index) {
if (arg_index == std::numeric_limits<unsigned>::max())
arg_index = parse_ctx_.next_arg_id();
else
parse_ctx_.check_arg_id(--arg_index);
return internal::get_arg(*this, arg_index);
}

template <typename OutputIt, typename Char, typename AF>
unsigned basic_printf_context<OutputIt, Char, AF>::parse_header(
template <typename OutputIt, typename Char>
unsigned basic_printf_context<OutputIt, Char>::parse_header(
const Char*& it, const Char* end, format_specs& spec) {
unsigned arg_index = std::numeric_limits<unsigned>::max();
char_type c = *it;
Expand Down Expand Up @@ -457,8 +456,9 @@ unsigned basic_printf_context<OutputIt, Char, AF>::parse_header(
return arg_index;
}

template <typename OutputIt, typename Char, typename AF>
OutputIt basic_printf_context<OutputIt, Char, AF>::format() {
template <typename OutputIt, typename Char>
template <typename ArgFormatter>
OutputIt basic_printf_context<OutputIt, Char>::format() {
auto out = this->out();
const Char* start = parse_ctx_.begin();
const Char* end = parse_ctx_.end();
Expand Down Expand Up @@ -567,7 +567,7 @@ OutputIt basic_printf_context<OutputIt, Char, AF>::format() {
start = it;

// Format argument.
visit_format_arg(AF(out, spec, *this), arg);
visit_format_arg(ArgFormatter(out, spec, *this), arg);
}
return std::copy(start, it, out);
}
Expand Down Expand Up @@ -712,6 +712,19 @@ inline int vfprintf(
return static_cast<int>(buffer.size());
}

/** Formats arguments and writes the output to the range. */
template <typename ArgFormatter, typename Char,
typename Context = basic_printf_context<
typename ArgFormatter::iterator, Char>>
typename ArgFormatter::iterator vprintf(internal::buffer<Char>& out,
basic_string_view<Char> format_str,
basic_format_args<Context> args) {
typename ArgFormatter::iterator iter(out);
Context(iter, format_str, args).template format<ArgFormatter>();
return iter;
}


/**
\rst
Prints formatted data to the stream *os*.
Expand Down
48 changes: 48 additions & 0 deletions test/printf-test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -554,3 +554,51 @@ TEST(PrintfTest, VSPrintfMakeWArgsExample) {
fmt::make_wprintf_args(42, L"something")));
#endif
}

typedef fmt::printf_arg_formatter<
fmt::back_insert_range<fmt::internal::buffer<char>>> formatter_t;
typedef fmt::basic_printf_context<formatter_t::iterator, char> context_t;

// A custom printf argument formatter that doesn't print `-` for floating-point
// values rounded to 0.
class custom_printf_arg_formatter : public formatter_t {
public:
using formatter_t::iterator;

custom_printf_arg_formatter(formatter_t::iterator iter, formatter_t::format_specs& spec, context_t& ctx)
: formatter_t(iter, spec, ctx) {}

using formatter_t::operator();

#if FMT_MSC_VER > 0 && FMT_MSC_VER <= 1804
template <typename T, FMT_ENABLE_IF(std::is_floating_point<T>::value)>
iterator operator()(T value) {
#else
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use the above SFINAE-based definition for all compilers and remove conditional compilation (adding a comment clarifying that this is a workaround for MSVC).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately this doesn't work. Other compilers (clang and gcc as far as I recall) complain then.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, let's keep it as is.

iterator operator()(double value) {
#endif
// Comparing a float to 0.0 is safe.
if (round(value * pow(10, spec()->precision)) == 0.0) value = 0;
return formatter_t::operator()(value);
}
};

typedef fmt::basic_format_args<context_t> format_args_t;

std::string custom_vformat(fmt::string_view format_str, format_args_t args) {
fmt::memory_buffer buffer;
fmt::vprintf<custom_printf_arg_formatter>(buffer, format_str, args);
return std::string(buffer.data(), buffer.size());
}

template <typename... Args>
std::string custom_format(const char* format_str, const Args&... args) {
auto va = fmt::make_printf_args (args...);
return custom_vformat(format_str, va);
}

TEST(CustomFormatterTest, Format) {
EXPECT_EQ("0.00", custom_format("%.2f", -.00001));
EXPECT_EQ("0.00", custom_format("%.2f", .00001));
EXPECT_EQ("1.00", custom_format("%.2f", 1.00001));
EXPECT_EQ("-1.00", custom_format("%.2f", -1.00001));
}