Merge lp:~corydodt/txgenshi/506147 into lp:txgenshi

[Cory Dodt]

[Cory Dodt (corydodt)]

Hide the loader behind a descriptor. Descriptors have a magical ability to access the instance they are attached to, which makes it a fine replacement for using the context (which is deprecated in Nevow). And it works with Elements.

[Duncan McGreggor (oubiwann)]

I remember having a passing thought about Elements (mostly dread and avoidance) a couple years ago. Fascinating approach; I love what you've done. +1 for merge with the following minor points addressed (or discussed):

[1] Let's not use the private "_" prefix for classes. Instead, let's augment the docstring with something like the following at the end:
  """
  ...
  This class is intended for private use and is liable to change.
  """

[2] If a class has no docstring, let's put a new line after the classes and before the first method. But I guess a docstring would really be better ;-)

[3] Can you fix the indentation in this old code:

34 + nevowRun = super(_GenshiCommonLoader, self).load(ctx=ctx,
35 preprocessors=preprocessors)

If it won't all fit on one line, let's change it to this:

nevowRun = super(GenshiCommonLoader, self).load(
    ctx=ctx, preprocessors=preprocessors)

Note to self: I should probably open a clean-up branch and take care of the old warts...

Again, nice work. You're doing a great job of breaking the tasks up into discrete branches, and not being tempted into tackling stuff that's out of scope. This is rare, and it makes successive branches a delight to review :-)

review: Approve

[Cory Dodt (corydodt)]

On Mon, Jan 11, 2010 at 10:48 PM, Duncan McGreggor <email address hidden>wrote:

> Review: Approve
> I remember having a passing thought about Elements (mostly dread and
> avoidance) a couple years ago. Fascinating approach; I love what you've
> done. +1 for merge with the following minor points addressed (or discussed):
>
> [1] Let's not use the private "_" prefix for classes. Instead, let's
> augment the docstring with something like the following at the end:
> """
> ...
> This class is intended for private use and is liable to change.
> """
>
> [2] If a class has no docstring, let's put a new line after the classes and
> before the first method. But I guess a docstring would really be better ;-)
>

[1,2] resolved, r37 ([2] resolved by adding docstrings).

Any feelings, one way or the other, about using __all__ to document
public-ness?

>
> [3] Can you fix the indentation in this old code:
>
> 34 + nevowRun = super(_GenshiCommonLoader, self).load(ctx=ctx,
> 35 preprocessors=preprocessors)
>
> If it won't all fit on one line, let's change it to this:
>
> nevowRun = super(GenshiCommonLoader, self).load(
> ctx=ctx, preprocessors=preprocessors)
>

* Resolved, r35.

>
> Note to self: I should probably open a clean-up branch and take care of the
> old warts...
>
> Again, nice work. You're doing a great job of breaking the tasks up into
> discrete branches, and not being tempted into tackling stuff that's out of
> scope. This is rare, and it makes successive branches a delight to review
> :-)
> --
> https://code.edge.launchpad.net/~corydodt/txgenshi/506147/+merge/17203
> You are the owner of lp:~corydodt/txgenshi/506147.
>

--
_____________________

[lp:~corydodt/txgenshi/506147]

35. By Cory Dodt on 2010-01-12

#506147: whitespace

36. By Cory Dodt on 2010-01-12

#506147: pyflakes cleanup

37. By Cory Dodt on 2010-01-12

#506147: style--no leading _ in class names

[Cory Dodt (corydodt)]

Based on prior approval with these changes, I am merging.

[Duncan McGreggor (oubiwann)]

I'm +1 for the use of __all__ for public objects.

=== modified file 'txgenshi/loader.py'
--- txgenshi/loader.py	2010-01-11 19:03:21 +0000
+++ txgenshi/loader.py	2010-01-12 07:47:11 +0000
@@ -17,7 +17,6 @@
 from inspect import getmembers
 from collections import deque
 
-from nevow import inevow
 from nevow.loaders import xmlstr
 
 from genshi.template import MarkupTemplate, Context
@@ -90,12 +89,23 @@
 
 
 def getGenshiString(template, data):
+    """
+    Execute a Genshi template, rendering Genshi markup into a markup string.
+    """
     tmpl = MarkupTemplate(template)
     ctx = TxGenshiContext(data)
     return tmpl.generate(ctx).render('xhtml')
 
 
-class genshistr(xmlstr):
+class GenshiCommonLoader(xmlstr):
+    """
+    Load a Genshi template and use it to render a resource or fragment
+
+    This class is intended for private use and is liable to change.
+    """
+    def __init__(self, container, template):
+        self.dataContainer = container
+        xmlstr.__init__(self, template)
 
     def load(self, ctx=None, preprocessors=()):
         """
@@ -111,14 +121,15 @@
         syntax in the template(s). If you will only be using Genshi, you can
         enable your own caching mechanism and not up-call to the parent class.
         """
-        renderer = inevow.IRenderer(ctx)
-        data = DataProxy(renderer)
+        if not hasattr(self, 'orig'):
+            self.orig = self._getTemplate()
+        data = DataProxy(self.dataContainer)
         # save a copy of the original, unmolested template, complete with
         # Genshi markup
         orig = self.template
-        self.template = getGenshiString(self.template, data)
-        nevowRun = super(genshistr, self).load(ctx=ctx,
-            preprocessors=preprocessors)
+        self.template = getGenshiString(self.orig, data)
+        nevowRun = super(GenshiCommonLoader, self).load(
+                ctx=ctx, preprocessors=preprocessors)
         # null out Nevow's caching of the template
         self._cache = None
         # revert to the unparsed Genshi template so that genshi attrs/methods
@@ -127,23 +138,60 @@
         return nevowRun
 
 
