March 07, 2012
On Wed, 07 Mar 2012 02:33:05 -0500, Dmitry Olshansky <dmitry.olsh@gmail.com> wrote:

> Exception is a graceful shutdown, as it calls destructors & finally blocks while unrolling the stack.

You're assuming the program uses finally/scope exit blocks to do shutdown logic.  This is not always the case.  A library shouldn't force certain development styles.

-Steve
March 07, 2012
On Tue, 06 Mar 2012 14:27:17 -0500, Jose Armando Garcia <jsancio@gmail.com> wrote:

> On Mon, Mar 5, 2012 at 5:32 PM, Jonathan M Davis <jmdavisProg@gmx.com> wrote:
>> 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.
>
> using namesapce std;
>
> matrix vector = new Matrix(...)
>
> The variable vector conflicts with std::vector. Honestly, I can sit
> here and come up with 10s or 100s of example where you want to use a
> symbol that is expose by another module. You don't have to go far just
> look at druntime and phobos. This the exact reason why modules and
> namespace exist and one of the reason why people hate C's preprocessor
> macros. D solved this problem years ago.

The not so trivial and important difference is here:

using namespace std;

That is, the default is, vector is *NOT* imported into your namespace.

For D modules, it is.

-Steve
March 07, 2012
On Wed, 07 Mar 2012 07:09:17 -0500, Steven Schveighoffer <schveiguy@yahoo.com> wrote:

> On Tue, 06 Mar 2012 14:39:28 -0500, Jose Armando Garcia <jsancio@gmail.com> wrote:
>
>> On Tue, Mar 6, 2012 at 12:25 AM, Jonathan M Davis <jmdavisProg@gmx.com> wrote:
>>> On Tuesday, March 06, 2012 09:14:16 so wrote:
>>>> On Tuesday, 6 March 2012 at 07:46:14 UTC, Jacob Carlborg wrote:
>>>> > On 2012-03-06 02:32, Jonathan M Davis wrote:
>>>> >
>>>> > The user can then alias "log!info" to "info" if he/she wants to.
>>>>
>>>> Again, you are now forcing 2 common names instead of one as it is
>>>> now.
>>>> When you instantiate log!info where do you get "info" from?
>>>
>>> Yes. My mistake - probably because the time stuff typicall takes such a
>>> template argument as string, which would make this log!"info"(msg). However,
>>> adding _log_ isn't necessarily bad, given that this is std.log that we're
>>> talking about. It's info and the rest that are the problem.
>>>
>> Seriously everyone. What are we spending some much effort on this?
>
> Because naming is important.  It's very difficult to change names once something is released.  I seriously could care less about implementation (didn't even look at it).  That can be fixed.  Naming can't.
>
>> What is wrong with:
>>
>> import log = std.log;
>> log.info("cool");
>
> What is wrong with
>
> import std.log;
> log.info("cool");

alternatively:

log_info("cool");
linfo("cool");
lginfo("cool");

There are so many choices besides just "info."  We should use something else.

-Steve
March 07, 2012
On Tue, 06 Mar 2012 14:19:27 -0500, Jose Armando Garcia <jsancio@gmail.com> wrote:

> On Mon, Mar 5, 2012 at 1:55 PM, Steven Schveighoffer
> <schveiguy@yahoo.com> wrote:
>> 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.
>>
> Then don't use std.log.fatal. It is not like you are forced to use it.
> You can implement the above by using std.log.error

Then I can't access the fatal level.

When I used Log4Net to log application errors, I only logged fatal errors to the event log (using an event log backend).  In fact, in that case, I was *catching* uncaught exceptions.  There was no need to throw another exception at that point.  My point is, whether to throw an exception or not should be up to the application, and having a fatal level can be utilized in other ways than "this is just like error but throws an exception".

Again, the logging library should not be in the business of dictating application design.

If fatal and critical logged an error at the "Error" logging level, and threw an appropriate exception, that would be a different story, because then it's just a convenience function.  But you have made two levels of logging unavailable without throwing exceptions.

As I brought up in another part of this thread, I envision the following pattern emerging:

try
{
   fatal("connection aborted!");
}
catch(LoggingError)
{
}

... // graceful shutdown of rest of the application
throw new LoggingError("connection aborted!");

