Files: 
CommentFileSizeAuthor
#81 1843746-views-view-field-81.patch3.24 KBtlattimore
PASSED: [[SimpleTest]]: [MySQL] 55,856 pass(es).
[ View ]
#81 interdiff.txt564 bytestlattimore
#73 1843746-73-views-view-field.patch3.27 KBjoelpittet
PASSED: [[SimpleTest]]: [MySQL] 56,012 pass(es).
[ View ]
#73 interdiff.txt1.58 KBjoelpittet
#69 interdiff.txt2.94 KBshanethehat
#69 twig-views-view-field-1843746-69.patch3.25 KBshanethehat
PASSED: [[SimpleTest]]: [MySQL] 56,108 pass(es).
[ View ]
#66 twig-views-view-field-1843746-66.patch9.05 KBKevin Morse
PASSED: [[SimpleTest]]: [MySQL] 55,466 pass(es).
[ View ]
#64 twig-views-view-field-1843746-64.patch3.23 KBshanethehat
PASSED: [[SimpleTest]]: [MySQL] 55,760 pass(es).
[ View ]
#60 interdiff.txt556 bytesjoelpittet
#60 1843746-60-views-view-field.patch3.93 KBjoelpittet
PASSED: [[SimpleTest]]: [MySQL] 55,399 pass(es).
[ View ]
#57 interdiff.txt532 bytesjoelpittet
#57 1843746-57-views-view-field.patch3.9 KBjoelpittet
FAILED: [[SimpleTest]]: [MySQL] 55,349 pass(es), 61 fail(s), and 0 exception(s).
[ View ]
#46 1843746-46.patch3.93 KBCottser
PASSED: [[SimpleTest]]: [MySQL] 55,399 pass(es).
[ View ]
#46 interdiff.txt407 bytesCottser
#39 1843746-39.patch730 bytesthedavidmeister
FAILED: [[SimpleTest]]: [MySQL] 54,850 pass(es), 1 fail(s), and 0 exception(s).
[ View ]
#35 drupal-1843746.patch717 bytesdawehner
FAILED: [[SimpleTest]]: [MySQL] 54,828 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#28 interdiff.txt1.18 KBjoelpittet
#28 twig-views-view-field-1843746-28.patch3.94 KBjoelpittet
PASSED: [[SimpleTest]]: [MySQL] 54,439 pass(es).
[ View ]
#18 drupal_twig_views_field_template-1843746-18.patch3.98 KBjastraat
FAILED: [[SimpleTest]]: [MySQL] 54,892 pass(es), 61 fail(s), and 21 exception(s).
[ View ]
#15 1843746-15.patch3.89 KBdamiankloip
FAILED: [[SimpleTest]]: [MySQL] 53,785 pass(es), 63 fail(s), and 21 exception(s).
[ View ]
#15 interdiff.txt546 bytesdamiankloip
#13 1843746-views-view-field-13.patch3.35 KBtrrroy
FAILED: [[SimpleTest]]: [MySQL] 53,677 pass(es), 144 fail(s), and 6 exception(s).
[ View ]
#13 interdiff.txt1.82 KBtrrroy
#11 1843746-views-view-field-11.patch3.23 KBtrrroy
FAILED: [[SimpleTest]]: [MySQL] 53,681 pass(es), 143 fail(s), and 6 exception(s).
[ View ]
#9 1843746-views-view-field-9.patch2.09 KBtlattimore
PASSED: [[SimpleTest]]: [MySQL] 53,897 pass(es).
[ View ]
#2 twig-views-field-test.zip8.49 KBrovo
#1 twig-views-field-1843746.patch1.05 KBjoelpittet
FAILED: [[SimpleTest]]: [MySQL] 53,848 pass(es), 62 fail(s), and 21 exception(s).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new1.05 KB
FAILED: [[SimpleTest]]: [MySQL] 53,848 pass(es), 62 fail(s), and 21 exception(s).
[ View ]

separated from meta patch, first draft

StatusFileSize
new8.49 KB

I've run this patch against the latest Front-end branch.

It created:
/drupal_8_twig_sandbox/core/themes/stark/templates/views-view.html.twig

I compared the output before and after the patch.

There were no errors, and appeared the same.

Attaching DOM capture of #content before and after the patch.

Is there more I should look for?

