python:test not functional in Page Template code invoked from Five.BrowserView

Bug #223528 reported by ikekrull
4
Affects Status Importance Assigned to Milestone
Zope 3
Won't Fix
Undecided
Unassigned
zope.tales
Invalid
Wishlist
Unassigned

Bug Description

python:test does not work in view templates that use newer classes like Five.BrowserView.

Marking 'invalid' and suggesting python:cond workarounds e.g

https://bugs.launchpad.net/zope3/+bug/97886

is really not acceptable.

Unless python:test is to be completely deprecated, and purged from all the books, online tutorials, code shipped with Zope etc. it would be batsh*t insane to ignore this issue. It is very confusing to have what amounts to two different dialects of ZPT bundled in the product.

Here is a patch that should illustrate a fix for this issue - i'm sure there is a better way to write this but i'm not a plone developer.

--- pythonexpr.py 2007-10-30 02:13:44.000000000 +1300
+++ /opt/Plone-3.0.6/lib/python/zope/tales/pythonexpr_new.py 2008-04-28 19:56:45.000000000 +1200
@@ -16,6 +16,9 @@
 $Id: pythonexpr.py 30451 2005-05-20 04:54:15Z fdrake $
 """

+from RestrictedPython.Utilities import utility_builtins
+test = utility_builtins['test'] # see DocumentTemplate/DT_Util.py
+
 class PythonExpr(object):
     def __init__(self, name, expr, engine):
         text = '\n'.join(expr.splitlines()) # normalize line endings
@@ -49,7 +52,8 @@
                 if val is not marker:
                     val = ExprTypeProxy(vname, val, econtext)
                     names[vname] = val
-
+ #Add this 'oddball' function to the secure 'builtin' namespace explicitly
+ builtins['test']=test
         names['__builtins__'] = builtins
         return names

Revision history for this message
ikekrull (pete-marchingcubes) wrote :

New patch to remove the dependency on RestrictedPython, which was said to be a bad thing on #plone IRC.

--- pythonexpr.py 2007-10-30 02:13:44.000000000 +1300
+++ /opt/Plone-3.0.6/lib/python/zope/tales/pythonexpr.py 2008-04-30 11:49:31.000000000 +1200
@@ -16,6 +16,15 @@
 $Id: pythonexpr.py 30451 2005-05-20 04:54:15Z fdrake $
 """

+def test(*args):
+ #Special 'test' function in ZPT does not exist, and should
+ #be supported for view code compatibility with Document Templates.
+ #see DocumentTemplate/DT_Util.py
+ l=len(args)
+ for i in range(1, l, 2):
+ if args[i-1]: return args[i]
+ if l%2: return args[-1]
+
 class PythonExpr(object):
     def __init__(self, name, expr, engine):
         text = '\n'.join(expr.splitlines()) # normalize line endings
@@ -49,7 +58,8 @@
                 if val is not marker:
                     val = ExprTypeProxy(vname, val, econtext)
                     names[vname] = val
-
+ #Add this 'oddball' function to the secure 'builtin' namespace explicitly
+ builtins['test']=test
         names['__builtins__'] = builtins
         return names

Revision history for this message
Christian Zagrodnick (zagy) wrote :

*If* we want to keep test the patch still isn't the way to go IMHO. It inserts tests to the builtins which should not be done. test should explicitly passed to the PT-context, should it not?

Revision history for this message
ikekrull (pete-marchingcubes) wrote :

I have no idea how this is best achieved as I try to avoid having to mess with zope internals too often. I'm not a plone dev, just someone trying to fix what I see as an issue.

Has python:test been deprecated? theres a lot of literature that references it, and many of the existing page templates in Plone use it. Why would it be a good thing to remove it? And if its going to be removed, why isn't it being removed entirely?

Sure, you'd have to rewrite a bunch of page templates - I mean, I could see a case for not having it, since there are other ways to acheive the result, but it seems like its just laziness that has seen it disappear in some parts of Zope with no explanation.

Revision history for this message
Brian Sutherland (jinty) wrote :

> Why would it be a good thing to remove it?

Because it's cruft, quoting the Zen of python:

"There should be one-- and preferably only one --obvious way to do it."

That way is the python "a and b or c" syntax (which also works in pure python). Having something special for page templates just forces people to learn two equivalent methods of achieving exactly the same thing. That makes the barrier to entry in zope just a little bit higher.

(if you are trying to maintain backwards compatibility of zope2 templates, then have a zope2 specific extension rather than building it into page templates for everyone.)

Revision history for this message
ikekrull (pete-marchingcubes) wrote :

Then there was never a rationale for python:test.

But since its been added, is documented in every ZPT reference and has been in zope/plone for years, then removing it with no explanation, and inconsistently - it works in page templates derived from built in DocumentTemplate based types, but not elsewhere - seems unreasonable.

And if you think deliberately not conforming to the ZPT spec and arbitrarily breaking the code presented in a huge quantity of Zope documentation is lowering the barrier of entry, i think you're way off base.

By all means, deprecate python:test - but if this is to be done, it should be done across the board, and the documentation updated. Otherwise, it should be supported across the board.

Currently, its a bug, and should be fixed. If it is to be removed from ZPT, at least remove it in a sane way.

Revision history for this message
Brian Sutherland (jinty) wrote :

> But since its been added, is documented in every ZPT reference and has been in zope/plone for years.

Zope 2/Plone that is. Zope 3 has never had it.

Perhaps it makes sense (for consistency with the rest of Zope 2) to add it to the Zope 2 implementation, probably somewhere in Products.Five, I'd guess. I have no opinion about that.

But, looking at the original, rejected, bug report https://bugs.launchpad.net/zope3/+bug/97886. I don't think that this is appropriate or that there is enough consensus to put it directly into zope.tales.

Tres Seaver (tseaver)
Changed in zope3:
status: New → Won't Fix
Tres Seaver (tseaver)
Changed in zope.tales:
importance: Undecided → Wishlist
status: New → Triaged
Revision history for this message
Colin Watson (cjwatson) wrote :

The zope.tales project on Launchpad has been archived at the request of the Zope developers (see https://answers.launchpad.net/launchpad/+question/683589 and https://answers.launchpad.net/launchpad/+question/685285). If this bug is still relevant, please refile it at https://github.com/zopefoundation/zope.tales.

Changed in zope.tales:
status: Triaged → Invalid
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.