>> 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.
>>
> This is a tough one. Should we be relying on D's module abstraction.
> It is not scalable as a module designer and implementer to think about
> other modules. This is why a lot of programming languages implement
> the concept of namespaces.

The problem is that by default D pulls in a module's symbols into the current scope.  You have to go out of your way to *avoid* this.  By default you should be able to just import std.log and not have your local symbols shadow std.log's.

There are many solutions to this, as I have brought up elsewhere.

> import log = std.log;
> log.info("hello world");

I like this better:

import std.log;
// alias log.info info // if you desire
log.info("hello world");

>> 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 have tried to explain this before but it looks like I have failed. I
> find it useful. If you are interested on a different explaination:
> http://google-glog.googlecode.com/svn/trunk/doc/glog.html

Someone else pointed that out.  I think the documentation explanation there is much more complete than your version, and I think vlog is fine, it just needs a lot more documentation.

>> 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?
>>
> My point of view here is that as a developer I never know how I want
> to categorize my log during development. Some people use class name as
> a hack for doing this. What about functional programs that don't use
> class/objects? What about logical component/classes that span multiple
> classes? I always found class based grouping for logging as a hack. D
> made the observation that classes are not always the best abstraction
> unit so it introduced modules. std.log filters based on modules
> (actually source files to be exact but if D had __MODULE__, std.log
> would use that instead.)

I wasn't speaking so much about filtering logging based on classes (and BTW, most logging libraries use hierarchical symbols to denote logging instances, they don't necessarily have to follow class names), but simply being able to have multiple log instances.  That is, instead of having one global instance of log, what about multiple instances (I don't care if they are named or not).  They don't have to be based on some sort of name, just another place you can configure independently of the rest of the application.  For example, I may want a log instance in my library that logs to some library log file independent of the full application log.

-Steve
March 07, 2012
On Tue, 06 Mar 2012 21:22:21 -0600, Andrei Alexandrescu <SeeWebsiteForEmail@erdani.org> wrote:

> On 3/6/12 6:05 PM, Geoffrey Biggs wrote:
>> That approach means that if I actually do have a fatal error, I can't
>> mark it as such.
>
> Use log.fatal for those.
>
> Andrei

But fatal "Logs a fatal severity message. Fatal log messages terminate the application after the message is persisted. Fatal log message cannot be disable at compile time or at run time." The point is that he want to log a fatal message and then terminate in a custom manner. I don't see a problem with convince functions that log and error and then throw, but not having the option to not throw is an unnecessary limitation.
March 07, 2012
On 3/6/12 5:39 PM, H. S. Teoh wrote:
> I don't like the current state of dlang.org docs either. There is little
> or no intro paragraph to explain what on earth the module is used for or
> why you should bother reading the rest of the page for the next hour or
> so. It also doesn't give any motivating examples (I'm thinking of
> std.range here) why this module is useful.
>
> I think a good intro is a must to good documentation. Include some code
> snippets to show typical usage of the module. How to change common
> settings. Some explanation of why the user might find the module
> helpful. It's OK to duplicate some docs here, within reason. It should
> also be concise without being uninformative.
>
> For example (from std.range):
>
> 	This module defines the notion of range (by the membership tests
> 	isInputRange, isForwardRange, isBidirectionalRange,
> 	isRandomAccessRange), range capability tests (such as hasLength
> 	or hasSlicing), and a few useful range incarnations.
>
> is concise, but not very informative. Why should the user care what a
> range is anyway? No explanation is given. Something like this may be a
> beginning to better documentation:
>
> 	This module defines the notion of a range. Ranges generalize the
> 	concept of arrays, lists, or anything that involves sequential
> 	access. This abstraction enables the same set of algorithms (see
> 	std.algorithm) to be used with a vast variety of different
> 	concrete types. For example, a linear search algorithm such as
> 	std.find works not just for arrays, but for linked-lists, input
> 	files, incoming network data, etc..
>
> 	This module defines several templates (<!--insert list
> 	here-->)for testing whether a given object is a range, and what
> 	kind of range it is.
>
> 	It also lets you construct new ranges out of existing ranges.
> 	For example, retro lets you access a bidirectional range in
> 	reverse, cycle creates a range that is an endless repetition of
> 	the original range. ...
>
> 	...
>
> Basically, you're writing an overview to the module, so highlight its
> main components, give some insight into why it's useful, etc., so that
> the user can make sense of the long list of declarations that follow.
>
> As it stands, std.range's page consists of a giant list of range-related
> declarations that gives no hint to the user as to how they all fit
> together. You basically have to wade through it until it somehow all
> "clicks" together. That is poor documentation. The overview should give
> some general categories of stuff that's found in the module (e.g. range
> tests, constructing new ranges, etc., as I've tried to do above in my
> one-shot attempt to improve std.range's docs). Include some examples of
> really clever stuff that you can do with the help of the module. Such
> examples are usually a very good way to get the user up-to-speed with
> what the module has to offer.

