[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Proper loglevels: cmd-line option --loglevel=NONE/ERROR/WARN/PROGRES
From: |
k-ohara5a5a |
Subject: |
Re: Proper loglevels: cmd-line option --loglevel=NONE/ERROR/WARN/PROGRESS/INFO/DEBUG (issue4822055) |
Date: |
Mon, 08 Aug 2011 06:52:46 +0000 |
I like it.
http://codereview.appspot.com/4822055/diff/6003/flower/include/warn.hh
File flower/include/warn.hh (right):
http://codereview.appspot.com/4822055/diff/6003/flower/include/warn.hh#newcode41
flower/include/warn.hh:41: // There is no separation between progress
and other info messages:
but define LOGLEVEL_INFO with an | LOG_INFO anyway,
just in case somebody tries to send a message with LOG_INFO
http://codereview.appspot.com/4822055/diff/6003/flower/warn.cc
File flower/warn.cc (right):
http://codereview.appspot.com/4822055/diff/6003/flower/warn.cc#newcode54
flower/warn.cc:54: debug_output (_f ("Log level set to `%d'\n",
loglevel));
Quotes around the number, `271', confused me
http://codereview.appspot.com/4822055/diff/6003/flower/warn.cc#newcode132
flower/warn.cc:132: /* Display a fatal error message. Also exits
lilypond. */
Can you now remove the function non_fatal_error() in favor of error() ?
It looks strange to use non_fatal_error to dipaly a fatal error message.
http://codereview.appspot.com/4822055/diff/6003/flower/warn.cc#newcode180
flower/warn.cc:180: // Use the progress loglevel for all normal messages
(including progress msg)
Confusing. Input::message() filters with LOG_INFO; shouldn't message()
do the same? (at least until the duplicate code is eventually merged)
http://codereview.appspot.com/4822055/diff/6003/lily/input.cc
File lily/input.cc (right):
http://codereview.appspot.com/4822055/diff/6003/lily/input.cc#newcode130
lily/input.cc:130: print_message (LOG_INFO, s);
has no effect, unless LOG_INFO is included in one of the bitmasks.
http://codereview.appspot.com/4822055/diff/6003/lily/main.cc
File lily/main.cc (right):
http://codereview.appspot.com/4822055/diff/6003/lily/main.cc#newcode172
lily/main.cc:172: "NONE, ERROR, WARNING, PROGRESS (default), DEBUG and
FULLDEBUG.")
Most of the new lines are too long for an 80-character terminal.
You wanted to remove FULLDEBUG.
http://codereview.appspot.com/4822055/