On Tue, Feb 14, 2006 at 09:19:07PM -0000, [EMAIL PROTECTED] wrote:
> +      unless (defined $value && $value !~ /^$/ &&
> +             (scalar @scores == 1 || scalar @scores == 4)) {
> +     $self->{parser}->lint_warn("config: score configuration option " .
> +       "requires a symbolic rule name and 1 or 4 scores, skipping: $line",
> +       $rule);
> +     return;
> +      }
>  
> +     unless (/^-?\d+(?:\.\d+)?$/) {
> +       $self->{parser}->lint_warn("config: the non-numeric score ($_) is ".
> +         "invalid, numeric score required, skipping: $line", $rule);
> +       return;
> +     }
>
> -        return $INVALID_VALUE;
> +        return;

I'm leaning towards a -1 on this patch.  You removed the $INVALID_VALUE
bits and just use a return in the new code.  Returning $INVALID_VALUE and
$MISSING_REQUIRED_VALUE is what flags that there's a problem in the code
that calls this function (Conf::Parser).  Returning undef specifically
removes the fact that there was an error -- it's the default return code.

What we typically do for conf is use lint_warn() only for information we
want to give out if the user is running --lint (there's very little that
uses this).  Otherwise, if we want a more friendly error message, specify
info("message") and return either INVALID_VALUE or MISSING_REQUIRED_VALUE.
If we don't want to specify a message, just return INVALID_VALUE or
MISSING_REQUIRED_VALUE and let the default error message indicate
the issue.

-- 
Randomly Generated Tagline:
 Bender: This is the Brooklyn-bound B train making local stops at wherever 
  the hell I feel like, watch for the closing doors.

Attachment: pgp3yNrg6tZkl.pgp
Description: PGP signature

Reply via email to