Meta issue: #1843738: [meta] Convert views module to Twig
#1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch

Steps to test:

1. Create a few articles with title and body
2. Visit rss.xml

Profiling results

=== views-view-row-rss-8.x..views-view-row-rss-8.x compared (51990c6b6ce43..51990d1de2dc4):

ct  : 63,239|63,239|0|0.0%
wt  : 301,459|301,096|-363|-0.1%
cpu : 285,428|285,449|21|0.0%
mu  : 13,096,408|13,096,408|0|0.0%
pmu : 13,230,208|13,230,208|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51990c6b6ce43&...

=== views-view-row-rss-8.x..views-view-row-rss-1843758-34 compared (51990c6b6ce43..51990ceb6ce00):

ct  : 63,239|63,540|301|0.5%
wt  : 301,459|301,009|-450|-0.1%
cpu : 285,428|284,199|-1,229|-0.4%
mu  : 13,096,408|13,126,136|29,728|0.2%
pmu : 13,230,208|13,259,232|29,024|0.2%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51990c6b6ce43&...

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joelpittet’s picture

Status: Active » Needs review
FileSize
610 bytes
mbrett5062’s picture

Issue tags: +VDC

Tagging.

jpamental’s picture

Converted to core patch. Removes views-view-row-rss.tpl.php, adds views-view-row-rss.html.twig

jpamental’s picture

Project: » Drupal core
Version: » 8.x-dev
Component: Twig templates conversion (front-end branch) » views.module

Reassigning to Drupal core.

joelpittet’s picture

Issue tags: +Twig

Adding twig tag

joelpittet’s picture

Title: Convert views/theme/views-view-row-rss.tpl.php to twig » Convert views/templates/views-view-row-rss.tpl.php to twig
dawehner’s picture

This looks pretty solid.

joelpittet’s picture

Issue tags: +@todo clean-up
FileSize
2.48 KB
2.74 KB

Did some Doc Cleanup to standards.

And replaced $vars with $variables in the preprocess.

There are some @todos for the variables that are going into the preprocess.

dawehner’s picture

