March 03, 2012
On 3/3/12 5:51 AM, Sönke Ludwig wrote:
> Am 17.02.2012 21:48, schrieb Andrei Alexandrescu:
>> On 2/17/12 12:06 PM, Jose Armando Garcia wrote:
>>> info("%s message", Severity.info);
>>
>> I think defaulting to formatted stuff may be confusing.
>>
>> Andrei
>
> Do you often log static messages? In my experience the majority of
> logging calls contains dynamic information and using a format string is
> the most convenient and readable form (compared to concatenating using
> multiple arguments).
>
> An additional infof(), warnf() etc. would be an acceptable alternative,
> although not very pretty. But if you'd always have to write
> info.format() or something similar, then this would be a real turn-off,
> at least for me.

I think it's fine to default to formatted. Also, it just occurred to me that we can adjust the formatting primitives to leave any "%"s alone if there are no matching arguments. Consider:

log.info("100% done!");

This is technically an error, but since D has static knowledge there are no variadics, it could simply output the string. That's faster, too, because no validation of the string is necessary.

Thoughts?


Andrei
March 04, 2012
On 04.03.2012 2:17, Andrei Alexandrescu wrote:
> On 3/3/12 5:51 AM, Sönke Ludwig wrote:
>> Am 17.02.2012 21:48, schrieb Andrei Alexandrescu:
>>> On 2/17/12 12:06 PM, Jose Armando Garcia wrote:
>>>> info("%s message", Severity.info);
>>>
>>> I think defaulting to formatted stuff may be confusing.
>>>
>>> Andrei
>>
>> Do you often log static messages? In my experience the majority of
>> logging calls contains dynamic information and using a format string is
>> the most convenient and readable form (compared to concatenating using
>> multiple arguments).
>>
>> An additional infof(), warnf() etc. would be an acceptable alternative,
>> although not very pretty. But if you'd always have to write
>> info.format() or something similar, then this would be a real turn-off,
>> at least for me.
>
> I think it's fine to default to formatted. Also, it just occurred to me
> that we can adjust the formatting primitives to leave any "%"s alone if
> there are no matching arguments. Consider:
>
> log.info("100% done!");
>
> This is technically an error, but since D has static knowledge there are
> no variadics, it could simply output the string. That's faster, too,
> because no validation of the string is necessary.
>
> Thoughts?

I would say just separate them please. Let it work as writeln/writefln and nobody is hurt.

With all manner of magic rules the innocent will catch a bullet sooner or latter. And it's always happens kind of point-blank, when the code is shipped & forgotten.
Turning on imagination, e.g. for rarely used stuff:
log.warn("100%done for task %s, yet the client timed out", taskName);


-- 
Dmitry Olshansky
March 04, 2012
On 3/4/12 3:17 AM, Dmitry Olshansky wrote:
> I would say just separate them please. Let it work as writeln/writefln
> and nobody is hurt.
>
> With all manner of magic rules the innocent will catch a bullet sooner
> or latter. And it's always happens kind of point-blank, when the code is
> shipped & forgotten.
> Turning on imagination, e.g. for rarely used stuff:
> log.warn("100%done for task %s, yet the client timed out", taskName);

Yah, agreed. Probably calls with no extra arguments should be safe.

Andrei


March 05, 2012
On Mon, 13 Feb 2012 10:50:04 -0500, David Nadlinger <see@klickverbot.at> wrote:

> There are several modules in the review queue right now, and to get things going, I have volunteered to manage the review of Jose's std.log proposal. Barring any objections, the review period starts now and ends in three weeks, on March 6th, followed by a week of voting.


Some notes:

I dislike that logging affects function execution.  In particular, I don't think the logging library should have any business throwing exceptions or errors.  It should be invisible to the application.  The equivalent function can be had by giving a wrapper function (i.e. log this message at the fatal level, and then throw an error).  A use case I can see is printing several fatal log messages before exiting.

The log aliases use names that are too common.  I think log.info is a better symbol for logging than just 'info', which could be a symbol in a myriad of places.  Given that D's symbol lookup rules allow shadowing of global symbols, this does not work out very well.

