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 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

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 but currently it works in Zope2.

You're probably referring to the NamedTemplate thing that zope.formlib has (and 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.'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 can entirely be ripped out.

Given that 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 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  -
**  No cross posts or HTML encoding!  **
(Related lists - )

Reply via email to