March 03, 2012 Re: Review of Jose Armando Garcia Sancio's std.log | |
|---|---|
Posted in reply to Sönke Ludwig | 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 Re: Review of Jose Armando Garcia Sancio's std.log | |
|---|---|
Posted in reply to Andrei Alexandrescu | 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 Re: Review of Jose Armando Garcia Sancio's std.log | |
|---|---|
Posted in reply to Dmitry Olshansky | 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 Re: Review of Jose Armando Garcia Sancio's std.log | |
|---|---|
Posted in reply to David Nadlinger | 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 Re: Review of Jose Armando Garcia Sancio's std.log | |
|---|---|
Posted in reply to Steven Schveighoffer | 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 Re: Review of Jose Armando Garcia Sancio's std.log | |
|---|---|
Posted in reply to David Nadlinger | 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 Re: Review of Jose Armando Garcia Sancio's std.log | |
|---|---|
Posted in reply to Steven Schveighoffer | 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 Re: Review of Jose Armando Garcia Sancio's std.log | |
|---|---|
Posted in reply to so | 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 Re: Review of Jose Armando Garcia Sancio's std.log | |
|---|---|
Posted in reply to so | 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 Re: Review of Jose Armando Garcia Sancio's std.log | |
|---|---|
Posted in reply to Steven Schveighoffer | 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.
|

Reply