RE: [boost] RE: Re: Formal review of "Output Formatters" librarybeginstoday
Vladimir Prus wrote:
Reece Dunn wrote:
You can explicitally state where/how new lines and indentation are going to be within my library by adding them to the open/close/separator formatting (see output-3D.cpp for an example).
Yes, but I'd rather write
os << v
and have the right formatting, even if 'v' is a vector of something, which has vector members, and the vector element type has another vector member, and so on...
This would further complicate the implementation of the library. To do what you want would require something similar to the Cascading Style Sheet proposal to maintain the default values for each format object type. This would be more complex for it to handle std::vector and std::list separately, but it may be possible to use the ID returned by type traits (differentiating sequential, associative and set containers).
The wrapping is indeed a separate issue. What I was asking is support of indenting in your library. For example, the output such as [snip] Yes, except that I'd want "multiline" to act recursively, so if I print vector of maps, the result will look like: [ { a: 1 b: 2 } { c: 3 } ]
or using a different syntax.
It would be possible to make use of a state object: a state object is an object that is rendered before or after an element is rendered. The implementation of this using the review version would be complex, but possible. I was considering adding pre+post support for a state object. Another idea is to implement an "event object" that gets called at the various stages of output. e.g.: Event::handler( before_decorator( io::open_decorator_id )); os << open; Event::handler( after_decorator( io::open_decorator_id )); // ... I'll need to think about this.
I'd suggest that you at least try to make top-level functions non-inline. That would reduce the code size for application which use your library -- I've had this experience both with program_options and function. For a single library, it's probably not a huge difference, but if all libraries use inline less liberaly, the code size of C++ apps would be lower.
ok. noted.
I might be missing something, but the mechanism for getting the type of formatter from a type to be output seems too complex. First, the type_deducer.hpp file is used, and 'select' computes a 'category'. Then format_deducer.hpp takes the category, and again uses 'select' to obtain the real type of formatter. Why the type_deducer.hpp is needed?
...
template<> struct deduce_type< io::seq_container_type > { template< typename CharT, typename T > struct type_from { typedef typename T::value_type value_type; typedef typename get_deducer< CharT, value_type >::type value_deducer;
typedef container_t < CharT, typename value_deducer::format_object > format_object;
static format_object deduce( const T & ) { return( format_object( value_deducer::deduce( value_type())));
Does this expects the T::value_type is DefaultConstructible? IIRC, containers only require that objects be CopyConstructible and Assignable.
Don't std::list implementations work like: struct list_node { T value; list_node * prev, next; }; typedef list_node list_node_block[ 100 ]; // ... so T needs to be DefaultConstructible. I'll need to consult C++98 to be sure.
}
Still, I don't understand the need for integer type category. Can't you have
'deduce_sequence_type' 'deduce_pair_type'
and so on, and use 'select' to map a type directly into those classes, not into integer?
Then I would need to do: select < is_std_vector< T >, deduce_sequence_type, is_std_list< T >, deduce_sequence_type, ...
This would not allow you to add a new container type into the mechanism without obtrusively altering the code. Also, there is a limit to the number of template parameters on select. The present system simplifies this by mapping to a category. (This also allows you to write more generic code, for example fmt::pair is implemented using these categories to work out how the pair type is constructed and fmt::container uses it to provide different behaviour for sequential, associative and set-based containers). In the new type deduction system, I map type --> type to extract a format object type, e.g. assoc_container_type to seq_container_type. This allows you to only provide one template overload. This system is more extensible than the review version (using select).
"providing an extensible framework that sits"
How it's extensible? I see only one section about extending and it's
just
one paragraph long
I really need to add more documentation regarding this. The idea is that you can:
* add your own decorators (delimiters in the review docs) to support more complex types (e.g. trees and graphs).
That's interesting. Can you explain?
You can write a class similar to [openclose_]formatter to add your own decorator type, e.g.: // new style template< typename CharT > struct binary_tree_decorators { decoration< CharT, 3 > left; decoration< CharT, 4 > right; // ... }; where 3 and 4 are the decorator IDs for the left and right decorations. You can then make use of this in a tree format object, e.g.: os << tree.left << left << tree.value << right << tree.right; resulting in something like: 1 <-- 2 --> 3 combining this with fmt::wrapper: [[1] <-- 2 --> [3] ] <-- 4 --> [[] <-- 5 --> [[6] <-- 7 --> []]] This is just an example. Consideration would need to be given to how the data is nested and how it interacts with the rest of the library.
Underlying type is: typeid(ob).name().
Sorry, that's the name of type ;-) Maybe you just mean "according to its type?" (also note "its", not "it's"). The "underlying" implies there's some another type, besides 'T'.
yes!
Maybe, you can use something like:
"The 'object' function can only output a container, so to output a range of iterators you need to make a container from it using io::range".
"output a container": the argument is not necessarily a container (e.g. std::pair). How about: "The 'object' function can only output an object of type T (it cannot output ranges declared using two arguments), so to output a range of iterators you need to make a container-like object from it using io::range".
The original sentence sounds like the format is something external to openclose_formatter, and the class is just a proxy which allows to set it.
It is a holder for several decorations (e.g. open/close or open/close/separator), allowing you to set more than one of these and - in the discussion of using expressions - dealing with the evaluation of one of these expressions, e.g.: fmt::container() / " + "
The code block before this comment defines two classes openclose_formatter_t and openclose_formatter. Is this a typo, or you really have two classes?
This is not a typo. The _t variants take a template parameter that contains the return value of the 'format' ('decorate') functions. This is so that: std::cout << io::object( vec, fmt::container().decorate( " | " )); works properly.
Why don't 'decorate' return the same type as 'fmt::container()'?
Because they are implemented in wrapper_decorators_t [review version: openclose_formatter_t] and sequence_decorators_t [formatter_t]. To return the same type as fmt::container() would require implementing the 'decorate' function in the fmt::basic_container class (i.e. implementing 5 or 6 functions). You would need to do this for all the other format objects, including the ones that the user writes, so why do this? Regards, Reece _________________________________________________________________ Express yourself with cool new emoticons http://www.msn.co.uk/specials/myemo
Reece Dunn wrote:
Yes, but I'd rather write
os << v
and have the right formatting, even if 'v' is a vector of something, which has vector members, and the vector element type has another vector member, and so on...
This would further complicate the implementation of the library. To do what you want would require something similar to the Cascading Style Sheet proposal to maintain the default values for each format object type. This would be more complex for it to handle std::vector and std::list separately, but it may be possible to use the ID returned by type traits (differentiating sequential, associative and set containers).
Let's split my request in two. First, I want the ability to property indent the output. I think this should not be too hard. One possible strategy is to add another parameter, "indent" to the "write" function. The "indenting formatters", which I hope will be the default, will increase the indent before calling nested formatters. Without this, proper output of complex data will be impossible. You can say that I can define such formatters myself. I don't know how -- in parcular, where can I store the indent level? If this can't be implemented easily, then there's something wrong with the library design. And if it can be implemented, then it's reasonable to provide it in the library. Again, we need both single-line [1,2,3] and multiline - 1 - 2 - 3 output, so we probably need an 'multiline_formatter'. The point is that all nested structures are also multiline. The second questions is sequence vs. set vs. map. I don't know how hard it will be implement, but that would be nice addition.
The wrapping is indeed a separate issue. What I was asking is support of indenting in your library. For example, the output such as [snip] Yes, except that I'd want "multiline" to act recursively, so if I print vector of maps, the result will look like: [ { a: 1 b: 2 } { c: 3 } ]
or using a different syntax.
It would be possible to make use of a state object: a state object is an object that is rendered before or after an element is rendered. The implementation of this using the review version would be complex, but possible. I was considering adding pre+post support for a state object.
This starts to look complex. Maybe, the "state" object is just not necessary?
Another idea is to implement an "event object" that gets called at the various stages of output. e.g.:
Event::handler( before_decorator( io::open_decorator_id )); os << open; Event::handler( after_decorator( io::open_decorator_id ));
Oh, that's was too complex. An additional parameter to 'write' is way simpler.
template<> struct deduce_type< io::seq_container_type > { template< typename CharT, typename T > struct type_from { typedef typename T::value_type value_type; typedef typename get_deducer< CharT, value_type >::type value_deducer;
typedef container_t < CharT, typename value_deducer::format_object > format_object;
static format_object deduce( const T & ) { return( format_object( value_deducer::deduce( value_type())));
Does this expects the T::value_type is DefaultConstructible? IIRC, containers only require that objects be CopyConstructible and Assignable.
Don't std::list implementations work like: struct list_node { T value; list_node * prev, next; }; typedef list_node list_node_block[ 100 ]; // ...
so T needs to be DefaultConstructible. I'll need to consult C++98 to be sure.
I don't know about list, but C++98 has only two requirements I've mentioned.
Still, I don't understand the need for integer type category. Can't you have
'deduce_sequence_type' 'deduce_pair_type'
and so on, and use 'select' to map a type directly into those classes, not into integer?
Then I would need to do: select < is_std_vector< T >, deduce_sequence_type, is_std_list< T >, deduce_sequence_type, ...
This would not allow you to add a new container type into the mechanism without obtrusively altering the code.
How select < is_std_vector< T >, sequence_category. is_std_list< T >, sequence_category, ... > is better?
Also, there is a limit to the number of template parameters on select. The present system simplifies this by mapping to a category.
I still don't understand. If you map to a different type, how this helps with the limited number of template parameters.
(This also allows you to write more generic code, for example fmt::pair is implemented using these categories to work out how the pair type is constructed and fmt::container uses it to provide different behaviour for sequential, associative and set-based containers).
In the new type deduction system, I map type --> type to extract a format object type, e.g. assoc_container_type to seq_container_type. This allows you to only provide one template overload. This system is more extensible than the review version (using select).
I think I share some of Gennadiy's concerns. The 'formatter' notion is very simple -- essentially a functional object. The additional machinery on top of this is too contrived.
You can write a class similar to [openclose_]formatter to add your own decorator type, e.g.:
// new style template< typename CharT > struct binary_tree_decorators { decoration< CharT, 3 > left; decoration< CharT, 4 > right; // ... };
where 3 and 4 are the decorator IDs for the left and right decorations. You can then make use of this in a tree format object, e.g.:
os << tree.left << left << tree.value << right << tree.right;
resulting in something like: 1 <-- 2 --> 3
combining this with fmt::wrapper: [[1] <-- 2 --> [3] ] <-- 4 --> [[] <-- 5 --> [[6] <-- 7 --> []]]
This is just an example. Consideration would need to be given to how the data is nested and how it interacts with the rest of the library.
Sure, if formatter is a fancy function I can write a new one, no problem. But if I wrote a formatter for tree, for do I make your library use that formatter when outputting vector<tree>. How to I change default formatting of tree? I start to think that each formatter should have a static member "default_format" which a user can change.
Underlying type is: typeid(ob).name().
Sorry, that's the name of type ;-) Maybe you just mean "according to its type?" (also note "its", not "it's"). The "underlying" implies there's some another type, besides 'T'.
yes!
Huh, I don't understand you, then.
Maybe, you can use something like:
"The 'object' function can only output a container, so to output a range of iterators you need to make a container from it using io::range".
"output a container": the argument is not necessarily a container (e.g. std::pair). How about:
"The 'object' function can only output an object of type T (it cannot output ranges declared using two arguments), so to output a range of iterators you need to make a container-like object from it using io::range".
I'd prefer "The 'object' function can only output an single object, so to output a range of iterators you need to make a container-like object from it using io::range". This avoid a questionable phrase "declared using two arguments" and avoid mentioning "type T", which does not add more information.
so that: std::cout << io::object( vec, fmt::container().decorate( " | " )); works properly.
Why don't 'decorate' return the same type as 'fmt::container()'?
Because they are implemented in wrapper_decorators_t [review version: openclose_formatter_t] and sequence_decorators_t [formatter_t]. To return the same type as fmt::container() would require implementing the 'decorate' function in the fmt::basic_container class (i.e. implementing 5 or 6 functions). You would need to do this for all the other format objects, including the ones that the user writes, so why do this?
Why? I presumed that all formatters have 'open', 'close' and 'separator' properties. Then, you'd have 'formatter' class which defines those properties as well as the 'decorate' method, and fmt::basic_container would derive from 'formatter'. - Volodya
participants (16)
-
Reece Dunn -
Vladimir Prus