Theo Van Dinter wrote:
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.
Using lint_warn() and return $INVALID_VALUE (or $MISSING_REQUIRED_VALUE)
like the existing code did results in double the number of actual lint
errors.
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.
I guess if we want the warnings to go to STDERR (or the log facility
when daemonized) even when NOT linting, the value returned could be
decided upon based on whether we're linting or not.
Or we can drop the lint_warn()s and go with the generic error messages.
Daryl