Needed to switch the Admin to Stark in order for Views to pick up the views-view.html.twig template.

Now the "H2 A" elements are not printing out in the Summaries on the generated View Page.
<h2><a href="/node/32" rel="bookmark"></a></h2>

Issue tags:+VDC

Tagging.

Title:Convert views/theme/views-view-field.tpl.php to twig Convert views/theme/views-view-field.tpl.php to Twig

Working OK for me except for image fields - however I suspect that might be due to a separate issue.

Views' PHP template:

<div class="field-content">
  <img src="http://d8/sites/default/files/field/image/blueberries.gif" width="100" height="79" alt="" typeof="foaf:Image">
</div>

Twig template:

<div class="field-content">
  <figure class="">
    <img src="" class="image-formatter">
  </figure>
</div>

The problem mentioned in #4 seems to be unrelated to this issue though for sure worth to report! Thank you.

Status:Needs review» Fixed

Committed and correctly attributed, thanks!

Status:Fixed» Closed (fixed)

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

Project:ARCHIVE: Drupal 8 Twig Sandbox» Drupal core
Version:» 8.x-dev
Component:Twig templates conversion (front-end branch)» theme system
Status:Closed (fixed)» Active
Issue tags:+Twig

Moving to core queue

Status:Active» Needs review
StatusFileSize
new2.09 KB
PASSED: [[SimpleTest]]: [MySQL] 53,897 pass(es).
[ View ]

This is a straight re-roll of @joelpittet's patch in #1 against 8.x HEAD and deleted the corresponding .tpl.php.

Status:Needs review» Needs work

Thanks @tlattimore

I am moving this back to needs work for some cleanup docs and the preprocessor.

There is a theme function for this that needs to be moved into the preprocessor.

/**
* Display a single views field.
*
* Interesting bits of info:
* $field->field_alias says what the raw value in $row will be. Reach it like
* this: @code { $row->{$field->field_alias} @endcode
*/
function theme_views_view_field($vars) {
  $view = $vars['view'];
  $field = $vars['field'];
  $row = $vars['row'];
  return $vars['output'];
}

Also, would like to get the variable $vars converted to $variables

/**
* Process a single field within a view.
*
* This preprocess function isn't normally run, as a function is used by
* default, for performance. However, by creating a template, this
* preprocess should get picked up.
*/
function template_preprocess_views_view_field(&$vars) {
  $vars['output'] = $vars['field']->advanced_render($vars['row']);
}

Assigned:Unassigned» trrroy
Status:Needs work» Needs review
StatusFileSize
new3.23 KB
FAILED: [[SimpleTest]]: [MySQL] 53,681 pass(es), 143 fail(s), and 6 exception(s).
[ View ]

Comment cleanup, theme function removed and 'vars' changed to 'variables'.

Status:Needs review» Needs work

A few more nitpicks:

+++ b/core/modules/views/templates/views-view-field.html.twig
@@ -0,0 +1,24 @@
+ *
+ * Variables available:
+ * - view: The view object
+ * - field: The field handler object that can process the input
+ * - row: The raw SQL result that can be used
+ * - output: The processed output that will normally be used.

1. Change "Variables available" to "Available variables" -- it's what we've been doing in all the other templates.
2. Add punctuation to the ends of the sentences.

+++ b/core/modules/views/templates/views-view-field.html.twig
@@ -0,0 +1,24 @@
+ *
+ * The above will guarantee that you'll always get the correct data,
+ * regardless of any changes in the aliasing that might happen if
+ * the view is modified.
+ */
+#}
+{{ output }}

Let's add an '\n@ingroup themeable' to the bottom of this docblock.

