Review: std.logger

[Robert burner Schadek via Digitalmars-d]

On Saturday, 12 July 2014 at 16:13:22 UTC, Sönke Ludwig wrote:
> Overall looks good to me. Some points that haven't been 
> mentioned so far in this review round:
>
>  - Using a class with static members doesn't seem to be very 
> idiomatic. It seems like the three member properties can simply 
> be made global and everything should be fine - the 
> "LogManager." prefix doesn't really add information. This has 
> been mentioned in the last review round and it's not a very 
> important point in this particular instance, but we should 
> really make a decision here that will also decide how future 
> modules go about this.
>
>  - Personally, I really find additional verbose log levels 
> useful. Currently there is only "trace". Previous proposals had 
> a generic "verbose(N)" set of levels, but I think it's 
> important for the proper interaction of multiple libraries to 
> define a semantic set of verbose levels. A proposal, which has 
> worked very well me (from low to high):
>
>   trace:
>      as now, used for low level tracing of program flow
>   debugv:
>     verbose debug messages, may flood the log
>   debug:
>     normal, low frequency debug messages, useful for the 
> developer
>   diagnostic:
>     diagnostic output also potentially useful for the user
>     this is what you typically would get with the -v command 
> line switch

the LogLevel enum has quite a lot of free number in between trace 
and info and so forth. In combination with a MultiLogger it is 
very easy to build verbose logging special to your needs.

>
>  - There is a "formatString" string mixin that includes a lot 
> of if-else cases that seem to do exactly what formattedWrite 
> would do anyway, is there something that it actually does in 
> addition to that? Also, why is it a string mixin in contrast to 
> a simple (template) function? It may be a matter of taste (but 
> also of compile time/memory), but I'd almost always prefer a 
> non-string-mixin solution.
>
>  - Even if it may be more typing (or maybe not?), actually 
> writing out the different signatures for each log level and the 
> c/f suffixes would be very advantageous for the documentation 
> and for code completion. It would also make the EBNF 
> unnecessary, which I agree with Johannes looks a little scary. 
> All of the functions could be based on the generic logl/loglf 
> functions, so that there wouldn't be much redundancy.

I'm about to change the codegen.

>
>  - I'm still really not sure if the "c" versions of the log 
> functions pull their own weight. They seem to be in line with 
> trivial convenience functions, which are generally discouraged 
> in Phobos (with the same issues, such as additional cognitive 
> load and bigger API size).

Anyone else?

>
>  - The functions error(), info(), fatal(), etc. don't follow 
> the usual rule for functions to start with a verb. The question 
> is if saving three characters over logError() is worth making 
> the code more ambiguous for the outside reader (e.g. "does 
> error() throw an exception? or set some internal error state?" 
> "does fatal() terminate the process?")

if I change it back, people will argue that that is redundant and 
unintuitive. Than I will change it back again and the discussion 
starts again.