[Grok-dev] regebro-guido-templates: comments

[Lennart Regebro]

On 10/16/07, Martijn Faassen <faassen at startifact.com> wrote:
> Hi there,
>
> I briefly looked at this branch last night. Overall I think the approach
> works well and I like how there are tests. :)
>
> Here are some of my comments:

OK, I have refactored things a bit after these comments.

> * We should seriously reconsider the naming_convention. Several methods
> are added that have underscores in them. Zope 3's camelCase and we tend
> to mostly follow those conventions for Grok, and this would break that.
> Of course if we can come up with good single word method names we can
> forgo this entire discussion.

The methods are now called _initFactory(), render() and namespace().

> * having to do self.__grok_module__ = martian.util.caller_module()
> everywhere in the Genshi integration isn't very nice. I wonder whether
> we can set this information externally during the grokking process so it
> won't put a constraint on subclassing.

I can't see any way to get rid of this.

> * while the Genshi integration code is already short, it'd be nice if we
> could make it go down even more. Not very urgent before we merge this
> branch though - just something to consider.

Now only this is needed for the tempate class:

   class MyPageTemplateFile(grok.components.GrokPageTemplate):

        def __init__(self, filename, _prefix=None):
            file = open(os.path.join(_prefix, filename)
            self._template = MyTemplate(file.read())
            self.__grok_module__ = martian.util.caller_module()

        def render(self, view):
            namespace = self.namespace(view)
            namespace.update(view.namespace())
            return self._template.render(**namespace)

Ain't gonna get much shorter than that. :-) (unless we can get rid of
the __grok_module__ magic, but that's still only one line).

> I think the responsibility for setting up the namespace should be
> primarily in the template language implementation. Therefore I'd suggest
> we move default_namespace() to the template language implementation,
> something like this:

> def namespace(self, view):
>     namespace = {}
>     namespace['request'] = view.request
>     namespace['view'] = view
>     namespace['context'] = view.context
>     namespace['static'] = view.static
>     return namespace

This is now the default for templates, so if this is what you want,
you don't have
to do anything in your subclass. If you don't want it, you can
override and for example return just {}.

> and the render_template() method in the template language extension does
> this:
>
> namespace = self.namespace()
> namespace.update(view.namespace())

So be it.

> Note that incidentally I eliminated the naming discussion, as both these
> methods are called 'namespace()'. (following Brandon's suggestion in an
> earlier thread)

Yup.

-- 
Lennart Regebro: Zope and Plone consulting.
http://www.colliberty.com/
+33 661 58 14 64