AMDG I mostly have documentation notes at this point. Andrey, since I have so many, and they're mostly fairly minor, would it be better if I made a patch for the documentation source? I think newbies should probably be replace globally with new users, since I kind of feel that the term newbie is somewhat derogatory. http://boost-log.sourceforge.net/libs/log/doc/html/log/moti.html: ================================================================================ Mismatched number: Most of the time ***applications*** ... to monitor ***its*** execution... The beginning of the second paragraph could be improved: "This is where logging may help. The application stores all essential information of its run time to log, and once something goes wrong this information can be used to analyze program behavior and make necessary corrections." Here's my suggestion: "This is where logging can help. The application stores all the essential information about its execution to a log, and when something goes wrong this information can be used to analyze the program's behavior and make the necessary corrections." "out-of-box" should be "out-of-the-box" This phrase sounds rather clunky to me: "...along with public interfaces ready to be used to extend the library." This too: "A user should be able to extend functionality of the library with regard to collecting and storing information into logs." "The library should make as least performance impact..." should be "The library should have as little performance impact..." http://boost-log.sourceforge.net/libs/log/doc/html/log/how_to_read.html: ================================================================================ In the beginning of the second paragraph, I don't like the phrase "to have the first glance on" How about something like "...to get a feel for the library's capabilities..." "The tutorial gives the overview of the library features..." Suggestion: "The tutorial gives an overview of the library's features..." "The simple form typically describes the most common and easy way to do the task and it is being recommended to be read by newbies." Suggestion: "The simple form typically describes the most common and easy way to do the task and is recommended for new users." "The advanced form usually gives an expanded way to do the same thing but with ability to do some extra customization and in-depth explanation." I would like a definite article before "ability" and I would also like "and in-depth explanation" to be placed differently so that it doesn't sound like you're saying that the user has the ability to do in-depth explanation. "This form may come handy for more experienced users and should generally be read if your needs are not satisfied with the easy way." Suggestion: "This form may come in handy for more experienced users and should generally be read if the easy way does not satisfy your needs." "This part gives description of other tools..." should be "This part gives descriptions of other tools..." "This chapter is better to be read on per-case basis." Suggestion: "This chapter is best read on a case by case basis." "there is a reference which gives the formal description of library components. " The work "description" should be plural, "descriptions". "For simplicity in the code snippets in this documentation it shall be assumed that the following namespace aliases were defined" Suggestion: "To keep the code snippets in this documentation simple, the following namespace aliases are assumed to be defined" "Aside from these namespaces the library also provides boost::log::experimental namespace." Suggestion: "In addition to these namespaces the library also provides the boost::log::experimental namespace." "and thus are not advertized for productional usage" The word "productional" should be "production" http://boost-log.sourceforge.net/libs/log/doc/html/log/supported_compilers.h...: ================================================================================ "The library should build and work with a reasonably well-compliant compiler" delete "well-" http://boost-log.sourceforge.net/libs/log/doc/html/log/installation.html ================================================================================ "The library has the compiled part which should be built as described in the Getting Started article." Suggestion: "The library has a separately compiled part which should be built as described in the Getting Started guide." For BOOST_LOG_NO_THREADS "Has effect on both the library compilation and user's code compilation." Suggestion: "Affects compilation of both the library and users' code." "The macro is automatically defined if no threading support detected." add "is" before "detected" For: BOOST_LOG_USE_CHAR Same as the first comment for BOOST_LOG_NO_THREADS. (This appear many times, I won't repeat it again) For: BOOST_LOG_USE_WCHAR_T "it is assumed the both character types support is enabled" Suggestion: "support for both character types will be enabled" "Defining only one of them allows to significantly reduce binary size of the library." Suggestion: "Defining only one of them significantly reduces the binary size of the library." For BOOST_LOG_NO_QUERY_PERFORMANCE_COUNTER: "If defined, disables support for QueryPerformanceCounter API" Suggestion: "If defined, disables support for the QueryPerformanceCounter API" For BOOST_LOG_USE_NATIVE_SYSLOG: "Has effect on the library compilation stage." Suggestion: "Affects only the compilation of the library." (repeated later as well) "If for some reason the support for native SysLog API is not detected, define this macro to forcibly enable support for native SysLog." Suggestion: "If for some reason support for the native SysLog API is not detected automatically, define this macro to forcibly enable it." For BOOST_LOG_NO_SETTINGS_PARSERS_SUPPORT: "If defined, all facilities related to settings parsing are not built." Suggestion: "If defined, none of the facilities related to the parsers for settings will be built." "You may define configuration macros..." should be "You can define configuration macros..." http://boost-log.sourceforge.net/libs/log/doc/html/log/defs.html ================================================================================ For Log record: I don't like the expression "A single pack of information," although I can't think of anything better off hand. "that is candidate to be put into log" Suggestion: "that is a candidate to be put into the log" For Attribute "functional objects" should be "function objects" "which return the actual attribute value upon invoking" Suggestion: "which return the actual attribute value when invoked" For Log sink: "collected from user's application" should be "collected from the user's application" "It is sink's nature that defines where..." Suggestion: "The sink defines where..." For Log source "An entry point for user's application..." should be "An entry point for the user's application..." "upon user's request" should be "upon the user's request" For Log formatter "functional object" should be "function object" "...that forms up the final shape of the textual output" is awkward. "Some sinks, like binary logging sink" should be "Some sinks, like the binary logging sink" For Logging core "The global entity that maintains connection between" should be "The global entity that maintains the connection between" "It is mainly used on the logging library initialization stage." replace "on" with "in." http://boost-log.sourceforge.net/libs/log/doc/html/log/design.html ================================================================================ I htink "scheme" with "figure" "The library provides configuration facilities to leave only one needed version of the library to be compiled." Suggestion: "The library provides configuration facilities to compile only the version of the library needed." "For example, if your application is in critical state" should be "For example, if your application is in a critical state" "so that user sees an error message..." should be "so that the user sees an error message..." "This allows to use the library" Suggestion: "This allows use of the library" "...require users to use loggers to write logs" "use" and "log" are used too many times here. "The more generic term "log source" is used to designate the entity" Suggestion: "The more generic term "log source" designates the entity" "You may see on the scheme that the" suggestion: "You can see on the scheme that the" "and allows to quickly wipe away unneeded log records" suggestion: "and allows quickly wiping away unneeded log records" anyway, don't split infinitives. "The sink-specific filtering allows to direct log records to particular sinks." Suggestion: "The sink-specific filtering allows log records to be directed to particular sinks." "Backends, on the opposite" suggestion: "Backends, on the other hand" "most probable places of the library extension" suggestion "most likely places for extending the library" "Along with the described above primary facilities" should be "Along with the primary facilities described above" ================================================================================ Issues: Instead of compiling a narrow or wide library, why don't you use libboost_log and libboost_wlog? I agree with other comments that I saw about Attribute being a poor choice for what it does. I would prefer that the term "Attribute" should be used for what you call an "Attribute Value," and that there should be a different term, like "Attribute Extractor" for what you call an "Attribute." In Christ, Steven Watanabe
AMDG I've finished reading through all the documentation except for the reference. Here are the comments I have so far. Overall the documentation is quite good. I thought that the Boost.Parameter convention is to give keywords names starting with an _. I think that it would be a little nicer to work with attributes with an interface like this: BOOST_LOG_ATTRIBUTE(logging::trivial::severity_level, severity); BOOST_LOG_ATTRIBUTE(unsigned int, line_id); fmt::stream << fmt::line_id() << ": <" << fmt::severity() << "> " << fmt::message() That way, it's easier to guarantee static type safety and is also a little less verbose when accessing the attributes. Since you allow use of Boost.Format, I don't think that the extra format argument of attr is necessary. I don't like the inconsistency of "sample_%N.log", vs. "[%TimeStamp%]: %_%" Could you use "sample_%N%.log"? Nevermind - I see that you're matching Boost.DateTime I would like logging::extract< unsigned int > to return a boost::optional or something like that instead of having an output parameter. Is it possible to declare and define a global logger separately? I think LineID may be a bad name, since when I saw it, I assumed it meant the line in the source that generated the log message, perhaps RecordID would be better? I don't like the inconsistency of logging::make_attr_ordering< unsigned int >("Line #"). I though the name was LineID? I don't like the use of a time in ordering_asynchronous_sink to specify how long to wait for a new record. It just seems too fragile. Suppose that the machine is running under a heavy load or I suspend the process, or put the machine to sleep? I don't like having the correctness depend on how long things take. If nothing else, it needs to be stated that ordering_asynchronous_sink is only a "best effort" mechanism and makes no guarantees. fmt::xml_dec. I would prefer a more intuitive name like xml_encode or xml_escape. (Since dec is "obviously" an abbreviation of decrement) In http://boost-log.sourceforge.net/libs/log/doc/html/log/detailed/sink_backend..., the documentation says that custom_level_mapping works for integral types, but the example uses std::string. Some of the code examples overflow the window. Why are there separate classes for handling the syslog and windows event log mappings? It seems like they're basically doing the same thing. Using scoped attributes to add attributes to a specific log record seems like a hack. I don't really like to see the lambda capabilities implemented from scratch. At the very least, I'd like to see plans for porting it to Phoenix v3 once it's ready. A full lambda library is far more feature complete than the minimal lambda that you provide. For instance satisfies, would be better as bind(f, attr<...>(...)); For string_literal, I seem to recall that Boost.Test already has a basic_cstring. Maybe this should be factored out instead of being duplicated. I wish that the names of functions and classes linked to the doxygen reference. You claim in http://boost-log.sourceforge.net/libs/log/doc/html/log/detailed/utilities.ht... that std::type_info is required to be unique for each type. I could not find such a requirement in the standard. Please provide the reference. Also, std::type_info has no member raw_name in the standard. If it exists, it's an extension. Why is type visitor::visit not spelled operator()? The stuff for extracting the values of attributes is so complex. Why can't you just have a single interface like visit<boost::mpl::vector<int, char, double> >(attr, f); rather than providing all these complicated layers. I guess that doesn't work when the set of possible types isn't known at compile time, but I still think it should be possible to cut down the number of redundant public interfaces. I'm not convinced that make_exception_handler belongs in this library. In the todo section I noticed a reference to #3278. This was fixed a while ago. In http://boost-log.sourceforge.net/libs/log/doc/html/log/extension.html under the minimalistic sink backend, the description of consume doesn't seem to match the code: "The record, as it was stated before, contains a set of attribute values (goes as the first argument of the function) and the message string (goes as the second one)." vs. void stat_collector::consume(record_type const& rec); What happens if... A filter or formatter tries to log something. In Christ, Steven Watanabe
On 03/15/2010 08:22 AM, Steven Watanabe wrote:
I thought that the Boost.Parameter convention is to give keywords names starting with an _.
I think, some time ago there were two recommendations in the Boost.Parameter docs - the underscore and the namespace. Personally, I don't like leading underscores as it always makes me think of some implementation details.
I think that it would be a little nicer to work with attributes with an interface like this:
BOOST_LOG_ATTRIBUTE(logging::trivial::severity_level, severity); BOOST_LOG_ATTRIBUTE(unsigned int, line_id);
fmt::stream << fmt::line_id() << ": <" << fmt::severity() << "> " << fmt::message()
That way, it's easier to guarantee static type safety and is also a little less verbose when accessing the attributes.
I thought if that. It was not done for two reasons: 1. The macro would have to have a formatter in its arguments. Given that the attribute definition should be exposed rather widely, this additional dependency would be unwelcome. 2. Guys that hate macros would start complaining.
Since you allow use of Boost.Format, I don't think that the extra format argument of attr is necessary.
Boost.Format is currently invoked on strings, composed by attr formatter. Also, allowing to specify format even for streaming formatters looks like a nice feature.
I would like logging::extract< unsigned int > to return a boost::optional or something like that instead of having an output parameter.
This would require the attribute value to be copyable. Also, "extract" may accept a MPL sequence of types, in which case it would have to return something like optional< variant< types > >. This looks like an overkill. The attribute value interface offers the "get" convenience method that returns optional. But, yes, you'd have to find the attribute value first in the view.
Is it possible to declare and define a global logger separately?
No. But it seems to be a reasonable improvement. Thanks.
I think LineID may be a bad name, since when I saw it, I assumed it meant the line in the source that generated the log message, perhaps RecordID would be better?
That should be ok.
I don't like the inconsistency of logging::make_attr_ordering< unsigned int >("Line #"). I though the name was LineID?
The names may differ, the point is the same. I didn't try to force any names with my code samples. Users are free to decide which names fit them best.
I don't like the use of a time in ordering_asynchronous_sink to specify how long to wait for a new record. It just seems too fragile. Suppose that the machine is running under a heavy load or I suspend the process, or put the machine to sleep? I don't like having the correctness depend on how long things take. If nothing else, it needs to be stated that ordering_asynchronous_sink is only a "best effort" mechanism and makes no guarantees.
The time-based window has much more sense than a record count-based one, for the reason you have pointed out. If the application is actively emitting log records, the window gets bigger in order to accommodate more records that may go out of order. If records are emitted once in a while, the window is nearly empty all the time. The time measurement also gives the idea of the sink minimum latency.
fmt::xml_dec. I would prefer a more intuitive name like xml_encode or xml_escape. (Since dec is "obviously" an abbreviation of decrement)
"Dec" stands for "decorator". But xml_escape is also fine.
In http://boost-log.sourceforge.net/libs/log/doc/html/log/detailed/sink_backend...,
the documentation says that custom_level_mapping works for integral types, but the example uses std::string.
Yeah, that section needs updating. It's custom_severity_mapping, actually, and it really does support non-integral types.
Some of the code examples overflow the window.
Which ones? Also, what resolution should I target? I'm using 1280x1024, and everything looks ok.
Why are there separate classes for handling the syslog and windows event log mappings? It seems like they're basically doing the same thing.
They are implemented with the common base class. Different top-level classes are defined in order to help with type safety. The custom_severity_mapping returns syslog::level_t, and mappings for event log return their corresponding types.
Using scoped attributes to add attributes to a specific log record seems like a hack.
Why?
I don't really like to see the lambda capabilities implemented from scratch. At the very least, I'd like to see plans for porting it to Phoenix v3 once it's ready. A full lambda library is far more feature complete than the minimal lambda that you provide.
I don't know about Phoenix 3, but when it's released, I'll take a look. I considered and even tried to port on top of Proto, but dropped in the end for several reasons: * I couldn't figure out how to make Proto expressions to be function objects. * The resulting code looked heavier than my current solution. * I didn't see much advantage in this port.
For instance satisfies, would be better as bind(f, attr<...>(...));
I think, "satisfies" is more telling than the binding you suggest.
For string_literal, I seem to recall that Boost.Test already has a basic_cstring. Maybe this should be factored out instead of being duplicated.
There are several tools in the utility folder that could be extracted from the library. I once asked if there is any interest in it on this list and got no response.
I wish that the names of functions and classes linked to the doxygen reference.
Sure, if I figure out how to do it. :)
You claim in http://boost-log.sourceforge.net/libs/log/doc/html/log/detailed/utilities.ht...
that std::type_info is required to be unique for each type. I could not find such a requirement in the standard. Please provide the reference.
I guess, I was inaccurate. It's non-copyable and non-assignable.
Also, std::type_info has no member raw_name in the standard. If it exists, it's an extension.
Right.
Why is type visitor::visit not spelled operator()?
No reason in particular. This can be changed.
The stuff for extracting the values of attributes is so complex. Why can't you just have a single interface like visit<boost::mpl::vector<int, char, double> >(attr, f); rather than providing all these complicated layers. I guess that doesn't work when the set of possible types isn't known at compile time, but I still think it should be possible to cut down the number of redundant public interfaces.
What interfaces do you have in mind? Type dispatchers are the lowest level tools. Attribute values dispatching builds on top of it. Your 'visit' seems something in between the type dispatchers and 'extract'. It so happened, I didn't need such middle-placed tool and thus didn't make it.
I'm not convinced that make_exception_handler belongs in this library.
Huh? Where does it belong then? Or do you mean it could be extracted into a separate library?
In the todo section I noticed a reference to #3278. This was fixed a while ago.
Yes, but not in Boost releases the library is currently compatible with. If Boost.Log gets accepted, I will move to Boost.Xpressive, the code is there, but commented.
In http://boost-log.sourceforge.net/libs/log/doc/html/log/extension.html under the minimalistic sink backend, the description of consume doesn't seem to match the code: "The record, as it was stated before, contains a set of attribute values (goes as the first argument of the function) and the message string (goes as the second one)." vs. void stat_collector::consume(record_type const& rec);
Right, it needs updating.
What happens if...
A filter or formatter tries to log something.
Bad things may happen. The logging library should not log. Or at least, log so that no recursion occurs.
AMDG Andrey Semashev wrote:
On 03/15/2010 08:22 AM, Steven Watanabe wrote:
I thought that the Boost.Parameter convention is to give keywords names starting with an _.
I think, some time ago there were two recommendations in the Boost.Parameter docs - the underscore and the namespace. Personally, I don't like leading underscores as it always makes me think of some implementation details.
The Boost.Parameter docs seem to recommend using both together. http://www.boost.org/libs/parameter/doc/html/index.html#keyword-naming
I think that it would be a little nicer to work with attributes with an interface like this:
BOOST_LOG_ATTRIBUTE(logging::trivial::severity_level, severity); BOOST_LOG_ATTRIBUTE(unsigned int, line_id);
fmt::stream << fmt::line_id() << ": <" << fmt::severity() << "> " << fmt::message()
That way, it's easier to guarantee static type safety and is also a little less verbose when accessing the attributes.
I thought if that. It was not done for two reasons:
1. The macro would have to have a formatter in its arguments. Given that the attribute definition should be exposed rather widely, this additional dependency would be unwelcome. 2. Guys that hate macros would start complaining.
You don't seem particularly shy about using macros in other places. This is a fairly standard use of macros in Boost.
Since you allow use of Boost.Format, I don't think that the extra format argument of attr is necessary.
Boost.Format is currently invoked on strings, composed by attr formatter. Also, allowing to specify format even for streaming formatters looks like a nice feature.
My comment here was actually tied to the one before. I'm willing to drop support for automatic invocation of Boost.Format, since it can always be handled explicitly.
I would like logging::extract< unsigned int > to return a boost::optional or something like that instead of having an output parameter.
This would require the attribute value to be copyable. Also, "extract" may accept a MPL sequence of types, in which case it would have to return something like optional< variant< types > >. This looks like an overkill.
Oh, right.
I don't like the inconsistency of logging::make_attr_ordering< unsigned int >("Line #"). I though the name was LineID?
The names may differ, the point is the same. I didn't try to force any names with my code samples. Users are free to decide which names fit them best.
Given that LineID is one of the common attributes that you define, I don't see the point.
Some of the code examples overflow the window.
Which ones? Also, what resolution should I target? I'm using 1280x1024, and everything looks ok.
You should aim for about 80 columns (a little more is okay), so that it looks okay in a pdf build. I've put a compiled pdf of the documentation at http://www.cs.hmc.edu/~swatanabe/log.pdf. There's some line wrapping on pages 83, and 85, for instance.
Why are there separate classes for handling the syslog and windows event log mappings? It seems like they're basically doing the same thing.
They are implemented with the common base class. Different top-level classes are defined in order to help with type safety. The custom_severity_mapping returns syslog::level_t, and mappings for event log return their corresponding types.
Using scoped attributes to add attributes to a specific log record seems like a hack.
Why?
Because it doesn't match the intuitive idea of what you're trying to do.
I don't really like to see the lambda capabilities implemented from scratch. At the very least, I'd like to see plans for porting it to Phoenix v3 once it's ready. A full lambda library is far more feature complete than the minimal lambda that you provide.
I don't know about Phoenix 3, but when it's released, I'll take a look. I considered and even tried to port on top of Proto, but dropped in the end for several reasons:
* I couldn't figure out how to make Proto expressions to be function objects.
http://www.boost.org/doc/html/proto/users_guide.html#boost_proto.users_guide...
* The resulting code looked heavier than my current solution. * I didn't see much advantage in this port.
We have no need for yet another lambda library in Boost.
For instance satisfies, would be better as bind(f, attr<...>(...));
I think, "satisfies" is more telling than the binding you suggest.
Perhaps it is, but satisfies is another name to learn, while bind should already be well-known, and is much more flexible.
For string_literal, I seem to recall that Boost.Test already has a basic_cstring. Maybe this should be factored out instead of being duplicated.
There are several tools in the utility folder that could be extracted from the library. I once asked if there is any interest in it on this list and got no response.
I wish that the names of functions and classes linked to the doxygen reference.
Sure, if I figure out how to do it. :)
I usually use something like [def __basic_core__ [classref boost::log::basic_core basic_core]] Also, in various places where you mention the examples directory, it would be nice if you could link to the source file for the specific example that you're referring to.
The stuff for extracting the values of attributes is so complex. Why can't you just have a single interface like visit<boost::mpl::vector<int, char, double> >(attr, f); rather than providing all these complicated layers. I guess that doesn't work when the set of possible types isn't known at compile time, but I still think it should be possible to cut down the number of redundant public interfaces.
What interfaces do you have in mind? Type dispatchers are the lowest level tools. Attribute values dispatching builds on top of it. Your 'visit' seems something in between the type dispatchers and 'extract'. It so happened, I didn't need such middle-placed tool and thus didn't make it.
You have extract, attribute_value_extractor, static_type_dispatcher, and dynamic_type_dispatcher. AFAICT, attribute_value_extractor and static_type_dispatcher serve exactly the same purpose. I don't see a good reason to have both as public interfaces.
I'm not convinced that make_exception_handler belongs in this library.
Huh? Where does it belong then? Or do you mean it could be extracted into a separate library?
I don't think that it belongs anywhere. This is a general purpose utility that has no particular relation to logging and is not used by the library except in the tests. I don't think that using it is any better than struct my_exception_handler { void operator()() const { try { throw; } catch(std::exception& e) { // some code } } };
What happens if...
A filter or formatter tries to log something.
Bad things may happen. The logging library should not log. Or at least, log so that no recursion occurs.
I figured that it was a bad idea. I was just curious. In Christ, Steven Watanabe
On 03/15/2010 11:21 PM, Steven Watanabe wrote:
I think that it would be a little nicer to work with attributes with an interface like this:
BOOST_LOG_ATTRIBUTE(logging::trivial::severity_level, severity); BOOST_LOG_ATTRIBUTE(unsigned int, line_id);
fmt::stream << fmt::line_id() << ": <" << fmt::severity() << "> " << fmt::message()
That way, it's easier to guarantee static type safety and is also a little less verbose when accessing the attributes.
I thought if that. It was not done for two reasons:
1. The macro would have to have a formatter in its arguments. Given that the attribute definition should be exposed rather widely, this additional dependency would be unwelcome. 2. Guys that hate macros would start complaining.
You don't seem particularly shy about using macros in other places. This is a fairly standard use of macros in Boost.
I'd say the current usage of macros is quite moderate.
Since you allow use of Boost.Format, I don't think that the extra format argument of attr is necessary.
Boost.Format is currently invoked on strings, composed by attr formatter. Also, allowing to specify format even for streaming formatters looks like a nice feature.
My comment here was actually tied to the one before. I'm willing to drop support for automatic invocation of Boost.Format, since it can always be handled explicitly.
I don't follow, could you explain? What exact usage of Boost.Format would you like to drop and why?
I don't like the inconsistency of logging::make_attr_ordering< unsigned int >("Line #"). I though the name was LineID?
The names may differ, the point is the same. I didn't try to force any names with my code samples. Users are free to decide which names fit them best.
Given that LineID is one of the common attributes that you define, I don't see the point.
You have a point, will change the docs.
Using scoped attributes to add attributes to a specific log record seems like a hack.
Why?
Because it doesn't match the intuitive idea of what you're trying to do.
It does. Scoped attributes mark records within the scope, no matter how wide the scope is - one record or 10.
I don't really like to see the lambda capabilities implemented from scratch. At the very least, I'd like to see plans for porting it to Phoenix v3 once it's ready. A full lambda library is far more feature complete than the minimal lambda that you provide.
I don't know about Phoenix 3, but when it's released, I'll take a look. I considered and even tried to port on top of Proto, but dropped in the end for several reasons:
* I couldn't figure out how to make Proto expressions to be function objects.
http://www.boost.org/doc/html/proto/users_guide.html#boost_proto.users_guide...
I've seen that. I can't remember the details now, I just remember that I stumbled upon something while following that approach.
I wish that the names of functions and classes linked to the doxygen reference.
Sure, if I figure out how to do it. :)
I usually use something like [def __basic_core__ [classref boost::log::basic_core basic_core]] Also, in various places where you mention the examples directory, it would be nice if you could link to the source file for the specific example that you're referring to.
I was hoping there was some automated way to do it...
The stuff for extracting the values of attributes is so complex. Why can't you just have a single interface like visit<boost::mpl::vector<int, char, double> >(attr, f); rather than providing all these complicated layers. I guess that doesn't work when the set of possible types isn't known at compile time, but I still think it should be possible to cut down the number of redundant public interfaces.
What interfaces do you have in mind? Type dispatchers are the lowest level tools. Attribute values dispatching builds on top of it. Your 'visit' seems something in between the type dispatchers and 'extract'. It so happened, I didn't need such middle-placed tool and thus didn't make it.
You have extract, attribute_value_extractor, static_type_dispatcher, and dynamic_type_dispatcher. AFAICT, attribute_value_extractor and static_type_dispatcher serve exactly the same purpose. I don't see a good reason to have both as public interfaces.
Not exactly. Type dispatchers, strictly speaking, are not tied to attributes at all. They are used to perform a double dispatch between the attribute value type and the visitor. They may well be used in another area (including, outside Boost.Log). Extractors are more specific to the library and belong to a higher level. They accept the view of attribute values, and perform lookup of the attribute value. When it's found, type dispatchers are used to extract it and pass to the receiving function.
I'm not convinced that make_exception_handler belongs in this library.
Huh? Where does it belong then? Or do you mean it could be extracted into a separate library?
I don't think that it belongs anywhere. This is a general purpose utility that has no particular relation to logging and is not used by the library except in the tests. I don't think that using it is any better than
Sure you can write it yourself. But having that tool makes it easier to define simple handlers. make_exception_handler< runtime_error, exception >( var(cout) << &_1->*&exception::what);
AMDG AMDG Andrey Semashev wrote:
On 03/15/2010 11:21 PM, Steven Watanabe wrote:
BOOST_LOG_ATTRIBUTE(logging::trivial::severity_level, severity); BOOST_LOG_ATTRIBUTE(unsigned int, line_id);
I thought if that. It was not done for two reasons:
1. The macro would have to have a formatter in its arguments. Given that the attribute definition should be exposed rather widely, this additional dependency would be unwelcome.
Why is it necessary to pass a formatter to the macro?
2. Guys that hate macros would start complaining.
You don't seem particularly shy about using macros in other places. This is a fairly standard use of macros in Boost.
I'd say the current usage of macros is quite moderate.
I'm just pointing out the the arguing that this solution is bad just because it uses macros, leads nowhere.
Since you allow use of Boost.Format, I don't think that the extra format argument of attr is necessary.
Boost.Format is currently invoked on strings, composed by attr formatter. Also, allowing to specify format even for streaming formatters looks like a nice feature.
My comment here was actually tied to the one before. I'm willing to drop support for automatic invocation of Boost.Format, since it can always be handled explicitly.
I don't follow, could you explain? What exact usage of Boost.Format would you like to drop and why?
attr<unsigned int>("LineID", format="%08x"); It's not that necessarily I want to drop this, but that I don't see it as an important feature.
Using scoped attributes to add attributes to a specific log record seems like a hack.
Why?
Because it doesn't match the intuitive idea of what you're trying to do.
It does. Scoped attributes mark records within the scope, no matter how wide the scope is - one record or 10.
No it doesn't. It is true that setting an attribute for one record is a special case of setting it for a scope, but it is a long way from being the first thing I would think of. In fact, the semantics are not what I would expect when I just want to pass a parameter, according to http://boost-log.sourceforge.net/libs/log/doc/html/log/rationale/why_weak_sc...
You have extract, attribute_value_extractor, static_type_dispatcher, and dynamic_type_dispatcher. AFAICT, attribute_value_extractor and static_type_dispatcher serve exactly the same purpose. I don't see a good reason to have both as public interfaces.
Not exactly. Type dispatchers, strictly speaking, are not tied to attributes at all. They are used to perform a double dispatch between the attribute value type and the visitor. They may well be used in another area (including, outside Boost.Log).
You're not writing a general purpose dynamic dispatching library, you're writing a logging library. If I wanted such a utility I would certainly not look for it in Boost.Log.
Sure you can write it yourself. But having that tool makes it easier to define simple handlers.
make_exception_handler< runtime_error, exception >( var(cout) << &_1->*&exception::what);
var(std::cout) << bind(&boost::current_exception_diagnostic_information) In Christ, Steven Watanabe
On 03/16/2010 01:46 AM, Steven Watanabe wrote:
BOOST_LOG_ATTRIBUTE(logging::trivial::severity_level, severity); BOOST_LOG_ATTRIBUTE(unsigned int, line_id);
I thought if that. It was not done for two reasons:
1. The macro would have to have a formatter in its arguments. Given that the attribute definition should be exposed rather widely, this additional dependency would be unwelcome.
Why is it necessary to pass a formatter to the macro?
In order to support your syntax: fmt::stream << fmt::line_id() << ": <" << fmt::severity() << "> " << fmt::message()
In fact, the semantics are not what I would expect when I just want to pass a parameter, according to http://boost-log.sourceforge.net/libs/log/doc/html/log/rationale/why_weak_sc...
And what would be the expected behavior?
You have extract, attribute_value_extractor, static_type_dispatcher, and dynamic_type_dispatcher. AFAICT, attribute_value_extractor and static_type_dispatcher serve exactly the same purpose. I don't see a good reason to have both as public interfaces.
Not exactly. Type dispatchers, strictly speaking, are not tied to attributes at all. They are used to perform a double dispatch between the attribute value type and the visitor. They may well be used in another area (including, outside Boost.Log).
You're not writing a general purpose dynamic dispatching library, you're writing a logging library. If I wanted such a utility I would certainly not look for it in Boost.Log.
I'm not advertising this tool to be used outside Boost.Log, I just admit that it's not tied with it.
On Tuesday 16 March 2010 18:07:55 Andrey Semashev wrote:
On 03/16/2010 01:46 AM, Steven Watanabe wrote:
BOOST_LOG_ATTRIBUTE(logging::trivial::severity_level, severity); BOOST_LOG_ATTRIBUTE(unsigned int, line_id);
I thought if that. It was not done for two reasons:
1. The macro would have to have a formatter in its arguments. Given that the attribute definition should be exposed rather widely, this additional dependency would be unwelcome.
Why is it necessary to pass a formatter to the macro?
In order to support your syntax:
fmt::stream << fmt::line_id() << ": <" << fmt::severity() << "> " << fmt::message()
Steven, Andrey, did you arrive to any decision on the point above? Thanks, Volodya
AMDG Vladimir Prus wrote:
On Tuesday 16 March 2010 18:07:55 Andrey Semashev wrote:
On 03/16/2010 01:46 AM, Steven Watanabe wrote:
> BOOST_LOG_ATTRIBUTE(logging::trivial::severity_level, severity); > BOOST_LOG_ATTRIBUTE(unsigned int, line_id); > I thought if that. It was not done for two reasons:
1. The macro would have to have a formatter in its arguments. Given that the attribute definition should be exposed rather widely, this additional dependency would be unwelcome.
Why is it necessary to pass a formatter to the macro?
In order to support your syntax:
fmt::stream << fmt::line_id() << ": <" << fmt::severity() << "> " << fmt::message()
Steven, Andrey,
did you arrive to any decision on the point above?
I think this discussion was continued in another thread, where it also came up. In Christ, Steven Watanabe
On 03/24/2010 01:25 PM, Vladimir Prus wrote:
On Tuesday 16 March 2010 18:07:55 Andrey Semashev wrote:
On 03/16/2010 01:46 AM, Steven Watanabe wrote:
> BOOST_LOG_ATTRIBUTE(logging::trivial::severity_level, severity); > BOOST_LOG_ATTRIBUTE(unsigned int, line_id);
I thought if that. It was not done for two reasons:
1. The macro would have to have a formatter in its arguments. Given that the attribute definition should be exposed rather widely, this additional dependency would be unwelcome.
Why is it necessary to pass a formatter to the macro?
In order to support your syntax:
fmt::stream << fmt::line_id() << ":<"<< fmt::severity() << "> "<< fmt::message()
Steven, Andrey,
did you arrive to any decision on the point above?
Well, my assertion stays valid - the macro would expose formatters to all code that uses the attribute, and that is not desirable (especially in light of the expressed concerns about compilation times). There was further discussion on porting filters and formatters to Boost.Proto. I can't tell if this would alleviate the problem, but if it appears that it is possible not to introduce that dependency or the dependency is ultra-light, then I will provide something like that.
AMDG Andrey Semashev wrote:
On 03/24/2010 01:25 PM, Vladimir Prus wrote:
On Tuesday 16 March 2010 18:07:55 Andrey Semashev wrote:
On 03/16/2010 01:46 AM, Steven Watanabe wrote:
>> BOOST_LOG_ATTRIBUTE(logging::trivial::severity_level, severity); >> BOOST_LOG_ATTRIBUTE(unsigned int, line_id); > > I thought if that. It was not done for two reasons: > > 1. The macro would have to have a formatter in its arguments. Given > that the attribute definition should be exposed rather widely, this > additional dependency would be unwelcome.
Why is it necessary to pass a formatter to the macro?
In order to support your syntax:
fmt::stream << fmt::line_id() << ":<"<< fmt::severity() << "> "<< fmt::message()
Well, my assertion stays valid - the macro would expose formatters to all code that uses the attribute, and that is not desirable (especially in light of the expressed concerns about compilation times).
There was further discussion on porting filters and formatters to Boost.Proto. I can't tell if this would alleviate the problem, but if it appears that it is possible not to introduce that dependency or the dependency is ultra-light, then I will provide something like that.
Proto isn't needed. If attr returns a struct like template<class Types> struct attr_t { const char* name; }; Then the extra code needed to treat it as a formatter can be added by operator<< and operator%. These operators don't need to be defined in the same header In Christ, Steven Watanabe
participants (3)
-
Andrey Semashev -
Steven Watanabe -
Vladimir Prus