Like others have stated, I think vlog is a) confusing, and b) unnecessary.  Even reading the docs, I can't understand what it's used for, and why it has such different syntax than the normal logging stuff.

I really like the every function, that's a great idea, one that I've manually implemented (at least the once every N times) many times.

Do we have to make the logger a singleton?  I'd like to see cases where I can have different log instances.  For example, an instance I can enable/disable per class type, or an instance that logs to a diffferent backend.  Or a non-shared instance which does not need to handle threading issues (i.e. a per-thread file log).  Does this help with the vlog issue?

-Steve
March 05, 2012
On Monday, 5 March 2012 at 21:55:08 UTC, Steven Schveighoffer wrote:
> The log aliases use names that are too common.  I think log.info is a better symbol for logging than just 'info', which could be a symbol in a myriad of places.  Given that D's symbol lookup rules allow shadowing of global symbols, this does not work out very well.

Originally, the code used log!info and so on, but it was changed to the current design right after review begin, the rationale being that you could always use »import log = std.log« if you want the extra namespace.

> Like others have stated, I think vlog is a) confusing, and b) unnecessary.  Even reading the docs, I can't understand what it's used for, and why it has such different syntax than the normal logging stuff.

I think this been modelled after glog's verbose logging support [1], just like much of the rest of the design (by the way, I think a note about this should added somewhere in the module docs). Does the feature as described in the glog docs make sense to you?

> I really like the every function, that's a great idea, one that I've manually implemented (at least the once every N times) many times.

I love it too, a similar design served me really well in some larger *shudder* ActionScript projects.

David


[1] http://google-glog.googlecode.com/svn/trunk/doc/glog.html#verbose
March 05, 2012
On Mon, 05 Mar 2012 18:30:03 -0500, David Nadlinger <see@klickverbot.at> wrote:

> On Monday, 5 March 2012 at 21:55:08 UTC, Steven Schveighoffer wrote:
>> The log aliases use names that are too common.  I think log.info is a better symbol for logging than just 'info', which could be a symbol in a myriad of places.  Given that D's symbol lookup rules allow shadowing of global symbols, this does not work out very well.
>
> Originally, the code used log!info and so on, but it was changed to the current design right after review begin, the rationale being that you could always use »import log = std.log« if you want the extra namespace.

That doesn't help.  Software isn't static.

import std.log;
import other; // defines B

class A : B
{
   void foo()
   {
      info("some info message"); // error! int isn't a function!
   }
}

other.d:

class B
{
   int info; // added later
}

>
>> Like others have stated, I think vlog is a) confusing, and b) unnecessary.  Even reading the docs, I can't understand what it's used for, and why it has such different syntax than the normal logging stuff.
>
> I think this been modelled after glog's verbose logging support [1], just like much of the rest of the design (by the way, I think a note about this should added somewhere in the module docs). Does the feature as described in the glog docs make sense to you?

It's good to know the root of where this comes from.  The docs in glog do make more sense than the vlog docs.

This may be a doc issue.  I'll have to think about it some more.

-Steve
March 06, 2012
On Monday, 5 March 2012 at 23:51:29 UTC, Steven Schveighoffer wrote:
> On Mon, 05 Mar 2012 18:30:03 -0500, David Nadlinger <see@klickverbot.at> wrote:
>
>> On Monday, 5 March 2012 at 21:55:08 UTC, Steven Schveighoffer wrote:
>>> The log aliases use names that are too common.  I think log.info is a better symbol for logging than just 'info', which could be a symbol in a myriad of places.  Given that D's symbol lookup rules allow shadowing of global symbols, this does not work out very well.
>>
>> Originally, the code used log!info and so on, but it was changed to the current design right after review begin, the rationale being that you could always use »import log = std.log« if you want the extra namespace.
>
> That doesn't help.  Software isn't static.
>
> import std.log;
> import other; // defines B
>
> class A : B
> {
>    void foo()
>    {
>       info("some info message"); // error! int isn't a function!
>    }
> }
>
> other.d:
>
> class B
> {
>    int info; // added later
> }

