Re: Proper loglevels: cmd-line option --loglevel=NONE/ERROR/WARN/PROGRESS/INFO/DEBUG (issue4822055)

[k-ohara5a5a]

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/