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

[WIP] Implementation of fmt::prepare. Implements #618 (and probably #613) #949

Closed
wants to merge 3 commits into from
Closed

[WIP] Implementation of fmt::prepare. Implements #618 (and probably #613) #949

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8c99ec0
Implemented fmt::prepare()
stryku Nov 27, 2018
c735a69
Removed unnecessary changes
stryku Jan 7, 2019
7772ca7
Removed parsed_specs_checker class
stryku Jan 9, 2019
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
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ endfunction()

# Define the fmt library, its includes and the needed defines.
add_headers(FMT_HEADERS chrono.h color.h core.h format.h format-inl.h locale.h
ostream.h printf.h time.h ranges.h)
ostream.h prepare.h printf.h time.h ranges.h)
set(FMT_SOURCES src/format.cc)
if (HAVE_OPEN)
add_headers(FMT_HEADERS posix.h)
Expand Down
14 changes: 10 additions & 4 deletions include/fmt/chrono.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,22 +372,28 @@ 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;
typedef internal::arg_ref<Char> arg_ref_type;
arg_ref_type 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;
basic_string_view<Char> format_str;

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(basic_string_view<Char> arg_id) {
context.check_arg_id(arg_id);
const auto str_val = internal::string_view_metadata(format_str, arg_id);
return arg_ref_type(str_val);
}

FMT_CONSTEXPR arg_ref_type make_arg_ref(internal::auto_id) {
return arg_ref_type(context.next_arg_id());
}
Expand All @@ -410,7 +416,7 @@ struct formatter<std::chrono::duration<Rep, Period>, Char> {
-> decltype(ctx.begin()) {
auto begin = ctx.begin(), end = ctx.end();
if (begin == end) return begin;
spec_handler handler{*this, ctx};
spec_handler handler{*this, ctx, format_str};
begin = internal::parse_align(begin, end, handler);
if (begin == end) return begin;
begin = internal::parse_width(begin, end, handler);
Expand Down
15 changes: 7 additions & 8 deletions include/fmt/color.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -221,14 +221,13 @@ enum class emphasis : uint8_t {
// We use rgb as name because some editors will show it as color direct in the
// editor.
struct rgb {
FMT_CONSTEXPR_DECL rgb() : r(0), g(0), b(0) {}
FMT_CONSTEXPR_DECL rgb(uint8_t r_, uint8_t g_, uint8_t b_)
: r(r_), g(g_), b(b_) {}
FMT_CONSTEXPR_DECL rgb(uint32_t hex)
: r((hex >> 16) & 0xFF), g((hex >> 8) & 0xFF), b((hex) & 0xFF) {}
FMT_CONSTEXPR_DECL rgb(color hex)
: r((uint32_t(hex) >> 16) & 0xFF), g((uint32_t(hex) >> 8) & 0xFF),
b(uint32_t(hex) & 0xFF) {}
FMT_CONSTEXPR rgb() : r(0), g(0), b(0) {}
stryku marked this conversation as resolved.
Show resolved Hide resolved
FMT_CONSTEXPR rgb(uint8_t r_, uint8_t g_, uint8_t b_) : r(r_), g(g_), b(b_) {}
FMT_CONSTEXPR rgb(uint32_t hex)
: r((hex >> 16) & 0xFF), g((hex >> 8) & 0xFF), b((hex)&0xFF) {}
FMT_CONSTEXPR rgb(color hex)
: r((uint32_t(hex) >> 16) & 0xFF), g((uint32_t(hex) >> 8) & 0xFF),
b(uint32_t(hex) & 0xFF) {}
uint8_t r;
uint8_t g;
uint8_t b;
Expand Down
30 changes: 22 additions & 8 deletions include/fmt/core.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,9 +454,11 @@ template <typename Char>
inline basic_string_view<Char>
to_string_view(basic_string_view<Char> s) { return s; }

template <typename Char>
template <typename Char, typename Traits, typename Allocator>
inline basic_string_view<Char>
to_string_view(const std::basic_string<Char> &s) { return s; }
to_string_view(const std::basic_string<Char, Traits, Allocator> &s) {
return {s.data(), s.size()};
}

template <typename Char>
inline basic_string_view<Char> to_string_view(const Char *s) { return s; }
Expand Down Expand Up @@ -504,6 +506,8 @@ template <typename T, typename Char, typename Enable = void>
struct convert_to_int: std::integral_constant<
bool, !std::is_arithmetic<T>::value && std::is_convertible<T, int>::value> {};

template <typename Char> struct basic_format_specs;

namespace internal {

struct dummy_string_view { typedef void char_type; };
Expand Down Expand Up @@ -877,16 +881,18 @@ FMT_CONSTEXPR typename internal::result_of<Visitor(int)>::type
template <typename Char, typename ErrorHandler = internal::error_handler>
class basic_parse_context : private ErrorHandler {
private:
basic_string_view<Char> format_str_;
int next_arg_id_;
basic_string_view<Char> primary_format_str_;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is primary_format_str_ needed? Shouldn't format_str_ be enough to resolve references?

Also the indent is a bit off.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIU, while parsing/formatting, basic_parse_context::advance_to() can be called, which calls format_str_.remove_prefix(), which 'invalidates' format_str_ and arg_ref::name (which is string_view_metadata) can not be created because it needs primary format string to calculate offset from the beginning of the format. Does that make sense, or I just messed up something?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right now it is not a problem because we agreed to address custom formatting separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So basic_parse_context::advance_to() is called only for custom formatters?

Copy link
Contributor

Choose a reason for hiding this comment

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

More or less. The whole context machinery is there to support extension mechanism (custom formatters).

basic_string_view<Char> format_str_;
int next_arg_id_;

public:
typedef Char char_type;
typedef typename basic_string_view<Char>::iterator iterator;

explicit FMT_CONSTEXPR basic_parse_context(
basic_string_view<Char> format_str, ErrorHandler eh = ErrorHandler())
: ErrorHandler(eh), format_str_(format_str), next_arg_id_(0) {}
explicit FMT_CONSTEXPR basic_parse_context(basic_string_view<Char> format_str,
ErrorHandler eh = ErrorHandler())
: ErrorHandler(eh), primary_format_str_(format_str),
format_str_(format_str), next_arg_id_(0) {}

// Returns an iterator to the beginning of the format string range being
// parsed.
Expand All @@ -913,13 +919,17 @@ class basic_parse_context : private ErrorHandler {
next_arg_id_ = -1;
return true;
}
void check_arg_id(basic_string_view<Char>) {}

FMT_CONSTEXPR void check_arg_id(basic_string_view<Char>) {}

FMT_CONSTEXPR void on_error(const char *message) {
ErrorHandler::on_error(message);
}

FMT_CONSTEXPR ErrorHandler error_handler() const { return *this; }
FMT_CONSTEXPR basic_string_view<Char> primary_format() const {
return primary_format_str_;
}
};

typedef basic_parse_context<char> format_parse_context;
Expand Down Expand Up @@ -1038,6 +1048,10 @@ class context_base {
void advance_to(iterator it) { out_ = it; }

locale_ref locale() { return loc_; }

basic_string_view<Char> primary_format() const {
return parse_context_.primary_format();
}
};

template <typename Context, typename T>
Expand Down
Loading