On 2014-03-16 at 21:35 +0000, Jeremy Harris wrote:
> As the docs say, it should (and they should evaluate to true).
>
> Easily fixable, but a comment in the code notes that the "bool"
> conditional should be kept in step - and the docs for *that*
> require that a negative number is an expansion failure:
>
> > It parses “true”, “false”, “yes” and “no” (case-insensitively); also >
> positive integer numbers map to true if non-zero, false if zero. An
> > empty string is treated as false. Leading and trailing whitespace is >
> ignored; thus a string consisting only of whitespace is false. All
> > other string values will result in expansion failure.
>
>
> The options seem to be
> 1) Don't fix 1005; change the definition of "condition =" and
> change the docs.
> 2) Fix 1005, and live with the mismatch in definitions of condiotonals
> 3) Fix both 1005 and "bool{<string>}" and change the docs of the latter.
>
> Opinions?
I wrote the bool conditional and it was designed to match the ACL
condition (as opposed to bool_lax, to match the router condition); the
comments merely reflect the API, designed to match the existing code in
the ACL condition logic.
Since currently negative numbers are a non-forced expansion failure,
nobody should be relying upon the current logic. Also, as noted, the
documentation says something else.
So: option 3, change ACL condition, keep bool{} in sync, fix the comment
in bool{} and also then change the documentation for bool{}, which is
currently correct for the implementation.
-Phil