+++ b/core/modules/views/views.theme.inc
@@ -319,28 +319,14 @@ function template_preprocess_views_view_grouping(&$vars) {
-/**
  * Process a single field within a view.
  *
  * This preprocess function isn't normally run, as a function is used by
  * default, for performance. However, by creating a template, this
  * preprocess should get picked up.
  */
-function template_preprocess_views_view_field(&$vars) {

Since the theme function is gone, the entirety of this docblock text should be:

"Prepares variables for views field templates.\n\n Default template: views-view-field.html.twig."

Status:Needs work» Needs review
StatusFileSize
new1.82 KB
new3.35 KB
FAILED: [[SimpleTest]]: [MySQL] 53,677 pass(es), 144 fail(s), and 6 exception(s).
[ View ]

Changes are applied.

Status:Needs review» Needs work

The last submitted patch, 1843746-views-view-field-13.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new546 bytes
new3.89 KB
FAILED: [[SimpleTest]]: [MySQL] 53,785 pass(es), 63 fail(s), and 21 exception(s).
[ View ]

Looks good, I just think the definition in views_theme needs changing too, and this is good to go.

This only part I'm not sure about is this part: "It is not actually used in default Views ..." huh? :)

Status:Needs review» Needs work

The last submitted patch, 1843746-15.patch, failed testing.

Title:Convert views/theme/views-view-field.tpl.php to Twig Convert views/templates/views-view-field.tpl.php to Twig

Status:Needs work» Needs review
StatusFileSize
new3.98 KB
FAILED: [[SimpleTest]]: [MySQL] 54,892 pass(es), 61 fail(s), and 21 exception(s).
[ View ]

Removed the line from http://drupal.org/node/1843746#comment-7252450 since without the theme function it no longer applies.
Added @see to documentation in line with http://drupal.org/node/1823416
Add $variables param to documentation in line with http://drupal.org/node/1913208

Status:Needs review» Needs work

The last submitted patch, drupal_twig_views_field_template-1843746-18.patch, failed testing.

@ damiankloip
Quick question about #15: could you explain a bit more about the change from pattern to template in views_theme()? I think views_view_grouping is a similar case, and I want to understand a little better.

I'm wondering whether we should have performance testing for this one ... as fields are rendered a lot. (could be more then hundred on a table view). Previously there is the theme_ vs. template trick, should we at least test this?

+++ b/core/modules/views/views.module
@@ -108,7 +108,7 @@ function views_theme($existing, $type, $theme, $path) {
   $hooks['views_view_field'] = $base + array(
-    'pattern' => 'views_view_field__',
+    'template' => 'views-view-field',
     'variables' => array('view' => NULL, 'field' => NULL, 'row' => NULL),

@jastraat I am almost positive that is not necessary for this conversion. Remove that change and give that a go again. Nice work so far:)

Status:Needs work» Needs review

I retract #22, that is the default pattern, so shouldn't be necessary and probably does need template because it had a theme_ function and template file...

This one needs another pair of eyes, please.

So the idea of this small piece of code was the following: Use the theme function by default, as it's certainly faster then using a template. As written before, if you have like 100 or more calls to this theme function on a page, this can be problematic.

As there will be no theme functions in D8, just twig, there is no need to special case the pattern/template bit.

@dawehner - Does that mean that the entire section for views_view_field in views_theme() can be removed? Since the theme function will no longer exist?

Status:Needs review» Needs work

I can answer #25, the hook_theme() definition is still needed even though the output will be through a template file.

Back to needs work, the test failures need to be addressed. Thanks everyone!

Issue tags:+Needs manual testing

Tagging.

Assigned:trrroy» Unassigned
Status:Needs work» Needs review
Issue tags:+Needs profiling
StatusFileSize
new3.94 KB
PASSED: [[SimpleTest]]: [MySQL] 54,439 pass(es).
[ View ]
new1.18 KB

Would love to see profiling as mentioned in #21.

This should fix the errors as as Views advanced_render() returns null from time to time.

Well, but what is the problem with maybe put out a NULL variable, is this a specific twig problem?

Status:Needs review» Needs work

#29 - Looks like it's a problem with the way that Twig is implemented in Drupal - It will throw an exception whenever a NULL variable is passed to a Twig template. Same problem we're seeing in #1961872: Convert html.tpl.php to Twig.

I've opened a couple of tickets to see what can be done about it, if anything #1975442: drupal_render() should always return a string., #1975462: Allow and test for NULL and integer 0 values in Twig templates..

Regardless of whether this is resolved within Twig itself, can we not put the workaround inside the template? I really think this will be confusing to front end developers, as they'll have no idea when something coming in could possibly be NULL - if we break the seal we'll see {% if foo is defined %} appearing *everywhere* in contrib/custom templates.

Just change +  $variables['output'] = $variables['field']->advanced_render($variables['row']); to something that enforces a string data type like:

<?php
  $variables
['output'] = (string) $variables['field']->advanced_render($variables['row']);
?>

Issue tags:-Needs manual testing

Aside from performance, manual testing of #28 looks good:

Before:

<div class="views-row views-row-1 views-row-odd views-row-first">
  <div class="views-field views-field-title">        <span class="field-content"><a href="/node/11">Commodo Inhibeo Letalis Nostrud Nunc Typicus</a></span>  </div>
  <div class="views-field views-field-field-image">    <span class="views-label views-label-field-image">Image: </span>    <div class="field-content"><a href="/node/11"><img typeof="foaf:Image" src="http://d8.localhost/sites/default/files/field/image/imagefield_9dafD4.jpg" width="298" height="269" alt="Elit eum lenis melior." title="Jumentum nutus praesent sed sudo."></a></div>  </div>
  <div class="views-field views-field-body">    <span class="views-label views-label-body">Body: </span>    <div class="field-content">Brevitas magna quidne quis ratis. Enim iustum ullamcorper. Duis luptatum magna persto tation vereor.
Ad exerci gilvus lobortis loquor roto usitas vindico. Abbas accumsan consequat haero jus olim quidne torqueo tum wisi. Caecus dolor elit magna persto vulputate. Abbas imputo nostrud praesent singularis tincidunt. Acsi si sino vero. Abluo ad dignissim hendrerit inhibeo interdico rusticus singularis velit. Capto eum gemino imputo quia tum.
</div>  </div>  </div>

After:

<div class="views-row views-row-1 views-row-odd views-row-first">
  <div class="views-field views-field-title">        <span class="field-content"><a href="/node/11">Commodo Inhibeo Letalis Nostrud Nunc Typicus</a></span>  </div>
  <div class="views-field views-field-field-image">    <span class="views-label views-label-field-image">Image: </span>    <div class="field-content"><a href="/node/11"><img typeof="foaf:Image" src="http://d8.localhost/sites/default/files/field/image/imagefield_9dafD4.jpg" width="298" height="269" alt="Elit eum lenis melior." title="Jumentum nutus praesent sed sudo."></a></div>  </div>
  <div class="views-field views-field-body">    <span class="views-label views-label-body">Body: </span>    <div class="field-content">Brevitas magna quidne quis ratis. Enim iustum ullamcorper. Duis luptatum magna persto tation vereor.
Ad exerci gilvus lobortis loquor roto usitas vindico. Abbas accumsan consequat haero jus olim quidne torqueo tum wisi. Caecus dolor elit magna persto vulputate. Abbas imputo nostrud praesent singularis tincidunt. Acsi si sino vero. Abluo ad dignissim hendrerit inhibeo interdico rusticus singularis velit. Capto eum gemino imputo quia tum.
</div>  </div>  </div>

Assigned:Unassigned» Cottser

Doing some profiling here and likely posting a revised patch to lose the if statement.

Yeah we do have a preprocess function, which allows us totally to fix this NULL problem. I guess "" is a proper value for that.

#33 - As in #1961872: Convert html.tpl.php to Twig, #1975442: drupal_render() should always return a string., if there's a function that's supposed to be "rendering" values - in this case Views advanced_render() - we probably want to make sure that it returns a string reliably rather than NULL. This would hopefully be a better fix than working around the Twig engine in the preprocess or the template.

Incidentally,

Views advanced_render() returns null from time to time.

advanced_render() sometimes calls drupal_render(), which sometimes returned NULL until yesterday. Is there any other way for Views advanced_render() to return NULL? If not, this problem is already fixed.

StatusFileSize
new717 bytes
FAILED: [[SimpleTest]]: [MySQL] 54,828 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Maybe this could fix this 100%, though i'm not sure.

Maybe this could fix this 100%, though i'm not sure.

Status:Needs work» Needs review

testbot

Status:Needs review» Needs work

The last submitted patch, drupal-1843746.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new730 bytes
FAILED: [[SimpleTest]]: [MySQL] 54,850 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

@dawehner - how about return (string) $value;? then yeah, from the looks of it, it should be impossible for Views to return anything non-stringy from advanced_render().

Status:Needs review» Needs work

The last submitted patch, 1843746-39.patch, failed testing.

Yeah, this could work. I guess we have to manually adapt one of the tests?

#41 - or drupal_render($value);? anyway, we're totally hijacking this thread now. We should make a new issue for the Views render.

We should just re-roll #28 without the "is not null" and see what happens.

Status:Needs work» Needs review

We should just re-roll #28 without the "is not null" and see what happens.

I agree.

I'm not sure we need the 'if' at all, will post a patch tonight and some preliminary profiling numbers.

Assigned:Cottser» Unassigned
Issue tags:-Needs profiling
StatusFileSize
new407 bytes
new3.93 KB
PASSED: [[SimpleTest]]: [MySQL] 55,399 pass(es).
[ View ]

It does make sense to keep the if block, at least with the current state of things.

Profiled with @Fabianx's xhprof-kit, and thanks to his help and patience I put together and profiled a scenario that renders 200 fields in a table view.

Comparisons:

  1. Theme function implementation (HEAD).
  2. PHPTemplate implementation, HEAD plus this change:
    diff --git a/core/modules/views/views.module b/core/modules/views/views.module
    index 8e2a378..d09b87c 100644
    --- a/core/modules/views/views.module
    +++ b/core/modules/views/views.module
    @@ -108,7 +108,7 @@ function views_theme($existing, $type, $theme, $path) {
       // Default view themes
       $hooks['views_view_field'] = $base + array(
    -    'pattern' => 'views_view_field__',
    +    'template' => 'views-view-field',
         'variables' => array('view' => NULL, 'field' => NULL, 'row' => NULL),
       );
       $hooks['views_view_grouping'] = $base + array(
  3. Twig template implementation (patch attached to this comment).

Theme function vs. PHPTemplate implementation:

=== baseline-views-field-theme-200-fields..views-field-tpl compared (51774c26bf054..51774cdfb1214):
ct  : 108,070|113,470|5,400|5.0%
wt  : 437,956|462,870|24,914|5.7%
cpu : 406,820|430,145|23,325|5.7%
mu  : 15,827,696|15,836,624|8,928|0.1%
pmu : 16,039,152|16,050,992|11,840|0.1%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51774c26bf054&...

Theme function vs. Twig template implementation:

=== baseline-views-field-theme-200-fields..views-field-twig compared (51774c26bf054..51774d289642f):
ct  : 108,070|117,830|9,760|9.0%
wt  : 437,956|472,152|34,196|7.8%
cpu : 406,820|440,780|33,960|8.3%
mu  : 15,827,696|15,851,768|24,072|0.2%
pmu : 16,039,152|16,057,240|18,088|0.1%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51774c26bf054&...

After looking at the profiling results and discussing with @Fabianx, we already have at least a few ideas for optimization:

  • Around 5ms: #1938430: Don't add a default theme hook class in template_preprocess() will remove the calls to drupal_html_class().
  • Around 4ms: Add static caching to Drupal\Core\Template\TwigEnvironment::loadTemplate(). (Issue to come)
  • Around 2ms: Use drupal_static_fast for $default_attributes (in addition to $default_variables) in template_preprocess().

Total - somewhere around 11ms, which should at least make the Twig implementation quicker than the PHPTemplate one. Longer term maybe we can get rid of template_preprocess() altogether or slim it down some more.

I'm also wondering (completely untested theory) if we could lose the whitespace operators here if that would mean better performance.

Scenario profiled:

  • 50 nodes created via devel generate, then disabled devel and devel generate
  • Stark theme
  • APC class loader enabled in settings.php
  • Created a table view of nodes displaying 50 rows with 4 columns: node title linked to the node, nid, date posted, and image
  • Total of 200 view fields rendered on the page

The site homepage was used for profiling, with one of the 50 nodes set as the default homepage and the table view added as a block to the homepage.

Issue tags:+Needs profiling

I'm also wondering (completely untested theory) if we could lose the whitespace operators here if that would mean better performance.

I couldn't detect any difference in performance at all after removing the whitespace modifiers. Good to know.

Issue tags:-Needs profiling

Oops, that'll teach me to reuse a tab :)

I am quite sure we don't need the if statement if we get this guy in #1975462: Allow and test for NULL and integer 0 values in Twig templates. If I remember right it's why I put that statement in the first place. Also, that should help a bit with the performance a bit as well. Cottser maybe you can roll the two for a test?

@joelpittet/#49 - after doing some tests it looks like #1975462: Allow and test for NULL and integer 0 values in Twig templates. will either have no impact on performance or a small negative impact for this particular scenario. The numbers are quite close though and it's tough to pin them down. After adding #1975462: Allow and test for NULL and integer 0 values in Twig templates. and removing the if block from views-view-field.html.twig there are 75 more function calls in the scenario described in #46.

Thanks for this great benchmark. So was far as I understand this performance regression is too much at the moment, so we should better wait until the points you listed went it. Hopefully this will lower then the costs.

Well, currently #1938430: Don't add a default theme hook class in template_preprocess() is postponed on conversion. Maybe we need to rethink that now if this can't be committed without it.

Edit: Or we do all the other optimizations we can and keep #1938430: Don't add a default theme hook class in template_preprocess() postponed?

Status:Needs review» Needs work

#1975462: Allow and test for NULL and integer 0 values in Twig templates. just landed. Does this mean we can remove the "if output" and just leave {{ output }} to improve performance?

Yes, but doing so didn't seem to have a measurable effect on performance from what I remember. Which is both good and bad :)

#54 - sure. well... remove it anyway for readability?

Yes, we should still remove it because it's unnecessary.

Status:Needs work» Needs review
StatusFileSize
new3.9 KB
FAILED: [[SimpleTest]]: [MySQL] 55,349 pass(es), 61 fail(s), and 0 exception(s).
[ View ]
new532 bytes

Removed twig if;)

Status:Needs review» Needs work

The last submitted patch, 1843746-57-views-view-field.patch, failed testing.

Is that failing because of whitespace?

Status:Needs work» Needs review
StatusFileSize
new3.93 KB
PASSED: [[SimpleTest]]: [MySQL] 55,399 pass(es).
[ View ]
new556 bytes

Yeah @thedavidmeister that seemed to be the issue.

Status:Needs review» Needs work

+ * - field: The field handler object that can process the input.
+ * - row: The raw SQL result that can be used.

How does "field handler object" and "raw SQL result" fit in with our rules against discussing PHP data types in template comments?

+ *   - field: @todo.
+ *   - row: @todo.

See http://drupal.org/node/1757550#comment-7358798

Assigned:Unassigned» shanethehat

We should also split off the theme_ function removal into its own issue along the same lines as #1843752: Convert views/templates/views-view-grouping.tpl.php to twig and #1912606: Remove theme_views_view_grouping function. Thanks!

Edit: In the end we will probably either keep the theme_ function (still allowing template overrides as we do now), or we may convert it to use a markup utility function - see #1986116: Improve performance by replacing very small and simple templates and theme function with "Markup Utility Functions" for discussion. So for now, maybe we don't need the other issue created, since it's a trivial patch to create.

Assigned:shanethehat» Unassigned
Status:Needs work» Needs review
StatusFileSize
new3.23 KB
PASSED: [[SimpleTest]]: [MySQL] 55,760 pass(es).
[ View ]

New patch, only replaces the template and modifies the documentation. Leaves theme function alone.

I've removed the paragraph about the preprocess not usually running because it is run when the theme function is used.

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

+ * Prepares variables for views field templates.
+ *
+ * Default template: views-view-field.html.twig.
+ *
+ * @param ar

trailing spaces here.

+ *   - field: The field handler object for the current field..

Double period at the end of this sentence.

+ * When fetching output from the row, this construct should be used:

Unnecessary comma.

+ * The above will guarantee that you'll always get the correct data,
+ * regardless of any changes in the aliasing that might happen if
+ * the view is modified.

These comments can be "reflowed" to use up the full 80 characters available.

Status:Needs work» Needs review
StatusFileSize
new9.05 KB
PASSED: [[SimpleTest]]: [MySQL] 55,466 pass(es).
[ View ]

This should address those issues. I also fixed a few other of the worst offenders in views.theme.inc with regards to comments going past 80 characters.

Status:Needs review» Needs work

I also fixed a few other of the worst offenders in views.theme.inc

I appreciate the good intentions but we really have to keep these views conversion patches from creeping in scope. There's a separate issue for every one of those preprocess functions that you modified the docs for at #1757550: [META-63] Convert core theme functions to Twig templates.

Could we get a re-roll with just the docs updates for views-view-field?

Issue tags:+Needs profiling

Tagging for profiling.

Status:Needs work» Needs review
StatusFileSize
new3.25 KB
PASSED: [[SimpleTest]]: [MySQL] 56,108 pass(es).
[ View ]
new2.94 KB

Fixes the docs issues from #65, and also puts the template in the templates directory!

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

Assigned:Unassigned» joelpittet

grabbing for profiling

Issue tags:-Needs profiling

Scenario

View table with 50 rows and 5 fields = 250 fields rendered. A views-view-field.tpl.php in 8.x and views-view-field.html.twig both in Stark on their respective branches.

View block full content node in the content region.

=== 8.x..8.x compared (519bd405e5fe3..519bd4c5b5ac8):
ct  : 109,395|109,395|0|0.0%
wt  : 808,806|811,711|2,905|0.4%
cpu : 768,192|769,879|1,687|0.2%
mu  : 41,201,936|41,201,936|0|0.0%
pmu : 41,460,888|41,460,888|0|0.0%

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

=== 8.x..1843746-views-view-field compared (519bd405e5fe3..519bd4feea4c7):
ct  : 109,395|114,683|5,288|4.8%
wt  : 808,806|814,682|5,876|0.7%
cpu : 768,192|774,967|6,775|0.9%
mu  : 41,201,936|41,251,104|49,168|0.1%
pmu : 41,460,888|41,438,176|-22,712|-0.1%

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

StatusFileSize
new1.58 KB
new3.27 KB
PASSED: [[SimpleTest]]: [MySQL] 56,012 pass(es).
[ View ]

Added the warning back in for that the template is not being used and is for reference as the theme_ function is used for performance.

No changes from performance teston #72

Also, reverted $variables to $vars because that is being done in. #1963942: Change all instances of $vars to $variables

Still RTBC, ready for review. Thanks @joelpittet!

Status:Reviewed & tested by the community» Needs work
Issue tags:-Twig, -VDC

The last submitted patch, 1843746-73-views-view-field.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Twig, +VDC

#73: 1843746-73-views-view-field.patch queued for re-testing.

Status:Needs review» Reviewed & tested by the community

Assigned:joelpittet» Unassigned

Status:Reviewed & tested by the community» Needs work

+++ b/core/modules/views/templates/views-view-field.html.twigundefined
@@ -0,0 +1,30 @@
+{% spaceless %}
+  {{ output }}
+{% endspaceless %}

Why are we using the spaceless functionality here? If we need it then we to doc why...

Ok I replied but I guess the internet was too slow or something...

@alexpott I put that in because the twig template didn't remove the last newline character and failed whitespace related tests.

I wanted to get the markup as close as possible. So the real purpose in this case is to remove the ending \n character.

I'm sure there could be another way to do this, either leave the newline and change the whitespace tests or possibly something better but spaceless does the trick.

StatusFileSize
new564 bytes
new3.24 KB
PASSED: [[SimpleTest]]: [MySQL] 55,856 pass(es).
[ View ]

After running some tests locally I am not sure we really need spacelacess here at all to get it to pass the tests. Here is a patch that removes spaceless stuff from this instance - we'll see if the testbot approves. Figures crossed!

Status:Needs work» Needs review

Oops. Tagging for review.

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

The last submitted patch, 1843746-views-view-field-81.patch, failed testing.

Status:Needs work» Needs review

#81: 1843746-views-view-field-81.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, 1843746-views-view-field-81.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+Twig, +VDC

#81: 1843746-views-view-field-81.patch queued for re-testing.

Thats nice! Passed tests without spaceless. Anything else here you're concerned about, @Alexpott? RTBC?

Status:Needs review» Reviewed & tested by the community

Great, thanks @tlattimore and @joelpittet! Back to @alexpott.

Nice @tlattimore. It seems that the twig templates are behaving like the docs indicated and remove the last newline as does the PHP template.

http://twig.sensiolabs.org/doc/templates.html#whitespace-control

Title:Convert views/templates/views-view-field.tpl.php to Twig [READY] Convert views/templates/views-view-field.tpl.php to Twig
Status:Reviewed & tested by the community» Closed (duplicate)

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

Committed c68896f and pushed to 8.x. Thanks!

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

Issue summary:View changes

Updated issue summary.