-class genshifile(xmlstr):
-
-    def __init__(self, filename):
-        self._filename = filename
-
-    def load(self, ctx=None, preprocessors=()):
-        if not hasattr(self, 'orig'):
-            self.orig = open(self._filename).read()
-            #self._reallyLoad(self._filename, ctx, preprocessors)
-        renderer = inevow.IRenderer(ctx)
-        data = DataProxy(renderer)
-        self.template = getGenshiString(self.orig, data)
-        nevowRun = super(genshifile, self).load(ctx=ctx,
-            preprocessors=preprocessors)
-        # null out Nevow's caching of the template
-        self._cache = None
-        # revert to the unparsed Genshi template so that genshi attrs/methods
-        # have the chance to generate fresh data
-        self.template = self.orig
-        return nevowRun
+class GenshiStr(GenshiCommonLoader):
+    """
+    A version of the "true" loader that accepts a string.
+
+    This class is intended for private use and is liable to change.
+    """
+    def _getTemplate(self):
+        return self.template
+
+
+class GenshiFile(GenshiCommonLoader):
+    """
+    A version of the "true" loader that accepts a filename.
+
+    This class is intended for private use and is liable to change.
+    """
+    def _getTemplate(self):
+        return open(self.template).read()
+
+
+class DescriptorLoader(object):
+    """
+    A descriptor that can be used as docFactory where you would normally find
+    a loader, at the class level.  If you wish to instantiate a loader at the
+    instance level, see GenshiFile and GenshiString.
+
+    This class is intended for private use and is liable to change.
+    """
+    loaderClass = None
+
+    def __init__(self, template):
+        self.template = template
+        self._cache = LAZY
+
+    def __get__(self, instance, cls):
+        if self._cache is not LAZY:
+            return self._cache
+
+        loader = self.loaderClass(instance, self.template)
+        self._cache = loader
+        return loader
+
+
+class genshistr(DescriptorLoader):
+    """
+    Loader to read a string containing a genshi template and output a
+    populated string.
+    """
+    loaderClass = GenshiStr
+
+
+class genshifile(DescriptorLoader):
+    """
+    Loader to read a file containing a genshi template and output a
+    populated string.
+    """
+    loaderClass = GenshiFile

=== modified file 'txgenshi/test/example.py'
--- txgenshi/test/example.py	2010-01-11 19:03:21 +0000
+++ txgenshi/test/example.py	2010-01-12 07:47:11 +0000
@@ -5,7 +5,7 @@
 
 from twisted.python.util import sibpath
 
-from nevow import rend
+from nevow import rend, page
 
 from txgenshi.loader import genshifile, genshistr
 
@@ -15,7 +15,8 @@
     color = "red"
 
 
-TEMPLATEFILE = sibpath(__file__, '../../demo/templates/genshimix.xhtml')
+TEMPLATEFILE = sibpath(__file__, '../../txgenshidemo/templates/genshimix.xhtml')
+ELEMENTFILE = sibpath(__file__, '../../txgenshidemo/templates/element.xhtml')
 
 
 class GenshiMixPage(rend.Page):
@@ -58,3 +59,11 @@
 
 class GenshiMixPageStr(GenshiMixPage):
     docFactory = genshistr(open(TEMPLATEFILE).read())
+
+
+class GenshiMixElement(page.Element):
+    docFactory = genshifile(ELEMENTFILE)
+
+    @page.renderer
+    def hello(self, request, tag):
+        return tag["Hello"]

=== modified file 'txgenshi/test/test_example.py'
--- txgenshi/test/test_example.py	2010-01-10 09:01:59 +0000
+++ txgenshi/test/test_example.py	2010-01-12 07:47:11 +0000
@@ -5,6 +5,8 @@
 
 from twisted.trial import unittest
 
+from nevow import flat
+
 from txgenshi.test import example
 
 
@@ -63,3 +65,26 @@
         self.fail("no implementation of xinclude yet") # pragma: nocover
 
     test_xinclude.skip = "need to implement xinclude"
+
+
+class ElementTest(unittest.TestCase):
+    """
+    Templates work just as well using Elements as they do using Pages
+    """
+    def setUp(self):
+        self.el = example.GenshiMixElement()
+        self.rendered = str(flat.flatten(self.el))
+
+    def test_nevow(self):
+        """
+        Nevow rendering works on Elements using genshifile
+        """
+        # verify that genshi is not adding any html boilerplate
+        self.assertEqual(self.rendered[:5], '<div>')
+        self.assertSubstring('<b>Hello</b>', self.rendered)
+
+    def test_genshi(self):
+        """
+        Genshi rendering works on Elements using genshifile
+        """
+        self.assertSubstring('<b>Earth</b>', self.rendered)

=== modified file 'txgenshi/util.py'
--- txgenshi/util.py	2010-01-11 01:42:19 +0000
+++ txgenshi/util.py	2010-01-12 07:47:11 +0000
@@ -200,6 +200,7 @@
     'classifiers': txgenshi.classifiers,
     'package_data': {
         'txgenshidemo': ['demo.tac',
+                 'templates/element.xhtml',
                  'templates/genshimix.xhtml',
                  'templates/genshiinclude.xhtml',
                  'templates/home.xhtml',

=== added file 'txgenshidemo/templates/element.xhtml'
--- txgenshidemo/templates/element.xhtml	1970-01-01 00:00:00 +0000
+++ txgenshidemo/templates/element.xhtml	2010-01-12 07:47:11 +0000
@@ -0,0 +1,5 @@
+<div xmlns:n=
+  "http://nevow.com/ns/nevow/0.1" xmlns:genshi=
+  "http://genshi.edgewall.org/"><b n:render=
+    "hello"/><b genshi:with=
+    "world='Earth'">$world</b></div>