[log4perl-devel] Level::is_valid() vs Logger::level()

[Jonathan M. <jma...@al...>]

Hello,

It seems that a textual log level ('INFO' vs $INFO) is accepted by 
Log::Log4Perl::Level::is_valid().

So, if I call: $log->level('INFO');
The string 'INFO' passes is_valid(), and becomes that logger's level.
But, level() doesn't do any conversion back the the numerical form, so 
what follows is lots of non-numeric comparison with <= errors.

I know you're *supposed* to import the levels and call 
$log->level($INFO) instead, but it seems like a flaw for the string form 
to be accepted by is_valid, but not work as a logging level.
Personally, I would prefer it if $log->level() just accepted the text 
string, and did the conversion to the numeric form for me.

I do see in the CVS log that this was a recent change to is_valid, so 
maybe it's there for a reason and these changes are already planned.
Perhaps this is too unlike Log4J, so that makes it a design decision on 
whether to accept the text string or not - completely your call.

Either way, a quick patch is provided below. It passes 'make test', 
though no test specifically tests the string forms of the levels - I'll 
go through the trouble to do this if this patch is approved.

I'd be interested to hear any suggestions/counter-arguments.

~Jonathan



--- Logger.pm.orig      Sun Jul 20 15:34:08 2003
+++ Logger.pm   Thu Oct  2 17:11:06 2003
@@ -362,7 +362,11 @@
      if(defined $level) {
          croak "invalid level '$level'"
                  unless Log::Log4perl::Level::is_valid($level);
-        $self->{level} = $level;
+       if($level =~ /[A-Z]/) {
+         $self->{level} = ${Log::Log4perl::Level::PRIORITY{$level}};
+       } else {
+         $self->{level} = $level;
+       }

          &reset_all_output_methods
              unless $dont_reset_all;  #keep us from getting overworked