I strongly agree, particularly in the case of ranges which are not a familiar concept to many, but also with the general principle.
March 07, 2012
On 07.03.2012 16:34, Steven Schveighoffer wrote:
> On Wed, 07 Mar 2012 02:33:05 -0500, Dmitry Olshansky
> <dmitry.olsh@gmail.com> wrote:
>
>> Exception is a graceful shutdown, as it calls destructors & finally
>> blocks while unrolling the stack.
>
> You're assuming the program uses finally/scope exit blocks to do
> shutdown logic. This is not always the case. A library shouldn't force
> certain development styles.
>

I do and within the reason. Doing graceful shutdown logic in scope/finally/destructors or what the heck the top-most catch(Exception) is not just a good practice. Otherwise an unexpected exception on the way up leaves debris and destruction behind, and the whole point of graceful shutdown is lost.
Yup, one can provide an alternative way of shutdown, yet one still has to think of unexpected exceptions.


-- 
Dmitry Olshansky
March 07, 2012
"Richard van Scheijen" <dlang@mesadu.net> wrote:
> When logging the severity level should convey a certain insight that the developer has about the code. This can be done with a 3 bit field. These are: known-cause, known-effect and breaks-flow.
> 
> This creates the following matrix:
> 
> KC KE BF Severity
> =================
> 1  1  0  Trace
> 0  1  0  Info
> 1  0  0  Notice
> 0  0  0  Warning
> 1  1  1  Error
> 0  1  1  Critical
> 1  0  1  Severe
> 0  0  1  Fatal
> 

Minor nit: if you rearrange it as

AllowFlow / KnownEffect / KnownCause

Then the description will correspond exactly to their integer values.

111 (7) = Trace
110 (6) = Info
101 (5) = Notice
100 (4) = Warning
011 (3) = Error
010 (2) = Critical
001 (1) = Severe
000 (0) = Fatal

> A known cause is when the developer knows why a log event is made. e.g.:
> if you cannot open a file, you do not know why.
> A known effect is when he/she knows what happens after. Basically, you
> can tell if it is a catch-all by this flag.
> 
> When a severity should only be handled by a debugger, the normal debug statement should be used. This is in essence a 4th bit.
> 
> I hope this helpful in the search for a good level system.
March 07, 2012
On Monday, 13 February 2012 at 15:50:05 UTC, David Nadlinger wrote:
> Barring any objections, the review period starts now and ends in three weeks, on March 6th, followed by a week of voting.

Okay, everyone, by the original schedule, the review period would have ended yesterday. However, I don't think it makes sense to start a vote right now, where several points are actively discussed.

After discussing the situation with Jose, I think the best option is to extend the review period for another few days, until next Monday. Jose will try to answer any open comments so everybody can get a good idea of what the library does and what others expect it to do until the vote starts next week.

Thanks for the comments and review so far,
David
March 07, 2012
On Wed, Mar 7, 2012 at 1:53 PM, David Nadlinger <see@klickverbot.at> wrote:
> On Monday, 13 February 2012 at 15:50:05 UTC, David Nadlinger wrote:
>>
>> Barring any objections, the review period starts now and ends in three weeks, on March 6th, followed by a week of voting.
>
>
> Okay, everyone, by the original schedule, the review period would have ended yesterday. However, I don't think it makes sense to start a vote right now, where several points are actively discussed.
>
> After discussing the situation with Jose, I think the best option is to extend the review period for another few days, until next Monday. Jose will try to answer any open comments so everybody can get a good idea of what the library does and what others expect it to do until the vote starts next week.
>

Thanks David! I will have to some time later today to answer some of the open questions.

> Thanks for the comments and review so far,
> David