+++ b/core/modules/views/views.theme.incundefined
@@ -922,17 +922,25 @@ function template_preprocess_views_view_rss(&$vars) {
+  $view     = &$variables['view'];
+  $options  = &$variables['options'];
+  $item     = &$variables['row'];

Let's drop the unneeded spaces here, as well as the & references.

joelpittet’s picture

Status: Needs review » Needs work

@dawehner good call. Changing this to needs work for the cleanup.

star-szr’s picture

Issue tags: +Needs manual testing

Tagging.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
2.26 KB
2.76 KB

Cleaned this up bit, reverted the $variables bit so it's easier to read the conversion changes.

Status: Needs review » Needs work
Issue tags: -Needs manual testing, -Twig, -VDC, -@todo clean-up

The last submitted patch, twig-views-view-row-rss-1843758-12.patch, failed testing.

star-szr’s picture

Status: Needs work » Needs review
Issue tags: +Needs manual testing, +Twig, +VDC, +@todo clean-up
thedavidmeister’s picture

Status: Needs review » Needs work

Nitpicks for #12:

+ * Default theme implementation to display a item in an views RSS feed.

"a" not "an" and "views" needs a capital V.

+ * Prepares variables for all RSS rows.

Maybe something like "Prepares variables for Views RSS item templates."

This issue summary needs some steps written for manual testing and somebody to actually do the testing.

star-szr’s picture

Issue tags: +Novice

Creating a revised patch and interdiff based on #15 would be a good novice task, tagging.

If you'd like to work on this, please assign the issue to yourself. If you get stuck, drop by #drupal-twig on IRC or post your questions here. Thanks!

tlattimore’s picture

Here is a patch with the doc updates requested in #15.

tlattimore’s picture

Status: Needs work » Needs review

blast. Forgot to tag for review.

joelpittet’s picture

Issue tags: -Novice

@tlattimore Nice work. Could we get someone to do some manual testing and possibly someone from VDC to fill in the 3 @todo doc stuff?

dawehner’s picture

+++ b/core/modules/views/views.theme.incundefined
@@ -922,12 +922,20 @@ function template_preprocess_views_view_rss(&$vars) {
+  $view = &$vars['view'];
...
+  $item = &$vars['row'];

The & is not needed, because we do have objects here (views has been written fro php4 comp ability back in d6).

star-szr’s picture

Status: Needs review » Needs work
Issue tags: +Novice

CNW for #20 and changing the @param to be vars instead of variables for now:

+++ b/core/modules/views/views.theme.incundefined
@@ -922,12 +922,20 @@ function template_preprocess_views_view_rss(&$vars) {
+ * @param array $variables
...
 function template_preprocess_views_view_row_rss(&$vars) {
joelpittet’s picture

Status: Needs work » Needs review
Issue tags: -Novice, -@todo clean-up
FileSize
2.18 KB
1.53 KB

@dawehner removed the variables that weren't used at all and &refs and documented 'row'.

Fixed a small typo too on the twig file.

Status: Needs review » Needs work
Issue tags: -Needs manual testing, -Twig, -VDC

The last submitted patch, 1843758-22-views-view-row-rss.patch, failed testing.

star-szr’s picture

Status: Needs work » Needs review
Issue tags: +Needs manual testing, +Twig, +VDC
thedavidmeister’s picture

Status: Needs review » Needs work

nitpick:

+ * - item_elements: RSS item elements (pubDate, creator, guid).

When we have an item with properties in a Twig template we usually document it like:

- elements: Summary about elements. Each element contains:
- foo: summary of element.foo
- bar: summary of element.bar

joelpittet’s picture

Issue tags: +Novice

How about:
* - item_elements: RSS item elements rendered as XML (pubDate, creator, guid).?
As this is using a format_xml_elements() to get build the rendered output.

thedavidmeister’s picture

#26 - yeah, IMO that little bit of extra clarification is better. I didn't realise that these elements were XML and not properties of item_elements, so I'd bet an end-user could be similarly confused.

star-szr’s picture

Issue tags: +needs profiling

Tagging for profiling.

joelpittet’s picture

Status: Needs work » Needs review
Issue tags: -Novice
FileSize
788 bytes
2.2 KB
thedavidmeister’s picture

Assigned: Unassigned » thedavidmeister

i'll have a shot at manual testing this as i think the code is neat now :)

thedavidmeister’s picture

Issue summary: View changes

Updated issue summary.

thedavidmeister’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs manual testing

HEAD: whitespace normalized for easy diffing:

<?xml version="1.0" encoding="utf-8" ?>
<rss version="2.0" xml:base="http://localhost:8888/d8html/" xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:content="http://purl.org/rss/1.0/modules/content/" xmlns:foaf="http://xmlns.com/foaf/0.1/" xmlns:og="http://ogp.me/ns#" xmlns:rdfs="http://www.w3.org/2000/01/rdf-schema#" xmlns:sioc="http://rdfs.org/sioc/ns#" xmlns:sioct="http://rdfs.org/sioc/types#" xmlns:skos="http://www.w3.org/2004/02/skos/core#" xmlns:xsd="http://www.w3.org/2001/XMLSchema#">
<channel>
<title>localhost</title>
<link>http://localhost:8888/d8html/</link>
<description>
</description>
<language>en</language>
<item>
<title>asdfasdfasdf</title>
<link>http://localhost:8888/d8html/node/2</link>
<description>&lt;div  class=&quot;field field-name-field-tags field-type-taxonomy-term-reference field-label-above clearfix&quot; data-edit-id=&quot;node/2/field_tags/und/rss&quot;&gt;&lt;h3 class=&quot;field-label&quot;&gt;Tags: &lt;/h3&gt;&lt;ul class=&quot;links&quot;&gt;&lt;li class=&quot;taxonomy-term-reference-0&quot; rel=&quot;dc:subject&quot;&gt;&lt;a href=&quot;/d8html/taxonomy/term/2&quot; typeof=&quot;skos:Concept&quot; property=&quot;rdfs:label skos:prefLabel&quot; datatype=&quot;&quot;&gt;;lkj&lt;/a&gt;&lt;/li&gt;&lt;/ul&gt;&lt;/div&gt;&lt;div class=&quot;field field-name-body field-type-text-with-summary field-label-hidden&quot; data-edit-id=&quot;node/2/body/und/rss&quot;&gt;&lt;div class=&quot;field-items&quot;&gt;&lt;div class=&quot;field-item even&quot; property=&quot;content:encoded&quot;&gt;&lt;p&gt;a;lskdjf;lakdjsf&lt;/p&gt;&lt;/div&gt;&lt;/div&gt;&lt;/div&gt;</description>
<pubDate>Mon, 13 May 2013 11:22:56 +0000</pubDate>
<dc:creator>admin</dc:creator>
<guid isPermaLink="false">2 at http://localhost:8888/d8html</guid>
<comments>http://localhost:8888/d8html/node/2#comments</comments>
</item>
<item>
<title>asdfasdf</title>
<link>http://localhost:8888/d8html/node/1</link>
<description>&lt;div  class=&quot;field field-name-field-tags field-type-taxonomy-term-reference field-label-above clearfix&quot; data-edit-id=&quot;node/1/field_tags/und/rss&quot;&gt;&lt;h3 class=&quot;field-label&quot;&gt;Tags: &lt;/h3&gt;&lt;ul class=&quot;links&quot;&gt;&lt;li class=&quot;taxonomy-term-reference-0&quot; rel=&quot;dc:subject&quot;&gt;&lt;a href=&quot;/d8html/taxonomy/term/1&quot; typeof=&quot;skos:Concept&quot; property=&quot;rdfs:label skos:prefLabel&quot; datatype=&quot;&quot;&gt;asdf&lt;/a&gt;&lt;/li&gt;&lt;/ul&gt;&lt;/div&gt;&lt;div class=&quot;field field-name-body field-type-text-with-summary field-label-hidden&quot; data-edit-id=&quot;node/1/body/und/rss&quot;&gt;&lt;div class=&quot;field-items&quot;&gt;&lt;div class=&quot;field-item even&quot; property=&quot;content:encoded&quot;&gt;&lt;p&gt;;lkj;ljk&lt;/p&gt;&lt;/div&gt;&lt;/div&gt;&lt;/div&gt;</description>
<pubDate>Mon, 13 May 2013 11:22:16 +0000</pubDate>
<dc:creator>admin</dc:creator>
<guid isPermaLink="false">1 at http://localhost:8888/d8html</guid>
<comments>http://localhost:8888/d8html/node/1#comments</comments>
</item>
</channel>
</rss>

with patch #29, whitespace normalized again:

<?xml version="1.0" encoding="utf-8" ?>
<rss version="2.0" xml:base="http://localhost:8888/d8html/" xmlns:dc="http://purl.org/dc/elements/1.1/" xmlns:content="http://purl.org/rss/1.0/modules/content/" xmlns:foaf="http://xmlns.com/foaf/0.1/" xmlns:og="http://ogp.me/ns#" xmlns:rdfs="http://www.w3.org/2000/01/rdf-schema#" xmlns:sioc="http://rdfs.org/sioc/ns#" xmlns:sioct="http://rdfs.org/sioc/types#" xmlns:skos="http://www.w3.org/2004/02/skos/core#" xmlns:xsd="http://www.w3.org/2001/XMLSchema#">
<channel>
<title>localhost</title>
<link>http://localhost:8888/d8html/</link>
<description>
</description>
<language>en</language>
<item>
<title>asdfasdfasdf</title>
<link>http://localhost:8888/d8html/node/2</link>
<description>&lt;div  class=&quot;field field-name-field-tags field-type-taxonomy-term-reference field-label-above clearfix&quot; data-edit-id=&quot;node/2/field_tags/und/rss&quot;&gt;&lt;h3 class=&quot;field-label&quot;&gt;Tags: &lt;/h3&gt;&lt;ul class=&quot;links&quot;&gt;&lt;li class=&quot;taxonomy-term-reference-0&quot; rel=&quot;dc:subject&quot;&gt;&lt;a href=&quot;/d8html/taxonomy/term/2&quot; typeof=&quot;skos:Concept&quot; property=&quot;rdfs:label skos:prefLabel&quot; datatype=&quot;&quot;&gt;;lkj&lt;/a&gt;&lt;/li&gt;&lt;/ul&gt;&lt;/div&gt;&lt;div class=&quot;field field-name-body field-type-text-with-summary field-label-hidden&quot; data-edit-id=&quot;node/2/body/und/rss&quot;&gt;&lt;div class=&quot;field-items&quot;&gt;&lt;div class=&quot;field-item even&quot; property=&quot;content:encoded&quot;&gt;&lt;p&gt;a;lskdjf;lakdjsf&lt;/p&gt;&lt;/div&gt;&lt;/div&gt;&lt;/div&gt;</description>
<pubDate>Mon, 13 May 2013 11:22:56 +0000</pubDate>
<dc:creator>admin</dc:creator>
<guid isPermaLink="false">2 at http://localhost:8888/d8html</guid>
<comments>http://localhost:8888/d8html/node/2#comments</comments>
</item>
<item>
<title>asdfasdf</title>
<link>http://localhost:8888/d8html/node/1</link>
<description>&lt;div  class=&quot;field field-name-field-tags field-type-taxonomy-term-reference field-label-above clearfix&quot; data-edit-id=&quot;node/1/field_tags/und/rss&quot;&gt;&lt;h3 class=&quot;field-label&quot;&gt;Tags: &lt;/h3&gt;&lt;ul class=&quot;links&quot;&gt;&lt;li class=&quot;taxonomy-term-reference-0&quot; rel=&quot;dc:subject&quot;&gt;&lt;a href=&quot;/d8html/taxonomy/term/1&quot; typeof=&quot;skos:Concept&quot; property=&quot;rdfs:label skos:prefLabel&quot; datatype=&quot;&quot;&gt;asdf&lt;/a&gt;&lt;/li&gt;&lt;/ul&gt;&lt;/div&gt;&lt;div class=&quot;field field-name-body field-type-text-with-summary field-label-hidden&quot; data-edit-id=&quot;node/1/body/und/rss&quot;&gt;&lt;div class=&quot;field-items&quot;&gt;&lt;div class=&quot;field-item even&quot; property=&quot;content:encoded&quot;&gt;&lt;p&gt;;lkj;ljk&lt;/p&gt;&lt;/div&gt;&lt;/div&gt;&lt;/div&gt;</description>
<pubDate>Mon, 13 May 2013 11:22:16 +0000</pubDate>
<dc:creator>admin</dc:creator>
<guid isPermaLink="false">1 at http://localhost:8888/d8html</guid>
<comments>http://localhost:8888/d8html/node/1#comments</comments>
</item>
</channel>
</rss>
thedavidmeister’s picture

Assigned: thedavidmeister » Unassigned
joelpittet’s picture

Doc updates, missing preprocess docs. Removed $options because they weren't being used and =& references.

joelpittet’s picture

Whoops, wrong one. Back to #29

star-szr’s picture

Assigned: Unassigned » star-szr

Dibs on this for profiling.

star-szr’s picture

Assigned: star-szr » Unassigned
Issue tags: -needs profiling

Added this to template_preprocess_views_view_rows_rss() to isolate the profiling to only this template conversion (not comparing loading Twig itself):

  $node = node_load(1);
  $vars['description'] .= theme('node', node_view($node));

Benchmarked /rss.xml.

=== views-view-row-rss-8.x..views-view-row-rss-8.x compared (51990c6b6ce43..51990d1de2dc4):

ct  : 63,239|63,239|0|0.0%
wt  : 301,459|301,096|-363|-0.1%
cpu : 285,428|285,449|21|0.0%
mu  : 13,096,408|13,096,408|0|0.0%
pmu : 13,230,208|13,230,208|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51990c6b6ce43&...

=== views-view-row-rss-8.x..views-view-row-rss-1843758-34 compared (51990c6b6ce43..51990ceb6ce00):

ct  : 63,239|63,540|301|0.5%
wt  : 301,459|301,009|-450|-0.1%
cpu : 285,428|284,199|-1,229|-0.4%
mu  : 13,096,408|13,126,136|29,728|0.2%
pmu : 13,230,208|13,259,232|29,024|0.2%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51990c6b6ce43&...

star-szr’s picture

Issue summary: View changes

added testing steps

star-szr’s picture

Re-ran with 1000 loops instead of 100 to confirm:

=== views-view-row-rss-8.x..views-view-row-rss-8.x compared (519a6b3fa9c9c..519a6c2cb620c):

ct  : 63,276|63,276|0|0.0%
wt  : 307,066|306,285|-781|-0.3%
cpu : 291,246|290,490|-756|-0.3%
mu  : 13,533,648|13,533,648|0|0.0%
pmu : 13,668,200|13,668,200|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519a6b3fa9c9c&...

=== views-view-row-rss-8.x..views-view-row-rss-1843758-34 compared (519a6b3fa9c9c..519a6f8ea14bd):

ct  : 63,276|63,577|301|0.5%
wt  : 307,066|305,878|-1,188|-0.4%
cpu : 291,246|290,087|-1,159|-0.4%
mu  : 13,533,648|13,563,472|29,824|0.2%
pmu : 13,668,200|13,697,352|29,152|0.2%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519a6b3fa9c9c&...

alexpott’s picture

Title: Convert views/templates/views-view-row-rss.tpl.php to twig » [READY] Convert views/templates/views-view-row-rss.tpl.php to twig
Status: Reviewed & tested by the community » Closed (duplicate)
alexpott’s picture

Title: [READY] Convert views/templates/views-view-row-rss.tpl.php to twig » Convert views/templates/views-view-row-rss.tpl.php to twig
Status: Closed (duplicate) » Fixed

Committed 7016229 and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Add profiling results