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

Support character range formatting #3863

Merged
merged 9 commits into from
Mar 7, 2024
Merged

Support character range formatting #3863

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7a8288d
initial commit
js324 Feb 23, 2024
181dbcf
fix build
js324 Feb 23, 2024
d17e9c9
format
js324 Feb 23, 2024
556e412
clean up
js324 Feb 27, 2024
5ebe8c8
separate func for writing debug strings
js324 Feb 27, 2024
6f6f222
fix return type
js324 Feb 27, 2024
e36ec04
format
js324 Feb 27, 2024
645fde4
implement changes
js324 Mar 4, 2024
5151ee4
move error checking
js324 Mar 6, 2024
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
75 changes: 72 additions & 3 deletions include/fmt/ranges.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <type_traits>

#include "base.h"
js324 marked this conversation as resolved.
Show resolved Hide resolved
#include "format.h"

FMT_BEGIN_NAMESPACE

Expand Down Expand Up @@ -388,6 +389,8 @@ struct range_formatter<
detail::string_literal<Char, '['>{};
basic_string_view<Char> closing_bracket_ =
detail::string_literal<Char, ']'>{};
bool is_string_format = false;
bool is_debug = false;

public:
FMT_CONSTEXPR range_formatter() {}
Expand All @@ -414,20 +417,86 @@ struct range_formatter<
if (it != end && *it == 'n') {
js324 marked this conversation as resolved.
Show resolved Hide resolved
set_brackets({}, {});
++it;
} else {
vitaut marked this conversation as resolved.
Show resolved Hide resolved
bool check_for_s = false;
if (it != end && *it == '?') {
++it;
detail::maybe_set_debug_format(underlying_, true);
set_brackets({}, {});
check_for_s = true;
is_debug = true;
}
if (it != end && *it == 's') {
if (!std::is_same<T, Char>::value) {
report_error("invalid format specifier");
}
if (!is_debug) {
set_brackets(detail::string_literal<Char, '"'>{},
detail::string_literal<Char, '"'>{});
}
check_for_s = false;
is_string_format = true;
++it;
}
if (check_for_s) {
report_error("invalid format specifier");
}
vitaut marked this conversation as resolved.
Show resolved Hide resolved
}

if (it != end && *it != '}') {
if (*it != ':') report_error("invalid format specifier");
if (is_string_format || *it != ':')
js324 marked this conversation as resolved.
Show resolved Hide resolved
report_error("invalid format specifier");
++it;
} else {
detail::maybe_set_debug_format(underlying_, true);
if (!is_string_format) detail::maybe_set_debug_format(underlying_, true);
js324 marked this conversation as resolved.
Show resolved Hide resolved
}

ctx.advance_to(it);
return underlying_.parse(ctx);
}

template <typename R, typename FormatContext>
template <typename R, typename FormatContext, typename U = T,
enable_if_t<std::is_same<U, Char>::value, bool> = true>
auto format(R&& range, FormatContext& ctx) const -> decltype(ctx.out()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid duplication let's merge the two format overloads and either use if constexpr or move only the string-specific logic into a separate function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried if constexpr but its introduced in C++17, so tests would break. I pushed the changes to make the string logic a separate function, though I'm unsure if there's a way to cleanly handle the non-Char case, so any advice here would be appreciated.

detail::range_mapper<buffered_context<Char>> mapper;
auto out = ctx.out();
out = detail::copy<Char>(opening_bracket_, out);
int i = 0;
auto it = detail::range_begin(range);
auto end = detail::range_end(range);
if (is_string_format) {
if (is_debug) {
auto buf = basic_memory_buffer<Char>();
for (; it != end; ++it) {
auto&& item = *it;
buf.push_back(item);
}
format_specs spec_str{};
spec_str.type = presentation_type::debug;
detail::write<Char>(
out, basic_string_view<Char>(buf.data(), buf.size()), spec_str);
} else {
for (; it != end; ++it) {
ctx.advance_to(out);
auto&& item = *it;
out = underlying_.format(mapper.map(item), ctx);
}
}
} else {
for (; it != end; ++it) {
if (i > 0) out = detail::copy<Char>(separator_, out);
ctx.advance_to(out);
auto&& item = *it;
out = underlying_.format(mapper.map(item), ctx);
++i;
}
}
out = detail::copy<Char>(closing_bracket_, out);
return out;
}

template <typename R, typename FormatContext, typename U = T,
enable_if_t<!(std::is_same<U, Char>::value), bool> = true>
auto format(R&& range, FormatContext& ctx) const -> decltype(ctx.out()) {
detail::range_mapper<buffered_context<Char>> mapper;
auto out = ctx.out();
Expand Down
5 changes: 5 additions & 0 deletions test/ranges-test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,13 @@ TEST(ranges_test, format_vector) {
EXPECT_EQ(fmt::format("{:n:#x}", v), "0x1, 0x2, 0x3, 0x5, 0x7, 0xb");

auto vc = std::vector<char>{'a', 'b', 'c'};
auto vec = std::vector<char>{'a', '\n', '\t'};
auto vvc = std::vector<std::vector<char>>{vc, vc};
EXPECT_EQ(fmt::format("{}", vc), "['a', 'b', 'c']");
EXPECT_EQ(fmt::format("{:s}", vc), "\"abc\"");
EXPECT_EQ(fmt::format("{:?s}", vec), "\"a\\n\\t\"");
EXPECT_EQ(fmt::format("{:s}", vec), "\"a\n\t\"");
EXPECT_EQ(fmt::format("{::s}", vvc), "[\"abc\", \"abc\"]");
EXPECT_EQ(fmt::format("{}", vvc), "[['a', 'b', 'c'], ['a', 'b', 'c']]");
EXPECT_EQ(fmt::format("{:n}", vvc), "['a', 'b', 'c'], ['a', 'b', 'c']");
EXPECT_EQ(fmt::format("{:n:n}", vvc), "'a', 'b', 'c', 'a', 'b', 'c'");
Expand Down
Loading