Discussion:
[Trac-dev] Jinja2 branch is now ready to test and review
Christian Boos
2017-01-12 16:31:33 UTC
Permalink
Hello everybody,

I took some steps to finalize my migration of Trac to the Jinja2
template engine, and the results are now visible in several places.

The latest code is in the jinja2-trunk-r15341 branch:

https://svn.edgewall.org/git/trac/devs/cboos
https://github.com/cboos/trac.git

A demo instance is available:

https://trac.edgewall.org/testing

(this one also contains my wiki-restyling changes, for good measure)

The #12649 ticket can be used to report trivial issues you'd find with
Jinja2 (#12640 for the restyling ones). Don't hesitate to create new
tickets for more serious issues, if any.

It would be nice to get some feedback at this point. You can contribute
by either reviewing the code, or simply by playing around in
t.e.o/testing and report any glitches you'd find there.

If you have any questions, the mailing list would probably be more
appropriate. Be sure to first go through the following wiki pages, as
they may already contain the answer you're looking for:

- https://trac.edgewall.org/wiki/TracDev/Proposals/Jinja
- https://trac.edgewall.org/wiki/TracDev/PortingFromGenshiToJinja


Known "issues" so far:

- the branch keeps Genshi templates and Jinja2 templates side by side
(e.g. search.html and jsearch.html); that was helpful during the
development, and can still be useful while reviewing the branch, but I'm
going to go back to the original names soon (jsearch.html ->
search.html), moving a step closer to the merge.

- I haven't tested the i18n support for Jinja2 templates in plugins
yet; this is what I'm going to work on next


Also, I want to address the concerns raised last year about the backward
compatibility with Genshi. I propose to use the whole 1.4.x series as a
transition period, during which both template engines will be supported.
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. There's slightly more to it than this,
but you get the idea.

I've also taken into account the plugins which inject content in
generated pages via the ITemplateStreamFilter interface. That way is
deprecated in favor of using JavaScript for that purpose, but the
interface will nevertheless be supported during the transition period as
well.

One problem is that this ITemplateStreamFilter support mostly works, but
is not perfect, and of course kills most of the speed improvement
brought by the switch to Jinja2. Consider this as an incentive to
migrate such plugins! If people are really motivated to fix/enhance this
aspect, feel free to do so, but I feel it's somewhat counter-productive
to invest more energy there.

I hope we can soon finalize the integration of this effort, before we
make more serious moves on the Python3 topic (also, Jinja2 brings in six
as a dependency).

-- 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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
Peter Suter
2017-01-12 17:49:01 UTC
Permalink
Hello,
Post by Christian Boos
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

Can IAdminPanelProvider.render_admin_panel() get a similar legacy mode
as IRequestHandler.process_request?
Post by Christian Boos
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"?


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

Peter
--
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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
Christian Boos
2017-01-14 13:56:08 UTC
Permalink
Hello Peter,
Post by Peter Suter
Hello,
Post by Christian Boos
https://trac.edgewall.org/testing
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.
Post by Peter Suter
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
Post by Peter Suter
Post by Christian Boos
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.
https://trac.edgewall.org/browser/cboos.git/trac/util/html.py?rev=jinja2-trunk-r15341&marks=383#L379
Post by Peter Suter
"will be in" -> "will be removed in"?
Thanks, fixed!
Post by Peter Suter
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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
RjOllos
2017-01-12 23:32:23 UTC
Permalink
Post by Christian Boos
Hello everybody,
I took some steps to finalize my migration of Trac to the Jinja2
template engine, and the results are now visible in several places.
https://svn.edgewall.org/git/trac/devs/cboos
https://github.com/cboos/trac.git
https://trac.edgewall.org/testing
(this one also contains my wiki-restyling changes, for good measure)
The #12649 ticket can be used to report trivial issues you'd find with
Jinja2 (#12640 for the restyling ones). Don't hesitate to create new
tickets for more serious issues, if any.
It would be nice to get some feedback at this point. You can contribute
by either reviewing the code, or simply by playing around in
t.e.o/testing and report any glitches you'd find there.
If you have any questions, the mailing list would probably be more
appropriate. Be sure to first go through the following wiki pages, as
- https://trac.edgewall.org/wiki/TracDev/Proposals/Jinja
- https://trac.edgewall.org/wiki/TracDev/PortingFromGenshiToJinja
- the branch keeps Genshi templates and Jinja2 templates side by side
(e.g. search.html and jsearch.html); that was helpful during the
development, and can still be useful while reviewing the branch, but I'm
going to go back to the original names soon (jsearch.html ->
search.html), moving a step closer to the merge.
- I haven't tested the i18n support for Jinja2 templates in plugins
yet; this is what I'm going to work on next
Also, I want to address the concerns raised last year about the backward
compatibility with Genshi. I propose to use the whole 1.4.x series as a
transition period, during which both template engines will be supported.
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. There's slightly more to it than this,
but you get the idea.
I've also taken into account the plugins which inject content in
generated pages via the ITemplateStreamFilter interface. That way is
deprecated in favor of using JavaScript for that purpose, but the
interface will nevertheless be supported during the transition period as
well.
One problem is that this ITemplateStreamFilter support mostly works, but
is not perfect, and of course kills most of the speed improvement
brought by the switch to Jinja2. Consider this as an incentive to
migrate such plugins! If people are really motivated to fix/enhance this
aspect, feel free to do so, but I feel it's somewhat counter-productive
to invest more energy there.
I hope we can soon finalize the integration of this effort, before we
make more serious moves on the Python3 topic (also, Jinja2 brings in six
as a dependency).
-- Christian
Some JavaScript issues noted so far:

* Missing group toggle checkbox on these tables: /admin/ticket/components,
/admin/ticket/milestones, /admin/ticket/versions,
/admin/versioncontrol/repository
* Missing group toggle feature on permissions table (#11417)
* Divs for plugins are not foldable on /admin/general/plugin

Also:
* On /roadmap, "Hide milestone with no due date" doesn't seem to stick,
though the submitted value appears to be effective in filtering the
milestones
* Some margin above "Repository Index" h1 on /browser page might be good
* "Visit", "View revisions", "View diff against" options on /browser page
might look better with bigger or different font
* Name input on /admin page could be made same width as URL input (General:
Basic Settings)
* Very minor preference: I like having just the box-shadow around the label
for the folding links (e.g. Attachments on ticket page), rather than a
border + box-shadow. Similarly for the folding links on the plugin admin
page where the corners of the box at least look too square, might be better
with a box-shadow and/or rounded.

Unrelated, it was a long time before I noticed the "View changes..." button
on the bottom of the /browser page. I think this has been discussed in a
ticket, but maybe that button should be made a link in the context nav?

I'll do more testing later today, or in coming days.

- Ryan
--
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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
Christian Boos
2017-01-14 19:40:03 UTC
Permalink
Hello Ryan,
/admin/ticket/components, /admin/ticket/milestones,
/admin/ticket/versions, /admin/versioncontrol/repository
* Missing group toggle feature on permissions table (#11417)
* Divs for plugins are not foldable on /admin/general/plugin
All due to missing "block head" in these templates, around the <script>
tags.
* On /roadmap, "Hide milestone with no due date" doesn't seem to stick,
though the submitted value appears to be effective in filtering the
milestones
One "|htmlattr" filter missing.
* Some margin above "Repository Index" h1 on /browser page might be good
Right, I removed the <br> but forgot to adjust this.
* "Visit", "View revisions", "View diff against" options on /browser
page might look better with bigger or different font
Oops, they still had a "10px", I need to check again for fixed sized
fonts, as obviously I missed some. Actually I missed a lot ;-)
* Name input on /admin page could be made same width as URL input
(General: Basic Settings)
OK.
* Very minor preference: I like having just the box-shadow around the
label for the folding links (e.g. Attachments on ticket page), rather
than a border + box-shadow. Similarly for the folding links on the
plugin admin page where the corners of the box at least look too square,
might be better with a box-shadow and/or rounded.
I made it a bit lighter, but no border at all counters the more "crisp"
aspect I want to get overall. I obviously like box shadows as I put them
everywhere, but after a while I felt that I've added too much "fuzziness".
Unrelated, it was a long time before I noticed the "View changes..."
button on the bottom of the /browser page. I think this has been
discussed in a ticket, but maybe that button should be made a link in
the context nav?
I'll first see how well my idea of page "actions" would work. I'm
thinking of a kind of "menu" for links/buttons that apply on the
resource itself. For wiki pages, that would be Edit / Attach / Rename /
History / Delete version / Delete page. For tickets, that would be
Modify / Attach / Comment / Delete.
"Report spam" would also go there rather than into the context
navigation. Context navigation will instead refocus on links to other
resources (the start, parent, and index for the wiki; the query, prev,
and next for tickets).

If that works well for the wiki page, ticket and milestone, I can do
that for the browser page as well, and then the "View changes..." will
be one of the actions.
I'll do more testing later today, or in coming days.
That would be great!

Thanks for the feedback so far. Here are the corresponding fixes:

(jinja2-trunk-r15341 branch)
- [a47b48a3/cboos.git] (#12639): fixing bugs spotted by Ryan.

(wiki-restyling branch)
- [6ff2ec27/cboos.git] (#633) second round...
- [1fbf96ce/cboos.git] (#12640) minor tweaks of foldable border, ...


Also, what would be great is if you (or anyone else for that matter)
could try to convert some plugins templates to Jinja2. That way, you
could tell me what is not clear in the migration guide, or simply still
missing from it.

-- 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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
Peter Suter
2017-01-14 22:22:14 UTC
Permalink
Post by Christian Boos
Also, what would be great is if you (or anyone else for that matter)
could try to convert some plugins templates to Jinja2. That way, you
could tell me what is not clear in the migration guide, or simply still
missing from it.
That's something I wanted to try anyway, since I have a few plugins that
use (relatively simple) templates.

I semi-randomly picked OnSiteNotificationsPlugin for a first test. It
only has two rather trivial templates.
https://trac-hacks.org/wiki/OnSiteNotificationsPlugin
https://trac-hacks.org/browser/onsitenotificationsplugin/trunk/onsitenotifications/templates/onsitenotification.html?rev=16170
https://trac-hacks.org/browser/onsitenotificationsplugin/trunk/onsitenotifications/templates/onsitenotification_list.html?rev=16170

Instead of actually properly reading through your nice, extensive notes
I first tried the lazy, impatient trial-by-error approach:
I changed the `return "...", data, None` to `return "...", data` to
actually use Jinja.
Then very briefly I looked at the first section of:
https://trac.edgewall.org/wiki/TracDev/PortingFromGenshiToJinja#ChangesintheHTML
I replaced the doctype and <html> as described.
I also searched and found the note about `# extends "layout.html"`,
but didn't immediately see where that should go, so I switched to:
https://trac.edgewall.org/wiki/TracDev/PortingFromGenshiToJinja/Example#Conversionexample
Since the templates looked so simple, I naively assumed that I was more
or less done and tried running it.
But nothing showed up; not even an error.
So back to reading a tiny bit more, I found out about `#block title` and
`#block content`.
Still nothing, back to reading. I added `${ super() }`.
Finally, I got some errors about `idx`, so I remembered I had to turn
`py:for` into `# for`.
Error about `enumerate`. Found:
https://trac.edgewall.org/wiki/TracDev/PortingFromGenshiToJinja#Noneedforenumerate
But nothing about "odd / even".
The linked site has an example though: `loop.cycle('odd',
'even')`http://jinja.pocoo.org/docs/dev/templates/#for
Now almost everything seemed to work, except `$item.resource_href `
showed up on the page uninterpreted.
I remembered that I had to use `${item.resource_href}` instead.

That was all I think. Everything seems to work now. That was not too
bad. Maybe I'll try another one later.

Peter


Just in case, the resulting templates:

# extends "layout.html"
<!DOCTYPE html>
<html>
<head>
<title>
# block title
Notification: ${message['subject']} ${ super() }
# endblock title
</title>
</head>
<body>
# block content
<h2>Notification: ${message['subject']}</h2>
<pre>${message['body']}</pre>

${ super() }

# endblock content
</body>
</html>


# extends "layout.html"
<!DOCTYPE html>
<html>
<head>
<title>
# block title
Notifications ${ super() }
# endblock title
</title>
</head>

<body>
# block content
<div id="content">
<h2>Notifications</h2>

<table class="listing">
<thead>
<tr class="trac-columns"><th>Resource:</th><th>Details:</th></tr>
</thead>
<tbody>
# for item in items:
<tr class="${loop.cycle('odd', 'even')}">
<td><a
href="${item.resource_href}">${item['resource']}</a></td>
<td><a href="${item.details_href}">${item['details']}</a></td>
</tr>
# endfor
</tbody>
</table>
</div>
<div class="buttons">
<form id="deletenotifications" action="" method="post">
<input type="hidden" name="action" value="deletenotifications" />
<input type="submit" class="trac-disable-on-submit"
value="${_('Delete notifications')}" />
</form>
</div>

${ super() }

# endblock content
</body>
</html>
--
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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
Peter Suter
2017-01-15 20:21:31 UTC
Permalink
Post by Christian Boos
Also, what would be great is if you (or anyone else for that matter)
could try to convert some plugins templates to Jinja2. That way, you
could tell me what is not clear in the migration guide, or simply still
missing from it.
Everything seems to work now. Maybe I'll try another one later.
Not quite everything. Apparently I need to add
`${jmacros.form_token_input()}` to every <form method="post">.
This is described here:
https://trac.edgewall.org/wiki/TracDev/PortingFromGenshiToJinja#Producingthecorrectcontentdirectly
Shouldn't this be moved out of the `ITemplateStreamFilter` section?

(Also I forgot to change
`from genshi.builder import tag, Markup`
to
`from trac.util.html import tag, Markup`
since the former still works at the moment.)

Next I converted https://trac-hacks.org/wiki/CardsPlugin
This plugin only has one simple template, but calls `render_template()`.
According to
https://trac.edgewall.org/wiki/TracDev/PortingFromGenshiToJinja#Generatingcontent
When one wants to directly render a template, the Chrome component
{{{
return Chrome(self.env).render_template(
req, 'query_results.html', data, None, fragment=True)
}}}
But actually it seems for Jinja templates one must now change this to:
{{{
return Chrome(self.env).render_template(
req, 'query_results.html', data, {'fragment': True})
}}}
no?

I also converted these three:
https://trac-hacks.org/wiki/PullRequestsPlugin
https://trac-hacks.org/wiki/MailArchivePlugin
https://trac-hacks.org/wiki/TimeTrackingPlugin
which needed also <xi:include />, # if, # with, # block head which are
all covered nicely in your guide.
I had to look up how admin panels should work (# block adminpanel, etc.)
in another example template, but that seems OK.


A small CSS quirk: When the "wiki expander" is set to "narrow", the
title underline of e.g. `[[TicketQuery?]]` strangely overlaps the content.

(BTW why are all the titles red now? I rather liked that red indicated
"link".)


Peter
--
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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
Christian Boos
2017-01-15 22:44:40 UTC
Permalink
Hello Peter,

Thanks for persisting ;-)
Post by Peter Suter
Post by Christian Boos
Also, what would be great is if you (or anyone else for that matter)
could try to convert some plugins templates to Jinja2. That way, you
could tell me what is not clear in the migration guide, or simply still
missing from it.
Everything seems to work now. Maybe I'll try another one later.
Not quite everything. Apparently I need to add
`${jmacros.form_token_input()}` to every <form method="post">.
https://trac.edgewall.org/wiki/TracDev/PortingFromGenshiToJinja#Producingthecorrectcontentdirectly
Shouldn't this be moved out of the `ITemplateStreamFilter` section?
Maybe yes, the connection with the `ITemplateStreamFilter` is only
indirect: both were taking benefit of Genshi stream filters.
Post by Peter Suter
(Also I forgot to change
`from genshi.builder import tag, Markup`
to
`from trac.util.html import tag, Markup`
since the former still works at the moment.)
Yes, the nice thing is that `from trac.util.html import tag, Markup`
works with all recent versions of Trac, so we can aggressively promote
this change!
Post by Peter Suter
Next I converted https://trac-hacks.org/wiki/CardsPlugin
This plugin only has one simple template, but calls `render_template()`.
According to
https://trac.edgewall.org/wiki/TracDev/PortingFromGenshiToJinja#Generatingcontent
When one wants to directly render a template, the Chrome component
{{{
return Chrome(self.env).render_template(
req, 'query_results.html', data, None, fragment=True)
}}}
{{{
return Chrome(self.env).render_template(
req, 'query_results.html', data, {'fragment': True})
}}}
no?
Right, forgot to update that one. The trac.web.chrome API changed
recently during the preparation for the merge.
Post by Peter Suter
https://trac-hacks.org/wiki/PullRequestsPlugin
https://trac-hacks.org/wiki/MailArchivePlugin
https://trac-hacks.org/wiki/TimeTrackingPlugin
which needed also <xi:include />, # if, # with, # block head which are
all covered nicely in your guide.
I had to look up how admin panels should work (# block adminpanel, etc.)
in another example template, but that seems OK.
I'll have a look and see if I can give some hints for some improvements.
One thing I've seen in the example you posted in your previous mail was
that you wrote plenty of `${x['key']}`. You can use the more compact
`${x.key}`, this will work as well
(http://jinja.pocoo.org/docs/2.9/templates/#variables).

Also, if some of your plugins are localized, you can now also play with
that part, I've finalized the l10n support just now.

See for the jinja2-trunk-r15341 branch:

- [3dd429ae/cboos.git] (#12639): rework the l10n support for Jinja2

(https://trac.edgewall.org/changeset/3dd429ae/cboos.git)


And the proof of concept with a new branch for the SpamFilter:

- [15351] (#12639) Apply basic Jinja2 compatibility patch.
- [15352] SpamFilter (jinja2): add support for l10n with Jinja2
- [15353] SpamFilter (jinja2): convert one template

(https://trac.edgewall.org/log/plugins/trunk/spam-filter-jinja2)
Post by Peter Suter
A small CSS quirk: When the "wiki expander" is set to "narrow", the
title underline of e.g. `[[TicketQuery?]]` strangely overlaps the content.
I'll check that, but yes, I'm probably not done with tweaking this.
Post by Peter Suter
(BTW why are all the titles red now? I rather liked that red indicated
"link".)
Well, just because the Trac logo is red, and we have red already for
links, so it mixes well ;-) But I'm open to changing that. I'll
experiment with a darker red.

-- 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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
RjOllos
2017-01-18 02:44:59 UTC
Permalink
Post by Christian Boos
Hello everybody,
I took some steps to finalize my migration of Trac to the Jinja2
template engine, and the results are now visible in several places.
https://svn.edgewall.org/git/trac/devs/cboos
https://github.com/cboos/trac.git
https://trac.edgewall.org/testing
(this one also contains my wiki-restyling changes, for good measure)
The #12649 ticket can be used to report trivial issues you'd find with
Jinja2 (#12640 for the restyling ones). Don't hesitate to create new
tickets for more serious issues, if any.
It would be nice to get some feedback at this point. You can contribute
by either reviewing the code, or simply by playing around in
t.e.o/testing and report any glitches you'd find there.
If you have any questions, the mailing list would probably be more
appropriate. Be sure to first go through the following wiki pages, as
- https://trac.edgewall.org/wiki/TracDev/Proposals/Jinja
- https://trac.edgewall.org/wiki/TracDev/PortingFromGenshiToJinja
- the branch keeps Genshi templates and Jinja2 templates side by side
(e.g. search.html and jsearch.html); that was helpful during the
development, and can still be useful while reviewing the branch, but I'm
going to go back to the original names soon (jsearch.html ->
search.html), moving a step closer to the merge.
- I haven't tested the i18n support for Jinja2 templates in plugins
yet; this is what I'm going to work on next
Also, I want to address the concerns raised last year about the backward
compatibility with Genshi. I propose to use the whole 1.4.x series as a
transition period, during which both template engines will be supported.
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. There's slightly more to it than this,
but you get the idea.
I've also taken into account the plugins which inject content in
generated pages via the ITemplateStreamFilter interface. That way is
deprecated in favor of using JavaScript for that purpose, but the
interface will nevertheless be supported during the transition period as
well.
One problem is that this ITemplateStreamFilter support mostly works, but
is not perfect, and of course kills most of the speed improvement
brought by the switch to Jinja2. Consider this as an incentive to
migrate such plugins! If people are really motivated to fix/enhance this
aspect, feel free to do so, but I feel it's somewhat counter-productive
to invest more energy there.
I hope we can soon finalize the integration of this effort, before we
make more serious moves on the Python3 topic (also, Jinja2 brings in six
as a dependency).
-- Christian
One more possible minor quirk: the level 3 headings from the TracIni macro
extend outside the #wikipage div:
https://trac.edgewall.org/testing/wiki/TracIni#attachment-max_zip_size-option

- Ryan
--
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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
RjOllos
2017-01-21 02:42:36 UTC
Permalink
I like the border around the h3 when linking to a comment. I noticed two
things:

1. The Reply button overlaps the border of the h3 (1).
2. Some padding between the border and "Description" would look nicer (2).

(1) https://trac.edgewall.org/testing/ticket/9#comment:3
(2) https://trac.edgewall.org/testing/ticket/9#comment:description
--
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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
RjOllos
2017-01-21 08:27:19 UTC
Permalink
Through inspecting the errors in the logs, I stumbled across an issue. The
log for /trunk/setup_wininst.bmp renders fine in 1.3.1 (1), but raises a
ValueError in the testing instance (2).

- Ryan

(1)
https://trac.edgewall.org/demo-1.3.1/log/trunk/setup_wininst.bmp?format=changelog&rev=1017&limit=100&mode=stop_on_copy
(2)
https://trac.edgewall.org/testing/log/trunk/setup_wininst.bmp?format=changelog&rev=1017&limit=100&mode=stop_on_copy
--
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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
RjOllos
2017-01-27 12:15:03 UTC
Permalink
Post by RjOllos
Through inspecting the errors in the logs, I stumbled across an issue. The
log for /trunk/setup_wininst.bmp renders fine in 1.3.1 (1), but raises a
ValueError in the testing instance (2).
- Ryan
(1)
https://trac.edgewall.org/demo-1.3.1/log/trunk/setup_wininst.bmp?format=changelog&rev=1017&limit=100&mode=stop_on_copy
(2)
https://trac.edgewall.org/testing/log/trunk/setup_wininst.bmp?format=changelog&rev=1017&limit=100&mode=stop_on_copy
<https://www.google.com/url?q=https%3A%2F%2Ftrac.edgewall.org%2Ftesting%2Flog%2Ftrunk%2Fsetup_wininst.bmp%3Fformat%3Dchangelog%26rev%3D1017%26limit%3D100%26mode%3Dstop_on_copy&sa=D&sntz=1&usg=AFQjCNEJE4VgpkfesQbWv3Ojj3oGPf9BTw>
Another traceback seen in the logs, unsure if it's related to the
"ValueError: Can't send unicode content" issue:

2017-01-26 09:57:35,825 Trac[main] ERROR: Internal Server Error:
<RequestWithSession "GET '/roadmap?format=ics'">, referrer None
Traceback (most recent call last):
File "build/bdist.linux-x86_64/egg/trac/web/main.py", line 630, in
_dispatch_request
dispatcher.dispatch(req)
File "build/bdist.linux-x86_64/egg/trac/web/main.py", line 252, in
dispatch
resp = chosen_handler.process_request(req)
File "build/bdist.linux-x86_64/egg/trac/ticket/roadmap.py", line 511, in
process_request
self._render_ics(req, milestones)
File "build/bdist.linux-x86_64/egg/trac/ticket/roadmap.py", line 598, in
_render_ics
self.env.project_name + ' - ' + _("Roadmap"))
File "build/bdist.linux-x86_64/egg/trac/ticket/roadmap.py", line 578, in
write_prop
buf.write(text[:75] + CRLF)
TypeError: 'unicode' does not have the buffer interface

- Ryan
--
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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
Christian Boos
2017-01-27 12:38:45 UTC
Permalink
Post by RjOllos
Through inspecting the errors in the logs, I stumbled across an
issue. The log for /trunk/setup_wininst.bmp renders fine in 1.3.1
(1), but raises a ValueError in the testing instance (2).
- Ryan
(1)
https://trac.edgewall.org/demo-1.3.1/log/trunk/setup_wininst.bmp?format=changelog&rev=1017&limit=100&mode=stop_on_copy
<https://trac.edgewall.org/demo-1.3.1/log/trunk/setup_wininst.bmp?format=changelog&rev=1017&limit=100&mode=stop_on_copy>
(2)
https://trac.edgewall.org/testing/log/trunk/setup_wininst.bmp?format=changelog&rev=1017&limit=100&mode=stop_on_copy
<https://www.google.com/url?q=https%3A%2F%2Ftrac.edgewall.org%2Ftesting%2Flog%2Ftrunk%2Fsetup_wininst.bmp%3Fformat%3Dchangelog%26rev%3D1017%26limit%3D100%26mode%3Dstop_on_copy&sa=D&sntz=1&usg=AFQjCNEJE4VgpkfesQbWv3Ojj3oGPf9BTw>
Another traceback seen in the logs, unsure if it's related to the
<RequestWithSession "GET '/roadmap?format=ics'">, referrer None
File "build/bdist.linux-x86_64/egg/trac/web/main.py", line 630, in
_dispatch_request
dispatcher.dispatch(req)
File "build/bdist.linux-x86_64/egg/trac/web/main.py", line 252, in
dispatch
resp = chosen_handler.process_request(req)
File "build/bdist.linux-x86_64/egg/trac/ticket/roadmap.py", line 511,
in process_request
self._render_ics(req, milestones)
File "build/bdist.linux-x86_64/egg/trac/ticket/roadmap.py", line 598,
in _render_ics
self.env.project_name + ' - ' + _("Roadmap"))
File "build/bdist.linux-x86_64/egg/trac/ticket/roadmap.py", line 578,
in write_prop
buf.write(text[:75] + CRLF)
TypeError: 'unicode' does not have the buffer interface
No, it's not, plain trunk has it as well:

https://trac.edgewall.org/demo-1.3/roadmap?format=ics


The issue with /trunk/setup_wininst.bmp should be fixed in latest
rebase of the jinja2 branch (jinja2-trunk-r15379), but I didn't have
time to refresh +testing yet.

-- 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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
RjOllos
2017-02-07 15:45:06 UTC
Permalink
Here's a traceback from the demo server logs:

2017-02-06 09:17:24,987 Trac[main] ERROR: [85.115.16.62] Internal Server
Error: <RequestWithSession "GET
'/changeset/022a729ef43190cdf3ae00663c5e24df41d2b328/cboos.git?annotate=%2Ftrac%2Fhtdocs%2Fcss%2Ftrac.css'">,
referrer
'https://trac.edgewall.org/testing/browser/cboos.git/trac/htdocs/css/trac.css?annotate=blame'
Traceback (most recent call last):
File "build/bdist.linux-x86_64/egg/trac/web/main.py", line 630, in
_dispatch_request
dispatcher.dispatch(req)
File "build/bdist.linux-x86_64/egg/trac/web/main.py", line 252, in
dispatch
resp = chosen_handler.process_request(req)
File
"build/bdist.linux-x86_64/egg/trac/versioncontrol/web_ui/changeset.py",
line 349, in process_request
self._render_html(req, repos, chgset, restricted, data)
File
"build/bdist.linux-x86_64/egg/trac/versioncontrol/web_ui/changeset.py",
line 666, in _render_html
req, 'changeset_content.html', data)
File "build/bdist.linux-x86_64/egg/trac/web/chrome.py", line 1485, in
generate_fragment
return self.generate_template_stream(template, data, text)
File "build/bdist.linux-x86_64/egg/trac/web/chrome.py", line 1571, in
generate_template_stream
bytes = template.render(data).encode('utf-8')
File
"/usr/local/virtualenv/src/trac-testing/.eggs/Jinja2-2.9.4-py2.7.egg/jinja2/environment.py",
line 1008, in render
return self.environment.handle_exception(exc_info, True)
File
"/usr/local/virtualenv/src/trac-testing/.eggs/Jinja2-2.9.4-py2.7.egg/jinja2/environment.py",
line 780, in handle_exception
reraise(exc_type, exc_value, tb)
File
"/tmp/.eggs-testing/Trac-1.3.2.dev0-py2.7.egg-tmp/trac/versioncontrol/templates/changeset_content.html",
line 309, in top-level template code
# call(note, page) jmacros.wikihelp('TracChangeset'):
File
"/usr/local/virtualenv/src/trac-testing/.eggs/Jinja2-2.9.4-py2.7.egg/jinja2/environment.py",
line 430, in getattr
return getattr(obj, attribute)
UndefinedError: 'jmacros' is undefined

- Ryan
--
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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
Christian Boos
2017-02-07 17:33:08 UTC
Permalink
Post by RjOllos
...
"/tmp/.eggs-testing/Trac-1.3.2.dev0-py2.7.egg-tmp/trac/versioncontrol/templates/changeset_content.html",
line 309, in top-level template code
File
"/usr/local/virtualenv/src/trac-testing/.eggs/Jinja2-2.9.4-py2.7.egg/jinja2/environment.py",
line 430, in getattr
return getattr(obj, attribute)
UndefinedError: 'jmacros' is undefined
This was already fixed in r15500. I updated /demo-1.3 to latest trunk
(r15506).

-- 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+***@googlegroups.com.
To post to this group, send email to trac-***@googlegroups.com.
Visit this group at https://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/d/optout.
Loading...