Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #2390 (closed defect: fixed)

Opened 7 years ago

Last modified 7 years ago

[PATCH] Displaying nested TG widgets from Genshi templates may give Internal Error

Reported by: chrisz Owned by: Chris Arndt
Priority: normal Milestone: 1.1.x bugfix
Component: TG Widgets Version: 1.1
Severity: normal Keywords: needs tests, genshi, kid, widgets
Cc:

Description

The problem can be reproduced with the following widgets:

from turbogears import widgets

class MyForm(widgets.TableForm):

    fields = [widgets.TextField(name='text')]

class MySuperForm(widgets.Widget):

    template = """<div>${subform()} and more</div>"""

    params = ['subform']

    def subform(self):
        return MyForm()

If you display MySuperForm() from a Kid template, everything works fine. But if you display it from a Genshi template, the following error appears:

  File "<string>", line 32, in _pull
  File ".../kid/parser.py", line 180, in _track
    ev, item = p
ValueError: too many values to unpack

The cause for this error seems to be a hack that was introduced with ticket #1355 for automatically wrapping transformed Kid widgets in an ET() call so that they can be displayed in Genshi templates - otherwise the ET() call must be coded into the Genshi template. This hack (in turbogears.widgets.base.Widget.display) checks the nesting level of the widget and applies the ET() call only at the top level for the Genshi engine. However, in cases like the one above the determination of the nesting level fails; it works only for compound input widgets.

Attachments

widgets.patch Download (1.2 KB) - added by chrisz 7 years ago.
Suggested patch, better determination of widget nesting level

Change History

comment:1 Changed 7 years ago by chrisz

This issue is probably also the cause of the error reported  here.

Changed 7 years ago by chrisz

Suggested patch, better determination of widget nesting level

comment:2 Changed 7 years ago by chrisz

I have added a patch that keeps track of the nesting level for all kinds of nesting.

We still need a regression test for this.

Further suggestions:

  • We should remove the ET() calls in the quickstart tempaltes since they are superfluous thanks to this automatism.
  • I think TG widgets should support Genshi too. The main reason for people to stay with TG 1.x is that they want to avoid upgrading old projects (e.g. TG widgets to TW). It's pretty inconsistent that TG 1.1 has Genshi as default engine, but widgets can only use Kid. This also causes problems like in this ticket.

comment:3 Changed 7 years ago by Chris Arndt

I looked at the patch. Looks good and sensible to me. Maybe a source code comment explaining what this check does would be in order.

I agree with your suggestion re removing ET() calls in the quickstart templates.

For the Genshi support for TG widgets, please open another ticket.

comment:4 Changed 7 years ago by Chris Arndt

  • Keywords genshi, kid, added; genshi kid removed
  • Owner set to Chris Arndt
  • Status changed from new to assigned
  • Component changed from TurboGears to TG Widgets
  • Summary changed from Displaying nested TG widgets from Genshi templates may give Internal Error to [PATCH] Displaying nested TG widgets from Genshi templates may give Internal Error

BTW, please follow the  patching guidelines. ;)

comment:5 Changed 7 years ago by Chris Arndt

  • Keywords needs tests, added

comment:6 Changed 7 years ago by chrisz

Ok, commited now in r6865. Did I forget anything else besides the [PATCH] tag?

comment:7 Changed 7 years ago by Chris Arndt

Patches should apply from the root of the branch.

comment:8 Changed 7 years ago by chrisz

Yes, I forgot that. Nice that Trac displays it correctly anyway.

comment:9 Changed 7 years ago by Chris Arndt

Can we close this ticket?

comment:10 Changed 7 years ago by chrisz

  • Status changed from assigned to closed
  • Resolution set to fixed

I think so.

Note: See TracTickets for help on using tickets.