resource that can render a twisted.web.template

[glyph]

Questions like  this one on stack overflow shouldn't need to be asked; there should be a Resource subclass within Twisted which knows how to render your template for you.

See also #4983 for addressing this problem with documentation.

[MostAwesomeDude]

(In [33837]) Branching to 'elementresource-5395'

[MostAwesomeDude]

(In [33839]) Add ElementResource, test, and topfile.

Time for reviews. Yay?

Refs #5395

[MostAwesomeDude]

Reviews please. This is super-simple, so I'm sure that there's obvious things I'm missing, but it can't be that difficult of a task.

[dreid]

1. Oh man, return 1 is terrible. If we're going to do that (and since it was glyph's idea I assume we are) you should expand on the comment why you're doing it and how it works.
2. For callbacks that ignore their result lambda _: seems to be the most common pattern in twisted. Followed by lambda ign:
3. Lines 547-549 of test_template.py should be indented one more level.
4. You committed these changes to the wrong branch.
5. I'm not entirely sure this should be render_GET vs just render. At which point it should probably just implement IResource itself vs subclassing Resource.

[MostAwesomeDude]

(In [33844]) t.w.template: Cleanup names, comments.

Refs #5395

[MostAwesomeDude]

1. Commented on "return 1" and put it at the top with a better name.
2. Changed that.
3. Changed that too.
4. Derp. :T Fixed it.
5. Nope, as dicsussed in person, doing this kind of rendering doesn't really make sense for non-GET.

Thanks for the review.

[dreid]

Ok, this looks better, as per discussion with glyph it sounds like this shouldn't actually be a resource it should be a function that can you can call as:

    def render_GET(self, request):
        return renderElement(request, MyElement(…))

And maybe in a separate ticket there should be a resource which has the same semantics as this resource and calls renderElement for you.

Also:

1. the deferred from flatten should have an errback.
   1. it should call log.err
   2. it should check request.site.displayTracebacks
      1. it should render a and write out a twisted.web.util.FailureElement
   3. it should call request.finish

[MostAwesomeDude]

Okay. Let's go for it again. dreid has recused himself, so somebody else will need to review.

[wsanchez]

In template.py, renderElement(), we should be rendering some sort of error message (but no traceback, or other leakage) if not request.site.displayTracebacks, rather than rendering nothing at all.

Over in test_simpleFailure(), there is code that is checking that we render nothing:

    self.assertEqual("".join(self.request.written), "")

But that would prevent renderElement() from passing if it did render a useful error message.

Nit: Glyph notes that the spacing conventions for code (three lines between classes, two between methods) aren't being followed here.

[dreid]

(In [33847]) Implement error output when displayTracebacks is False. Improve docstrings. Correct spacing.

Refs #5395

[dreid]

I went ahead and dealt with the review from wsanchez. Mostly I added docstrings, fixed spacing, added an error for when we don't want to display the traceback. I also refactored the tests slightly to avoid invoking _render.

[wsanchez]

Yes, this makes the sense.

The only thing I might reconsider is the use of <h1> for the error message. Since the error could happen in a nested element, an <h1> may not be appropriate. I think a weenier web weenie than I might prefer a generic <div class="RenderingError"> or some such thingo that could be styled. But that could be addressed in another ticket from someone how has run into this and has more thoughtfulness about that the right way to do that is.

[dreid]

(In [33853]) Render a better error. Now it is big and red on an explicit white background.

Refs #5395

[dreid]

(In [33855]) Fix missing end quote on style attribute.

Refs #5395

[dreid]

[13:11:13] <exarkun> dreid: were you asleep during the `@d.addCallback´ conversation?
[13:13:05] <exarkun> dreid: It's easy to fix.  Please do.

[dreid]

(In [33858]) Address exarkun's review about @d.addCallback being bad.

Refs #5395

[wsanchez]

+1

[dreid]

(In [33859]) Assert that only one error is flushed.

Refs #5395

[dreid]

exarkun had some informal review which I addressed.

[exarkun]

1. twisted/web/template.py
   1. Please file a ticket for fixing NOT_DONE_YET. There are other ways to avoid the circularity without relying on accidental (!!) integer identity semantics. Link to the ticket from that comment.
   2. Always pass a message argument to log.err. renderElement is calling it with only a failure argument.
   3. renderElement needs to be in __all__
2. twisted/web/test/test_template.py
   1. TestElement needs a class docstring
   2. TestFailureElement too
   3. The docstrings for the test methods on TestRenderElement should not start with "Test that ...".
   4. Some of the indentation is funny. It should all look like the indentation in test_simpleFailure, for the assertion self.request.written.
   5. assertEqual is not the right check to use with NOT_DONE_YET. It should be assertIdentical, because the implementation uses is (in one place, == in another, ha ha).
   6. Please remove the blank line at the beginning of some of the test methods.

Thanks very much for your work on this, and to the efforts of all the previous reviewers. I think after these changes are made the branch will be in good shape, and you should merge it if you get a good build on buildbot.

[dreid]

(In [33887]) Fix indentation, docstrings, NOT_DONE_YET assertions.

Also reference the ticket for fixing the circular NOT_DONE_YET imports.

Refs #5395

[dreid]

(In [33888]) Deal with circular dependency between util and template using the fully qualified name of FailureElement.

Refs #5395

[dreid]

Builds succeeded except for intermittent and usual windows bugs.  http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/elementresource-5395

[dreid]

(In [33889]) Merge elementresource-5395: Add renderElement, for rendering Elements.

Authors: MostAwesomeDude, dreid Reviewer: wsanchez, exarkun Fixes: #5395

Add renderElement, a function for rendering Elements from resources. It returns NOT_DONE_YET, writes the template incrementally to the request, and handles errors during rendering. It will display a traceback if the Site is configured to do so.