Thanks for looking into this, Hanno! Here's my feedback:
Hanno Schlichting wrote:
Hanno Schlichting wrote:
I kept my promise and added the simple tests for the first two issues
I found while doing testing against Plone.
I have meanwhile fixed the first trivial issue (conflicting argument
called 'instance')
I took a look at it. Thanks! It's too bad that we have to duplicate so
much code from Zope 3 now. Therefore I would suggest that we fix up
zope.app.pagetemplate properly so that it won't conflict with
'instance'. Then we can get rid of the duplication in Five. Thanks to
the individual projects nowadays, it should be trivial to update
zope.app.pagetemplate.
[...]
Now as you can see the only difference is that one of them uses a class
variable for assigning the template and the other one is using an
instance variable.
ViewPageTemplateFile etc. are only meant to be used as class attributes,
never as instance attributes. This statement is also true for the
current, acquisition-based one from Five. In my opinion, the fact that
it accidentally worked as an instance variable isn't a very strong
argument for continuing to support it. To me, this is a prime example of
misusing a Five component which now leads to problems when we go pure Zope3.
From what I understand some magic place in between doesn't find the
template instance variable during ZCML processing as it operates on
classes only and therefor doesn't turn the template into a
BoundPageTemplateFile.
It doesn't happen during ZCML processing. It's a simple class
descriptor, so the magic happens in ViewPageTemplateFile's __get__ which
is invoked each time you do view.template (the '.' invokes __get__). I
recommend reading about new-style class descriptors and properties.
Using an instance variable called template and calling it later on
without passing in the view as the first argument doesn't work at all in
Zope3. In Zope2 it did so far, as the ViewPageTemplateFile would use
Acquisition to find its view.
Right. This led to all sorts of weird and icky problems, so thank God
it's gone now.
I don't have any good idea on how to handle this problem.
Do we really have to support instance-level templates? I would still
argue that if anybody's using ViewPageTemplateFile like this, they're
using it wrong. I personally have no intent on supporting this.
If we really have to support it, then there's a simple solution: don't
use ViewPageTemplateFile for instance-level attributes, use a variant
that we provide instead. We could introduce this variant in both
pre-AQ-parent and post-AQ-parent Zope versions to ease compatibility.
We can
probably walk up the stack frame to find the view in most cases, as the
template is called in almost all cases directly from the view.
-1. Walking up stack frames for guessing stuff like this will usually
destroy you.
But the third test failure, which I haven't written a test for yet,
breaks even then. Essentially it puts in an adapter in between the view
and the template where the adapter doesn't have any reference to the
view anymore, so getting to the view from the template is impossible
even by walking up the stack frame. This use-case is highly specialized
(the code is in plone.app.form._named) but currently it works in Zope2.
You're probably referring to the NamedTemplate thing that zope.formlib
has (and plone.app.form reimplements the stuff for Zope 2). In
zope.formlib, the same __get__ technique that ViewPageTemplateFile uses
is used to get a hold of the view instance when you do view.template.
plone.app.form's replacement for this technique is to use acquisition to
get at the view. This obviously has to go since acquisition is no longer
supported for views. In fact, if I'm not mistaken, this bit of
plone.app.form can entirely be ripped out.
Given that plone.app.form does monkey patches (which I'm unwilling to
support in any means, the original authors made potential
incompatibilities their problem when they introduced them), I'm almost
certain that there will never be one version of plone.app.form that will
work with both the pre-AQ-parent and the post-AQ-parent Zope. You could
still try, of course. At the very least, you could make a try/except
clause. Either way, as I said, I'm not offering my help for this package
since it contains monkey patches.
___
Zope-Dev maillist - Zope-Dev@zope.org
http://mail.zope.org/mailman/listinfo/zope-dev
** No cross posts or HTML encoding! **
(Related lists -
http://mail.zope.org/mailman/listinfo/zope-announce
http://mail.zope.org/mailman/listinfo/zope )