That is not a counter-argument to something related to this library but everything that lies in global namespace.
At its first state both severity levels and the "log" was in global namespace. Now only severity levels.

You are also overlooking one crucial fact that this library will be part of phobos, standard library. Which requires everyone to adopt. When you see codes like this (below), you don't blame standard library designers do you?

using namespace std;
int cout;

March 06, 2012
On Mon, 05 Mar 2012 20:22:05 -0500, so <so@so.so> wrote:

> On Monday, 5 March 2012 at 23:51:29 UTC, Steven Schveighoffer wrote:
>> On Mon, 05 Mar 2012 18:30:03 -0500, David Nadlinger <see@klickverbot.at> wrote:
>>
>>> On Monday, 5 March 2012 at 21:55:08 UTC, Steven Schveighoffer wrote:
>>>> The log aliases use names that are too common.  I think log.info is a better symbol for logging than just 'info', which could be a symbol in a myriad of places.  Given that D's symbol lookup rules allow shadowing of global symbols, this does not work out very well.
>>>
>>> Originally, the code used log!info and so on, but it was changed to the current design right after review begin, the rationale being that you could always use »import log = std.log« if you want the extra namespace.
>>
>> That doesn't help.  Software isn't static.
>>
>> import std.log;
>> import other; // defines B
>>
>> class A : B
>> {
>>    void foo()
>>    {
>>       info("some info message"); // error! int isn't a function!
>>    }
>> }
>>
>> other.d:
>>
>> class B
>> {
>>    int info; // added later
>> }
>
> That is not a counter-argument to something related to this library but everything that lies in global namespace.
> At its first state both severity levels and the "log" was in global namespace. Now only severity levels.
>
> You are also overlooking one crucial fact that this library will be part of phobos, standard library. Which requires everyone to adopt. When you see codes like this (below), you don't blame standard library designers do you?
>
> using namespace std;
> int cout;

Except 'info', 'error', 'warning' are all common names, likely to be a very attractive name for something that has nothing to do with (or cares about) logging.  cout is not a common name or even an english word, so it's unlikely someone has or wants to create a cout member.

Couple this with the fact that all of these are nouns -- likely candidates for fields.

Your argument has some merit, but I would add that my argument is only against *common* global namespace names.

Another solution besides using a namespace is to make the names less common, like linfo instead of just info.

-Steve
March 06, 2012
On Tuesday, March 06, 2012 02:22:05 so wrote:
> That is not a counter-argument to something related to this
> library but everything that lies in global namespace.
> At its first state both severity levels and the "log" was in
> global namespace. Now only severity levels.
> 
> You are also overlooking one crucial fact that this library will be part of phobos, standard library. Which requires everyone to adopt. When you see codes like this (below), you don't blame standard library designers do you?
> 
> using namespace std;
> int cout;

Except that cout is not exactly something that would be considered a normal variable name. Something like info _is_. This logging module is taking incredibly common names and shoving them as far into the global namespace as anything can go in D which isn't a compiler built-in. _Not_ a good idea IMHO - not without good reason. And I really don't think that this merits it. log!info(msg) would work just fine and would be _far_ better.

- Jonathan M Davis
March 06, 2012
On Tuesday, 6 March 2012 at 01:30:41 UTC, Steven Schveighoffer wrote:

> Except 'info', 'error', 'warning' are all common names, likely to be a very attractive name for something that has nothing to do with (or cares about) logging.  cout is not a common name or even an english word, so it's unlikely someone has or wants to create a cout member.
>
> Couple this with the fact that all of these are nouns -- likely candidates for fields.
>
> Your argument has some merit, but I would add that my argument is only against *common* global namespace names.
>
> Another solution besides using a namespace is to make the names less common, like linfo instead of just info.

I have no objections against changing names. For example, instead of "info" i use "note" for my logger. Not 100% sure about "error" but i think "warning" also implies logging and don't see any use case where it would be used as a variable name.