Mailing List Archive

------- You are receiving this mail because: -------
You are the QA contact for the bug.

http://bugs.exim.org/show_bug.cgi?id=167

Brad "anomie" Jorsch <anomie@users.sourceforge.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |anomie@users.sourceforge.net




--- Comment #13 from Brad "anomie" Jorsch <anomie@users.sourceforge.net> 2008-09-13 15:29:38 ---
I notice that the "condition" ACL condition accepts numbers as boolean values,
while your patch doesn't.


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

------- You are receiving this mail because: -------
You are the QA contact for the bug.

http://bugs.exim.org/show_bug.cgi?id=167

Phil Pennock <exim-dev@spodhuis.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |exim-dev@spodhuis.org




--- Comment #14 from Phil Pennock <exim-dev@spodhuis.org> 2008-09-14 00:03:48 ---
(In reply to comment #13)
> I notice that the "condition" ACL condition accepts numbers as boolean values,
> while your patch doesn't.

Yeah, I opted to accept the values which readconf interprets as bools, then in
the comment wrote "condition =" (and didn't specify ACL vs Router).

Okay, ACLs explicitly check for no/false/yes/true or if the value consists
entirely of digits and if so, if non-zero.

Routers use expand_check_condition(), which checks for the empty string, or
"0", "no", "false"; those return FALSE, all other values are TRUE (and forced
failures or search failures are also FALSE). This matches the documentation.
Note then that "0000" is FALSE in an ACL condition but TRUE in a Router
condition.

So I think that the cleanest solution is to clarify in the comment that it's
the ACL condition rules which apply and to accept numbers (which I had actually
thought about, but decided against, doh).

The way the code is written, abstracting this into a common function would add
a complicated API and I think that this is simple enough that logic replication
is reasonable. I'll add cross-reference comments to both places, though.

Patch forthcoming.


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

------- You are receiving this mail because: -------
You are the QA contact for the bug.

http://bugs.exim.org/show_bug.cgi?id=167

Phil Pennock <exim-dev@spodhuis.org> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #271 is|0 |1
obsolete| |




--- Comment #15 from Phil Pennock <exim-dev@spodhuis.org> 2008-09-14 00:24:23 ---
Created an attachment (id=272)
--> (http://bugs.exim.org/attachment.cgi?id=272)
Adds "bool" expansion condition (v2)

Parses numbers. Has cross-reference comments for the logic parsing to ensure
these are kept in-sync.


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

------- You are receiving this mail because: -------
You are the QA contact for the bug.

http://bugs.exim.org/show_bug.cgi?id=167

Tony Finch <dot@dotat.at> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |dot@dotat.at




--- Comment #16 from Tony Finch <dot@dotat.at> 2008-09-15 13:17:23 ---
What do you think of wrapping the generic condition in bare braces, i.e.
eliminating the bool keyword? So the following would be roughly equivalent...

$acl_m_foo
${if {$acl_m_foo} }
${if {$acl_m_foo} {true} {false] }

The following should also work

${if and{{ $acl_m_foo }{ $acl_m_bar }} }


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

------- You are receiving this mail because: -------
You are the QA contact for the bug.

http://bugs.exim.org/show_bug.cgi?id=167




--- Comment #17 from Phil Pennock <exim-dev@spodhuis.org> 2008-09-16 00:20:09 ---
(In reply to comment #16)
> What do you think of wrapping the generic condition in bare braces, i.e.
> eliminating the bool keyword? So the following would be roughly equivalent...

For myself, I like the proposed syntax (first two cases, but see below). But
for explaining it, time after time, to those people who visit exim-users and
refuse to read documentation, I'm not so sure. I'm also not so sure that bool
should be eliminated, as opposed to made an implicit default -- some people
will prefer to see the explicit bool in their configs and I'm, in some cases,
one of those people: I like stuff to be cleanly self-documenting and will
happily do things like, in Perl, use the 'scalar' keyword explicitly, even in
clearly scalar context. I think this is somewhat the same. Doesn't mean that
I'll force this on others, just that I don't think bool{} should disappear
entirely.

Going from memory, it should be fairly simple to code as a check in the ECOND_*
area, if the next character is a { then implicitly bool.

However,
> ${if and{{ $acl_m_foo }{ $acl_m_bar }} }

The basic proposal would require:
${if and {{ {$acl_m_foo} }{ {$acl_m_bar} }} }
to have the extra bare braces. To accept your syntax here, wouldn't each of
the possible boolean values need to become an ECOND_* token? Or just handled
explicitly, same as for a bare "{"?

I'm not so sure I like where that's going.


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

------- You are receiving this mail because: -------
You are the QA contact for the bug.

http://bugs.exim.org/show_bug.cgi?id=167




--- Comment #18 from Jakob Hirsch <jh.exim-bugzilla@plonk.de> 2008-09-16 08:40:51 ---
(In reply to comment #16)
> What do you think of wrapping the generic condition in bare braces, i.e.
> eliminating the bool keyword? So the following would be roughly equivalent...

Shouldn't we make this consistent with "def:"? Otherwise it would be new
syntax.
Like that:

${if bool:some_var {do-if-true...} {do-if-false...}}

and

${if and {{bool:some_var} {bool:another_var}} {do-if-true...} {do-if-false...}}

Same applies to the other syntax ${if bool {$some_var}...}.


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

------- You are receiving this mail because: -------
You are the QA contact for the bug.

http://bugs.exim.org/show_bug.cgi?id=167




--- Comment #19 from Tony Finch <dot@dotat.at> 2008-09-16 13:34:09 ---
Phil's point about and{ {{$acl_m_foo}} {{$acl_m_bar}} } is very good. I agree
that it shows my suggestion is not so good.

I like Jakob's suggestion of bool:acl_m_foo. Perhaps it could be supported as
well as bool{$acl_m_foo}.


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

------- You are receiving this mail because: -------
You are the QA contact for the bug.

http://bugs.exim.org/show_bug.cgi?id=167




--- Comment #20 from Phil Pennock <exim-dev@spodhuis.org> 2008-09-17 01:30:32 ---
Jakob: bool{$whatever} is not new syntax, merely a new conditional operator.

Now, bool:foo is new. The reason for def: is that it's an existence test, as
well as a definedness tests. With def:, there's no errors for unknown
variables, no expansion failures. I don't see the justification for doing this
for anything other than meta-checks on the variable itself, rather than the
value of the variable. Aside from !, all other ECOND operators use {braces},
including the unary condition tests (exists{filename}, ldapauth{query},
radius{auth-str}).

For the expansion operators, things are different. But those don't affect this
bool test case.

I think, on balance, at present I'm opposed to bool:varname as an expansion
condition; a reasoned use-case for why we need it, or why it's cleaner, or
strong support for bool:varname from someone like TF, NM or PH will see me
provide a revised patch, but for now I stand by the current version (v2).


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

On Wed, 17 Sep 2008, Phil Pennock wrote:

> I think, on balance, at present I'm opposed to bool:varname as an expansion
> condition; a reasoned use-case for why we need it, or why it's cleaner, or
> strong support for bool:varname from someone like TF, NM or PH will see me
> provide a revised patch, but for now I stand by the current version (v2).

I'm with you. Quite apart from inventing a new syntax, if you have
bool:varname, you can test only one variable, whereas if you have
bool{...} the ... can be any arbitrary expansion string.

--
Philip Hazel

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

------- You are receiving this mail because: -------
You are the QA contact for the bug.

http://bugs.exim.org/show_bug.cgi?id=167




--- Comment #21 from Philip Hazel <ph10@hermes.cam.ac.uk> 2008-09-17 17:34:24 ---
On Wed, 17 Sep 2008, Phil Pennock wrote:

> I think, on balance, at present I'm opposed to bool:varname as an expansion
> condition; a reasoned use-case for why we need it, or why it's cleaner, or
> strong support for bool:varname from someone like TF, NM or PH will see me
> provide a revised patch, but for now I stand by the current version (v2).

I'm with you. Quite apart from inventing a new syntax, if you have
bool:varname, you can test only one variable, whereas if you have
bool{...} the ... can be any arbitrary expansion string.


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##

------- You are receiving this mail because: -------
You are the QA contact for the bug.

http://bugs.exim.org/show_bug.cgi?id=167




--- Comment #22 from Phil Pennock <exim-dev@spodhuis.org> 2008-09-28 12:11:32 ---
Created an attachment (id=275)
--> (http://bugs.exim.org/attachment.cgi?id=275)
Documentation update.

Nobody provided any further feedback, so here's the documentation.


--
Configure bugmail: http://bugs.exim.org/userprefs.cgi?tab=email

--
## List details at http://lists.exim.org/mailman/listinfo/exim-dev Exim details at http://www.exim.org/ ##