Hello Peter,

On 1/12/2017 6:49 PM, Peter Suter wrote:
> Hello,
>
> On 12.01.2017 17:31, Christian Boos wrote:
>> A demo instance is available:
>>
>>     https://trac.edgewall.org/testing
>
> I stumbled on this:
> https://trac.edgewall.org/testing/admin/spamfilter/bayes
> Trac detected an internal error: TemplateNotFound: jadmin_bayes.html
>
> I guess all admin panels are assumed to use Jinja templates (but the
> plugin ones don't)?
>
https://trac.edgewall.org/browser/cboos.git/trac/admin/web_ui.py?rev=jinja2-trunk-r15341&marks=96,97,109#L72
>

This 'j' prefix was present in various places in the templates and in
the code, which made troubleshooting this annoying. I have now removed
that prefix and all the special cases associated to this temporary hack,
so I could start testing this scenario.


> Can IAdminPanelProvider.render_admin_panel() get a similar legacy mode
> as IRequestHandler.process_request?

In render_admin_panel we already return just a pair (template, data). If
we keep that as the legacy API, we have to come up with something
different for the "new" API. I prefer to pay the cost of a small
backward incompatibility here, and keep (template, data) for the new
API, as it's simpler in the long run and consistent with what we already
do in IRequestHandler.process_request.

Plugins like the SpamFilter will have to be modified to return
(template, data, None) here. No big deal, see:


https://trac.edgewall.org/attachment/ticket/12639/spamfilter-jinja2-compat-r15349.diff

We have the same issue with the preference panel.

I've just pushed the changes that make this happen, see in particular:

    https://trac.edgewall.org/changeset/0f3aee46/cboos.git


>> The plugins making use of Genshi templates won't need to be changed a
>> bit, while those which want to port to or start using Jinja2 templates
>> will have to use a new API: `IRequestHandler.process_request` methods
>> will have to return a `(template, data)` pair instead of the legacy
>> `(template, data, None)` triple.
>
> The difference seems quite subtle. But I guess it's tricky to be more
> explicit without breaking backward compatibility or introducing an ugly
> wart for the new way.
>
>
> One tiny, tiny thing:
>
https://trac.edgewall.org/browser/cboos.git/trac/util/html.py?rev=jinja2-trunk-r15341&marks=383#L379
>
> "will be in" -> "will be removed in"?
>

Thanks, fixed!

>
> Unfortunately I can't do a more substantial review, sorry, but from what
> I read this sounds great.
>

Thanks again!

-- Christian

-- 
You received this message because you are subscribed to the Google Groups "Trac 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to trac-dev+unsubscr...@googlegroups.com.
To post to this group, send email to trac-dev@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.

Reply via email to