Problem/Motivation

Now that the Twig engine is in core, we must convert all existing core *.tpl.php template files and theme_ functions to use Twig templates. Preprocess functions and hook_theme definitions must also be updated.

TODO

DOCS: Coding standards for conversion:

Resources

Proposed resolution

See sub-issues below.

Remaining tasks

Template file conversions:

  • Replace all tpl.php files with .html.twig equivalents
  • Update preprocess functions for the .html.twig versions of .tpl.php templates, to use renderables
  • Benchmark the .html.twig versions of template files to assure they aren't very much slower

Convert theme_* functions to Twig templates. See #2348381: [META-0 theme functions left] Convert/refactor core theme functions.

  • Replace all theme functions with .html.twig equivalent templates
  • Add new preprocess functions for the .html.twig versions of theme functions (if necessary)
  • Benchmark the .html.twig versions of theme functions to assure they aren't very much slower
  • Update all hook_theme definitions to include 'template' => 'foo', where foo.html.twig is the template being added

RTBC

    Needs review

    @see Documentation for reviewing patches

      Needs profiling

      @see Documentation for profiling twig patches

      Needs manual testing

      @see Documentation for manually testing twig patches

        Needs work

        Twig + Needs reroll search

        Needs work (needs Tests)

        @see Documentation for writing automated tests

          Needs work (performance tuning)

            Postponed

            Fixed/closed (roughly reverse chronological)


            User interface changes

            None.

            API changes

            Likely minimal to no API changes.

            Possible new theme layer process order:
            http://gist.io/5650535

            Resources

            Google Docs:

            Twig sandbox: http://drupal.org/sandbox/pixelmord/1750250 (all conversions have been moved over from the sandbox and we are working directly in the core queue, the sandbox link is only here for reference).

            Related Issues

            ### These issues were actually blocking progress ###

            ## This issue will decrease the work we need to do ##

            These issues are related to our work, and will block the release of D8

            Twig engine issues

            Performance issues

            After Commit Issues

            Notes for reviewing Twig Patches manually

            To find a list of patches that are theoretically good to go but manual testing is holding them back see http://drupal.org/project/issues/search/drupal?text=convert&status%5B%5D....

            It is helpful to use Dreditor to review patches and to test patches applied to fresh install of Drupal8 via simplytest.me.

            Generally, you are trying to see if the HTML output has changed from a clean install of Drupal 8 versus a patched install. Many Twig issues have a "To test this code" section in the issue summary that will direct you how to test the patch. If you find that testing the patch using simplytest.me has not produced the results you expected, or only some of the expected output, you will want to test the patch manually.

            Check out this screencast on how to use Daisy Diff to compare markup: http://www.youtube.com/watch?v=Bv4PY_ZEP4Q

            Comments

            larowlan’s picture

            Issue tags: +Twig

            tagging

            larowlan’s picture

            Issue summary: View changes

            Updated issue summary.

            podarok’s picture

            Project: Drupal core »
            Version: 8.x-dev »
            Component: theme system » Twig templates

            moving to proper project

            podarok’s picture

            Project: » Drupal core
            Issue summary: View changes

            Updated issue summary.

            c4rl’s picture

            Issue summary: View changes

            Update summary to reflect google doc

            c4rl’s picture

            Project: Drupal core »
            Component: Twig templates » Twig templates conversion (front-end branch)

            Updated issue summary to reflect that we're tracking these in a Google Doc.

            catch’s picture

            Title: [meta] Add twig support to core templates » [meta] Add convert core templates to Twig
            Project: » Drupal core
            Version: » 8.x-dev
            Component: Twig templates conversion (front-end branch) » theme system
            Priority: Normal » Critical

            I think this is more appropriate if we're going to get Twig conversion done anywhere near approaching API freeze.

            Berdir’s picture

            Title: [meta] Add convert core templates to Twig » [meta] Convert core templates to Twig

            Fixing title.

            jenlampton’s picture

            We're nowhere near being able to replace all the themable output in core with Twig templates yet (I wasn't aware that had anything to do with API freeze?), we don't even have a theme component library ready for use yet.

            That said, we are ready to do a straight conversion of all the .tpl.php files into .html.twig files, if that would buy us some time to rethink all the theme functions.

            In theory, we could do a straight conversion of everything we have now into Twig - but I think we can do better than that. Doing a straight conversion to twig first would be a bit of a time sink (since some/most of it would get thrown out later) but if we need to go that route to get it in we can.

            catch’s picture

            I haven't seen any theme component library patches land recently although might have missed some committed by Dries/webhick? #1595614: [meta] Remove all the theme functions and templates in core that simply output a link. Replace with #type 'link' render arrays for example hasn't had an update for nearly a month. Is there work going on with the theme function/template consolidation that I've missed?

            As we're converting templates, we'll likely hit issues with the Twig/Drupal integration and/or want to work on enhancements like removing a bunch of stuff from preprocess for performance. Some of those things might end up getting blocked on API freeze although it depends what they are. But the earlier conversions go in the more chance there is to tweak things as we go.

            If functions like theme_links(), theme_item_list() and theme_table() end up getting removed that's quite a significant API change in itself. Module authors need to be able to start porting once API freeze hits, and 1. know that anything that's on its way out is really, really on its way out and they shouldn't use it 2. have enough solid ground for new stuff that replaces it to refer to that they're able to port to it via core examples. i.e. there needs to be enough in to tip the balance by then I think.

            mbrett5062’s picture

            About time the Google doc was updated, just checked a number of 'needs review' issues and they were all fixed - closed and committed more then 2 weeks ago. Would be nice to see what the actual status is, and where help is needed.

            jenlampton’s picture

            Issue summary: View changes

            Remove links

            steveoliver’s picture

            Issue summary: View changes

            Updating with issue summary template.

            jenlampton’s picture

            @catch so it sounds like we should prioritize everything currently in system module / theme.inc and maybe get those in ASAP - even if that means changing them later to fit with the theme component library (still not done yet, no core patches in progress).

            I'll see where we are today in the hangout and maybe we can get going on those.

            @mbrett5062 I hope you updated the google doc to show the correct status of all the issues you looked at? :)

            mbrett5062’s picture

            @jenlampton, I don't think I have access to update the doc. Correct me if I am wrong, would be happy to help in any way I can.

            mbrett5062’s picture

            Ignore last comment @jenlampton, I was able to update the sheet. Have now marked all rows with correct status of relevant issue. Some were demoted from fixed, and will need looking at again. Lots marked as fixed now as issue has been closed fixed for more then 2 weeks. Hope that helps a little.

            eigentor’s picture

            Ah O.K. was looking for a twig equivalent for page.tpl.php - so I guess this is a bit soon in the process.

            jenlampton’s picture

            Issue summary: View changes

            cleanup

            jenlampton’s picture

            @eigentor In the long run, page.tpl.php will likely be left to the blocks and layouts initiative and will perhaps be replaced with a layout. For now, we've just redone the D7-style template in twig syntax.

            @mbrett5062 thanks for the help, there are so many moving pieces it's hard to keep them all in sync! :)

            @catch I'm not sure how removing functions like theme_links() is an API change. From the perspective of modules, they should still be calling theme('links') exactly as they were before. Is it because themes will need to write a links.html.twig file instead? I guess that's a pretty huge difference :) Forward we go...

            catch’s picture

            @jenlampton - the example would be if theme('links') got replace with theme('item_list') - then theme('links') would just break.

            eigentor’s picture

            @jenlampton the redone page.tpl.php is not in core yet, right?

            webchick’s picture

            $ find core -name *.tpl.php | wc -l
                  66
            
            $ grep -r 'function theme_' core | wc -l
                 172
            
            $ find core -name *.twig | grep -cv 'vendor'
                 5
            

            This is a really spooky place to be in, less than one month before feature freeze, folks... :( While we don't have to have all conversions done until July 1, we still have basically no idea right now if we can actually use Twig as a performant, secure, and themer-friendly way yet, because nothing in core other than node and toolbar is actually using Twig.

            It would be one thing if the fantastic energy and momentum the Twig team had kept up around BADCamp had kept on, and we were committing a few patches a week to solve underlying Twig things we still don't know how to solve (how will preprocess functions work? will we use the sanitize feature or not? etc.). Instead, it seems like the team has largely dissipated and other than #1812724: Consolidate all form element templates and add theme_hook_suggestons I can't recall committing any Twig-affecting patches in recent memory. http://drupal.org/project/issues/search/drupal?issue_tags=Twig seems to back that up.

            There needs to be some pick-up here, real soon, or we might need to have some very hard discussions come mid-Feb. :(

            webchick’s picture

            Issue summary: View changes

            added links to theme.inc patches

            jenlampton’s picture

            @webchick I think the Twig team has mostly been fixated on bigger / harder issues that will actually solve some of the problems our theme system faces regardless of the engine in use. Unfortunately, it's really hard for us to make progress on issues that big quick enough for you to commit several underlying patches per week. :) see #939462: Specific preprocess functions for theme hook suggestions are not invoked

            We can (and will) start giving you more patches that replace all the .tpl.php files with .html.twig ones, but that's only a very small piece of fixing our theme system. Hopefully making some progress on those issues though will at least make it feel like we're still moving things in the right direction :)

            007g3m1n1’s picture

            At this week's San Diego Drupal Camp, we have planned a Twig sprint on Saturday, Jan 26th
            https://www.sandcamp.org/twig-sprint-at-sandcamp
            @jenlampton, would you by any chance be able to come and join us?
            -Gemini

            eigentor’s picture

            I don't know how hard it is to convert at least the most common templates. But especially if it is not, it would make the system at least basically usable. I guess there are not too many people trying to create a twig-based theme at the moment.

            But even those cannot do that. So Twig remains a black box for too long. The fact that it is usable needs proof.
            So if I can help to convert page.tpl.PHP with no coding skills, point me to where I can help. If not, try to get someone to start on it. This might get more people interested.

            eigentor’s picture

            Issue summary: View changes

            ling to aggregator

            jenlampton’s picture

            Issue summary: View changes

            remove message to leave theme in place

            c4rl’s picture

            Issue summary: View changes

            Add list of all core modules that invoke hook_theme

            c4rl’s picture

            Issue summary: View changes

            Removed extra files from module list

            c4rl’s picture

            Issue summary: View changes

            Block module issue

            c4rl’s picture

            Issue summary: View changes

            Linked custom_block module issue

            c4rl’s picture

            Issue summary: View changes

            Linked book module issue

            c4rl’s picture

            Issue summary: View changes

            Linked color module issue

            c4rl’s picture

            Issue summary: View changes

            Linked comment module issue

            c4rl’s picture

            Issue summary: View changes

            Linked dblog module issue

            c4rl’s picture

            Issue summary: View changes

            Linked field module issue

            c4rl’s picture

            Issue summary: View changes

            Linked field_ui module

            c4rl’s picture

            Issue summary: View changes

            Linked file module issue

            c4rl’s picture

            Issue summary: View changes

            Linked filter module issue

            c4rl’s picture

            Issue summary: View changes

            Linked forum module issue

            c4rl’s picture

            Issue summary: View changes

            Linked image module issue

            c4rl’s picture

            Issue summary: View changes

            Linked language module

            c4rl’s picture

            Issue summary: View changes

            Linked layout module issue

            c4rl’s picture

            Issue summary: View changes

            Linked link module issue

            c4rl’s picture

            Issue summary: View changes

            Linked locale module issue

            c4rl’s picture

            Issue summary: View changes

            Linked menu module issue

            c4rl’s picture

            Issue summary: View changes

            Linked node module issue

            c4rl’s picture

            Issue summary: View changes

            Linked overlay module issue

            c4rl’s picture

            Issue summary: View changes

            Linked picture module issue

            c4rl’s picture

            Issue summary: View changes

            Linked rdf module issue

            c4rl’s picture

            Issue summary: View changes

            Linked search module issue

            c4rl’s picture

            Issue summary: View changes

            Linked shortcut module issue

            c4rl’s picture

            Issue summary: View changes

            Linked simpletest module issue

            c4rl’s picture

            Issue summary: View changes

            Linked system module issue

            c4rl’s picture

            Issue summary: View changes

            Linked common_test module issue

            c4rl’s picture

            Issue summary: View changes

            Linked theme_test module

            c4rl’s picture

            Issue summary: View changes

            Linked taxonomy module issue

            c4rl’s picture

            Issue summary: View changes

            Convert toolbar module to Twig

            c4rl’s picture

            Issue summary: View changes

            Linked update module issue

            c4rl’s picture

            Issue summary: View changes

            Linked user module issue

            c4rl’s picture

            Issue summary: View changes

            LInked views module issue

            c4rl’s picture

            Issue summary: View changes

            Linked views_ui module issue

            c4rl’s picture

            Issue summary: View changes

            Linked pager.inc issue

            c4rl’s picture

            Title: [meta] Convert core templates to Twig » [meta] Convert core theme functions and templates to Twig

            More descriptive title

            c4rl’s picture

            Issue summary: View changes

            Linked menu.inc issue

            c4rl’s picture

            Issue summary: View changes

            Linked form.inc issue

            webchick’s picture

            Awesome, the issues filed under http://drupal.org/project/issues/search/drupal?issue_tags=Twig make me very, very, very happy! :D Hope the sprint this weekend at SandCamp goes well!!

            c4rl’s picture

            I think we may have a potential blocker here, that I will explain, and perhaps others have some insight.

            Much of the Twig conversion work has been done in the Twig sandbox project. FabianX rebases the sandbox against D8 HEAD frequently, but I believe since the big Twig patch came in during BADCamp, there has been work done in the sandbox that isn't yet part of HEAD (e.g. usage of *.html.twig extension vs *.twig extension).

            Inasumch, in order for people to have working patches against core, do we need another maintenance patch on HEAD to bring its Twig integration up to date? And if so, is the most recent rebase on the sandbox working well enough to make this happen? Looking at this last night it appears there are some outstanding bugs in the sandbox, and I've seen a few new bug reports in the sandbox project in the past 24 hours (last rebase was yesterday).

            Can someone else confirm this is accurate?

            007g3m1n1’s picture

            eigentor’s picture

            Looking into the sandbox code, one finds all the templates inside the Stark theme in the templates folder.
            Do you have to rework all theses to integrate them into the proper core modules, or can they basically stay the same?

            c4rl’s picture

            The sandbox assumes that both PHPTemplate and Twig are valid theme engines, and PHPTemplate is the default. Therefore, stark provides all the markup for every core module. We needed to get Twig working before we could replace PHPTemplate.

            The curent proposal now is to replace PHPTemplate with Twig in D8 HEAD, which means that for every core module (and some .inc files) will need to reflect Twig as the default theme engine (and thus will provide .html.twig files). Stark, thus, reverts back to simply being a .info file (as it was in D7).

            However, I think there are problems, as I mentioned in my comment #22 since I believe some of the Twig internals have shifted in the sandbox. I need confirmation of this from others -- that is, is Twig even working in HEAD? I'm not certain that it is.

            steveoliver’s picture

            Twig works in 8.x head. Make a change to core/modules/node/templates/node.html.twig and see it is being used instead of the .tpl.php in the same directory.

            steveoliver’s picture

            Edit: not .html.twig -- as per #22 there are Twig internals like the .html.twig file extension which needs to be updated.

            steveoliver’s picture

            steveoliver’s picture

            Since #1905584: Move base theme system templates into /core/templates, we'll put all templates from core/includes/*.inc that don't end up in their own module (like menu), in core/modules/system/[templates].

            steveoliver’s picture

            Issue summary: View changes

            More descriptive summary

            thedavidmeister’s picture

            Issue summary: View changes

            Added follow up for comment module.

            c4rl’s picture

            Yes, based on #29, it seems that the issues listed above regarding the *inc files would be accommodated by #1898454: system.module - Convert PHPTemplate templates to Twig, and I've listed this as a comment in each of those issues.

            That could result in a big patch, so if it seems unwieldily, I suppose we can make #1898454: system.module - Convert PHPTemplate templates to Twig a meta issue and split up the rest.

            c4rl’s picture

            Issue summary: View changes

            Updated link to older/original views conversion issue.

            Cottser’s picture

            Related issue about preprocess function documentation, could use some more eyes and opinions please: #1913208: [policy] Standardize template preprocess function documentation

            Cottser’s picture

            Issue summary: View changes

            added a follow up issue for image module

            Cottser’s picture

            Issue summary: View changes

            Add link to screencast in summary

            c4rl’s picture

            Issue summary: View changes

            Move screencast link to top

            c4rl’s picture

            Issue summary: View changes

            Reformatting

            joelpittet’s picture

            Issue summary: View changes

            Moved the items that related to #1876712 into their own little section

            joelpittet’s picture

            Issue summary: View changes

            typo

            joelpittet’s picture

            Issue summary: View changes

            moved simple test down to the tables hold up area

            steveoliver’s picture

            Issue summary: View changes

            Added 'Remove theme functions' to list of proposed resolution(s).

            jenlampton’s picture

            Issue summary: View changes

            theme stuff

            jenlampton’s picture

            Issue summary: View changes

            move blocked issues

            jenlampton’s picture

            Issue summary: View changes

            stark link

            jenlampton’s picture

            Issue summary: View changes

            move

            jenlampton’s picture

            Issue summary: View changes

            added top 8 to top

            jenlampton’s picture

            Issue summary: View changes

            rest label

            jenlampton’s picture

            Issue summary: View changes

            sub theme.inc issue

            jenlampton’s picture

            Issue summary: View changes

            added node into blocked list

            Cottser’s picture

            Issue summary: View changes

            Updating blocker for node and custom_block

            Cottser’s picture

            Issue summary: View changes

            Removing other blocker for node and custom_block. I think it would be messy to try and convert the core themes before converting the modules.

            jenlampton’s picture

            Issue summary: View changes

            added more blockers

            Cottser’s picture

            Issue summary: View changes

            Remove sub-issue from theme.inc, see meta issue instead.

            Cottser’s picture

            Issue summary: View changes

            Update theme.inc link

            jenlampton’s picture

            Issue summary: View changes

            update theme.inc stuffs

            joelpittet’s picture

            Issue summary: View changes

            shuffling and making a needs reviewed pot

            joelpittet’s picture

            Issue summary: View changes

            move more around for needs review

            jenlampton’s picture

            Issue summary: View changes

            unblock

            jenlampton’s picture

            Issue summary: View changes

            order

            joelpittet’s picture

            Issue summary: View changes

            added rtbc area and moved layout into review needed

            joelpittet’s picture

            Issue summary: View changes

            Added needs work subhead

            jenlampton’s picture

            Issue summary: View changes

            alphabetize

            jenlampton’s picture

            Issue summary: View changes

            rework

            jenlampton’s picture

            Issue summary: View changes

            re

            jenlampton’s picture

            Issue summary: View changes

            re

            jenlampton’s picture

            Issue summary: View changes

            blocker

            jenlampton’s picture

            Issue summary: View changes

            block

            jenlampton’s picture

            Issue summary: View changes

            ]

            jenlampton’s picture

            Issue summary: View changes

            bold

            jenlampton’s picture

            Issue summary: View changes

            todos

            jenlampton’s picture

            Issue summary: View changes

            related

            c4rl’s picture

            Issue summary: View changes

            Move blockers to top

            Cottser’s picture

            Issue summary: View changes

            Tweak wording

            jenlampton’s picture

            Issue summary: View changes

            remove dupe

            joelpittet’s picture

            Issue summary: View changes

            Added #type=>table to the blocker list because it's holding up some combined conversions or not needed conversions.

            joelpittet’s picture

            Issue summary: View changes

            Views meta issues added.

            steveoliver’s picture

            Issue summary: View changes

            Added template_preprocess cleanup issue to list of blockers.

            Cottser’s picture

            If you've been thinking about getting involved, there's no time like the present. We're working hard to get everything converted and could really use your help.

            See the announcement posted on g.d.o/core for more information:
            http://groups.drupal.org/node/292068

            Cottser’s picture

            Issue summary: View changes

            added a blocking issue

            webchick’s picture

            Issue summary: View changes

            x

            Cottser’s picture

            Issue summary: View changes

            Update hook_theme instructions

            Cottser’s picture

            Issue summary: View changes

            Serial comma ;)

            Cottser’s picture

            Issue summary: View changes

            Fix alphabetical order

            Cottser’s picture

            I put together a Google docs spreadsheet today based on the sandbox conversion spreadsheet (in the issue summary now also):
            https://docs.google.com/spreadsheet/ccc?key=0AsJ6gAy_YfPadGlkX2lMU3F5S1p...

            We still need help working on patches, but now that I see the numbers, we really could use more reviewers! As I'm writing this we have 42 conversion patches at needs review that could use some eyes.

            Reviewers and contributors, drop by #drupal-twig on IRC if you have any questions or just leave a comment on the issue.

            Cottser’s picture

            Issue summary: View changes

            Add new spreadsheet to summary

            Cottser’s picture

            Issue summary: View changes

            Add ckeditor and datetime modules for conversion.

            Cottser’s picture

            Issue summary: View changes

            Add conversion spreadsheet higher up in the issue as well.

            Cottser’s picture

            Issue summary: View changes

            Update views and views_ui subissues now that views_ui is a meta

            joelpittet’s picture

            Issue summary: View changes

            added the $vars to $variables conversion as after commit

            Cottser’s picture

            We also need help manually testing almost all of the conversion patches to ensure we don't have regressions in markup or functionality. Search for issues that are at 'needs review' and tagged 'Twig' and 'Needs manual testing'.

            There are two main options for testing, the second option only requires a web browser:

            1. Compare two local installations of Drupal 8, one with the patch and one without.
            2. Use simplytest.me to compare. If you install Dreditor you'll get a simplytest.me button beside each patch on drupal.org.

            If you're testing locally, enabling $settings['twig_debug'] in settings.php will show HTML comments around the output of each Twig template that indicate which template the output comes from.

            If you find differences in the markup other than whitespace, please add a comment on the issue with the markup before and after applying the patch in <code> tags. If you spot any visual differences, before and after screenshots can be uploaded.

            If the patch works as expected, you can remove the 'Needs manual testing' tag after completing testing and commenting with your findings.

            Cottser’s picture

            Issue summary: View changes

            Add coding standards for templates and template preprocess functions

            ressa’s picture

            Issue summary: View changes

            Corrected link "Template preprocess function coding standards (RTBC draft)"

            jenlampton’s picture

            Issue summary: View changes

            actual blockers

            Cottser’s picture

            Issue summary: View changes

            Update spreadsheet links

            joelpittet’s picture

            Issue summary: View changes

            Added the entire theme.inc meta sub issues under for this overview.

            joelpittet’s picture

            Issue summary: View changes

            shuffle closed view issues

            joelpittet’s picture

            Issue summary: View changes

            move the droped views ui ones to the end

            thedavidmeister’s picture

            Issue summary: View changes

            Added an "after commit" issue

            joelpittet’s picture

            Issue summary: View changes

            added twig loop bug

            thedavidmeister’s picture

            A week later from #33 and we still have 43 issues tagged as "needs review".

            Things have obviously moved since then, look at http://drupal.org/project/issues/search/drupal?status%5B%5D=8&issue_tags... to see that none of our conversion issues have been ignored in over a week but we need to keep all these "yellow" issues moving to "red" (needs work) or "green" (RTBC) as fast as possible if we're to maintain our forward momentum - receiving a negative review for a patch you think is finished after a week or two without any update is much more demotivating than hearing the bad news straight away.

            If you're not actively responding to feedback on a patch you've written please make providing feedback for others your next highest priority - don't underestimate how important timely and quality reviews are to getting this issue over the line. Easy things to look out for that have been appearing in the reviews that I've been doing this weekend for patches that would otherwise be RTBC are:

            - Not adhering to the generic Drupal coding standards, particularly incorrect whitespace or documentation formatting.
            - Not using the latest Twig documentation standards, #1913208: [policy] Standardize template preprocess function documentation and [#1823416] are the references. Look out for "Preprocesses X" instead of "Prepare variables for X" in preprocess function comments. Look out for PHP data types being referenced in Twig templates (they shouldn't be).
            - Hardcoded classes/attributes in Twig templates instead of building these inside an Attribute object in the preprocess function.
            - Leftover process/theme functions that would be better deleted or merged into another function/template.
            - Incomplete documentation in Twig templates - Look out for variables being used in the template that aren't documented in the main @file docblock or maybe a variable is mis-named in the docblock. Remember that many developers will be relying on this documentation exclusively to write a template - they may not even realise that they can cross-reference the preprocess function to get the right name/structure of a variable.
            - Forgetting to reference the preprocess function with an @see in the Twig template docblock.

            These "little" things are all going to block a patch getting committed, are easy to forget yet easy to spot if you're looking out for them and are easy for the creator of a patch to fix up once they've been identified by a reviewer.

            Only once a green (according to testbots) patch has been cleaned out of all the "nitpick" obvious stuff like these those I've pointed out here should we be trying to manually A/B markup output from the Templates. If you are the author of a patch that needs manual review and you're submitting it to "needs review" status please provide step by step instructions showing how to generate/see each of the templates that need testing, see the issue summary of #1898432: node.module - Convert PHPTemplate templates to Twig for a good example.

            carsonblack’s picture

            Issue summary: View changes

            Updated issue summary.

            carsonblack’s picture

            Issue summary: View changes

            Updated issue summary.

            Cottser’s picture

            Issue summary: View changes

            theme_views_ui_rearrange_form() is gone!

            Cottser’s picture

            Issue summary: View changes

            Add a note that theme.inc issues appear in two places, remove issue referenced under image.module (should be covered by gut l() I think.)

            c4rl’s picture

            Issue summary: View changes

            Added link to Daisy Diff screencast

            c4rl’s picture

            Issue summary: View changes

            Formatting

            c4rl’s picture

            Issue summary: View changes

            Link to Daisy Diff

            jenlampton’s picture

            Issue summary: View changes

            add engine section

            jenlampton’s picture

            Issue summary: View changes

            reorder

            jenlampton’s picture

            Issue summary: View changes

            #

            jenlampton’s picture

            Issue summary: View changes

            autoescape

            webchick’s picture

            Just a note that https://github.com/LionsAd/twig-in-d8-core is a fork of core with all of the Twig conversions that are outstanding all in one spot!

            Should be an excellent way to both profile and get a sense of what all the markup looks like at the end of this.

            webchick’s picture

            Issue summary: View changes

            escape

            thedavidmeister’s picture

            Issue summary: View changes

            added an "after commit" issue

            Fabianx’s picture

            Just a note that https://github.com/LionsAd/twig-in-d8-core is a fork of core with all of the Twig conversions that are outstanding all in one spot!

            Should be an excellent way to both profile and get a sense of what all the markup looks like at the end of this.

            To be precise:

            * All RTBC and CNR issues are in there and new are added as they become ready.
            * Please use the develop branch - 8.x is the state of core this was forked from.

            Fabianx’s picture

            Issue summary: View changes

            Added two engine issues

            thedavidmeister’s picture

            Issue summary: View changes

            added an issue blocking progress

            Fabianx’s picture

            Issue summary: View changes

            Added new engine issue.

            Fabianx’s picture

            Issue summary: View changes

            Add another meta issue

            Cottser’s picture

            Consolidation is on the horizon. I propose we remove all @todos and comments from conversion patches that refer to consolidation or removal of templates. In my opinion the @todos are cluttering up our templates and making them less committable at this point in the code cycle. I also think consolidation issues will be easier to track in a meta issue than comments scattered across core :)

            …and, if we don't get to all of the consolidations, we're still improving themer experience significantly!

            I ran this idea by @c4rl and he approved.

            Consolidation @todo removal steps:

            A. If there is already an issue link in the @todo, make sure it is part of the issue summary in the consolidation meta: #1804614: [meta] Consolidate theme functions and properly use theme suggestions in core
            (if it's not in the issue summary, edit the issue summary to add it!)

            B. If there isn't an issue created for the proposed consolidation, create the issue, tag it with 'Twig' and 'theme system cleanup', and add it to the meta: #1804614: [meta] Consolidate theme functions and properly use theme suggestions in core. Post a link to the issue to the relevant Twig conversion issue as well.

            In both cases, roll a new patch to remove the @todo comment from the patch.

            Cottser’s picture

            Issue summary: View changes

            Attribution of issues

            Cottser’s picture

            Issue summary: View changes

            Update after commit issues - remove specific consolidation issue, add general consolidation issue. Add attribution @ sign magic.

            Cottser’s picture

            Issue summary: View changes

            Tweak capitalization on Dreditor, linkify simplytest.me

            thedavidmeister’s picture

            Issue summary: View changes

            added manual testing instructions

            Fabianx’s picture

            Issue summary: View changes

            Add performance issues.

            Fabianx’s picture

            I updated:

            https://github.com/LionsAd/twig-in-d8-core

            with the newest patches from here. After careful checking, I also included all "needs work" patches as those were mostly cosmetic issues.

            Unfortunately 12 patches failed to apply and need a re-roll:

            => ERROR: Convert node module to Twig #1898432: node.module - Convert PHPTemplate templates to Twig (needs review) -- http://drupal.org/files/twig-node-1898432-82.patch failed to apply. (http://drupal.org/node/1898432)
            => ERROR: Convert custom_block module to Twig #1898038: custom_block.module - Convert theme_ functions to Twig (reviewed &amp; tested by the community) -- http://drupal.org/files/1898038-34-twig-custom-block.patch failed to apply. (http://drupal.org/node/1898038)
            => ERROR: Convert taxonomy module to Twig #1898460: taxonomy.module - Convert PHPTemplate templates to Twig (needs review) -- http://drupal.org/files/1898460-42.patch failed to apply. (http://drupal.org/node/1898460)
            => ERROR: Convert system module to Twig #1898454: system.module - Convert PHPTemplate templates to Twig (needs review) -- http://drupal.org/files/drupal-1898454-33.patch failed to apply. (http://drupal.org/node/1898454)
            => ERROR: Convert views/templates/views-view.tpl.php to twig  #1843744: Convert views/templates/views-view.tpl.php to twig  (needs review) -- http://drupal.org/files/twig-views-view-1843744-45.patch failed to apply. (http://drupal.org/node/1843744)
            => ERROR: Convert views/templates/views-view.tpl.php to twig  #1843744: Convert views/templates/views-view.tpl.php to twig  (needs review) -- http://drupal.org/files/twig-views-view-1843744-45.patch failed to apply. (http://drupal.org/node/1843744)
            => ERROR: Convert theme_views_view_mapping_test() to Twig #1963764: Convert theme_views_view_mapping_test() to Twig (reviewed &amp; tested by the community) -- http://drupal.org/files/1963764-17.patch failed to apply. (http://drupal.org/node/1963764)
            => ERROR: Convert views/views_ui/templates/views-ui-display-tab-bucket.tpl.php to Twig #1843772: Convert views/views_ui/templates/views-ui-display-tab-bucket.tpl.php to Twig (reviewed &amp; tested by the community) -- http://drupal.org/files/1843772-20-twig-views-ui-tab-bucket.patch failed to apply. (http://drupal.org/node/1843772)
            => ERROR: Convert views/views_ui/templates/views-ui-display-tab-setting.tpl.php to Twig #1843774: Convert views/views_ui/templates/views-ui-display-tab-setting.tpl.php to Twig (needs work) -- http://drupal.org/files/1843774-29-twig-views-ui-display-tab-setting.patch failed to apply. (http://drupal.org/node/1843774)
            => ERROR: Convert theme_views_ui_style_plugin_table to Twig  #1918648: Convert theme_views_ui_style_plugin_table to Twig  (needs review) -- http://drupal.org/files/twig-views-ui-style-plugin-table-1918648-4.patch failed to apply. (http://drupal.org/node/1918648)
            => ERROR: Convert theme_views_ui_view_info() to Twig #1963986: Convert theme_views_ui_view_info() to Twig (needs review) -- http://drupal.org/files/1963986-13-twig-views-ui-view-info.patch failed to apply. (http://drupal.org/node/1963986)
            => ERROR: Convert theme_views_ui_view_preview_section() to Twig #1963988: Convert theme_views_ui_view_preview_section() to Twig (reviewed &amp; tested by the community) -- http://drupal.org/files/1963988-19-twig-views-ui-view-preview-section.patch failed to apply. (http://drupal.org/node/1963988)
            

            I already send a re-test to all of them, so they have been marked CNW automatically.

            Fabianx’s picture

            Issue summary: View changes

            fix typo

            thedavidmeister’s picture

            Issue summary: View changes

            added a performance issue

            Cottser’s picture

            The Twig team has decided to focus on .tpl.php to .html.twig conversions for the immediate future, with the goal of getting these conversions committed before or at DrupalCon Portland. As a result, some of our existing conversion issues will need to be split up and the .tpl.php conversions and related preprocess changes moved to the new issues. I've created a new section near the top of the issue summary to hold the direct PHPTemplate to Twig conversion issues.

            The main reasons why we decided to do this are:

            1. Significantly reduces the risk of Drupal 8 shipping with both .tpl.php and .html.twig files: focussing on removing PHPTemplate templates as soon as possible and ensuring good performance is much more achievable than trying to convert all templates and theme functions at once.
            2. Getting 35 issues RTBC all at once will be easier than getting 80+ issues RTBC all at once :)
            3. Allows more time to determine which theme functions should be converted to templates and which shouldn't. This will help us solve performance issues - i.e. #1986116: Improve performance by replacing very small and simple templates and theme function with "Markup Utility Functions"
            4. Although it's not the end game, just removing .tpl.php files and making Twig the default theme engine in Drupal 8 will be a big win by itself.

            We could use help splitting up the patches, I will split up aggregator.module as an example.

            Cottser’s picture

            Issue summary: View changes

            Add new issues focussing on PHPTemplate to Twig conversions, rearranging.

            jenlampton’s picture

            Title: [meta] Convert core theme functions and templates to Twig » [meta] Convert core theme functions to Twig template files
            Priority: Critical » Major

            Okay, Since this issue is getting much too big, I've created #1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch

            We should do all the template file conversions over there, and leave the bulk of our work (theme functions) here where it started. Changing title to match new plan for this issue.

            I suppose that means this issue doesn't have to be critical anymore (since the critical stuff is all over in #1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch) so changing priority back to major.

            c4rl’s picture

            Though I suppose I agree with the premise of converting the templates first, then approaching the concatenation-based functions, I can't say that I understand wholly (and subsequently can't say I agree wholly) with the administrative decisions that have been made in the past 24 hours. Why were these decisions not saved until the weekly call?

            #40

            some of our existing conversion issues will need to be split up and the .tpl.php conversions and related preprocess changes moved to the new issues

            Why? Why not use existing issues? For example, why was it decided that instead of changing the scope and direction of #1898432: node.module - Convert PHPTemplate templates to Twig to simply include the tpl files we now have #1987406: node.module - Convert theme_ functions to Twig.

            Do the original issues that were part of this meta issue have relevance any longer? What is now the purpose of the former existing issues? Should they be postponed?

            Cottser’s picture

            Sorry for any confusion @c4rl, and great suggestion.

            Slight change of plan:
            We will re-use existing issues for the .tpl.php conversions e.g. #1898432: node.module - Convert PHPTemplate templates to Twig because they have more followers and we can use those eyes on the higher priority issues. We'll still split up the patches as discussed in #40 (theme_ function conversions in one issue, .tpl.php conversions in a separate issue), but we'll instead move the theme_ function conversions to the newly-created issues since they are not our highest priority at this time.

            I also didn't make clear in #40 that @Fabianx and I discussed this course of action last night on IRC after the recent discussions with the whole team and decided to push forward with the plan despite the fact that we were not able to consult with the entire team at the time.

            c4rl’s picture

            Okay, I am going to start retitling existing issues to make the clarification that they are for tpl.php conversions and the new issues that were created last night should be fore theme functions.

            Also, RE: #41, @jenlampton, can you describe why you created a second meta issue? I really don't think we need one, it just gives us more documentation to maintain. What are your thoughts?

            c4rl’s picture

            Issue summary: View changes

            moved other issues to other meta

            c4rl’s picture

            Issue summary: View changes

            Revise formatting given new direction to prioritize .tpl.php conversions first.

            c4rl’s picture

            Title: [meta] Convert core theme functions to Twig template files » [meta] Convert core PHPTemplate files and theme functions to Twig

            Retitling such that per my second comment on #44, I think this issue should accommodate both PHPTemplate file conversion and theme_ function conversion.

            c4rl’s picture

            Issue summary: View changes

            Emphasize issues

            c4rl’s picture

            I've revised the issue summary so that the highest priority issues (i.e. those that contain .tpl.php files) are shown in bold, prefixed with the word [HIGH].

            c4rl’s picture

            Issue summary: View changes

            Add back in new theme_ function issues

            Cottser’s picture

            Issue summary: View changes

            Add seven theme functions issue

            Cottser’s picture

            Issue summary: View changes

            Moving high priority issues to the top and re-adding views and views ui sub-issues so we can see the status of all 35 high priority issues at a glance

            Cottser’s picture

            Issue summary: View changes

            Updated issue summary.

            Cottser’s picture

            Issue summary: View changes

            Add note about conversion doc

            c4rl’s picture

            If the sentiment expressed in #41 that this meta is getting "too big," let's remove any sub-issue references on this issue above that are not marked with [HIGH] (i.e., those that do not contain .tpl.php files) and leave #1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch as-is.

            If, OTOH, if the formatting improvements I made makes this present issue seem more manageable, I vote we simply use this present issue as the sole meta and mark #1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch as duplicate.

            @jenlampton, let me know your thoughts :)

            jenlampton’s picture

            Priority: Major » Critical

            I created another issue because we need someplace to put the patch. "The patch" being the patch that goes into core, the one that contains all the .tpl.php file conversions. The one that is the aggregate of all the tpl.php issues.

            Moving that first patch onto it's own issue will prevent this one from being closed when that gets in. I don't want to loose the momentum on this issue, so I want it to remain open. I guess that makes this a meta-meta issue :)

            jenlampton’s picture

            Issue summary: View changes

            Move sandbox link down to reference area

            c4rl’s picture

            #48@jenlampton, okay that makes sense.

            c4rl’s picture

            Issue summary: View changes

            benchmark

            c4rl’s picture

            Issue summary: View changes

            Formatting: Strong tags were a bit, well, strong :)

            thedavidmeister’s picture

            Issue summary: View changes

            added after commit issue

            joelpittet’s picture

            Issue summary: View changes

            Phrasing and caps

            thedavidmeister’s picture

            Issue summary: View changes

            Updated issue summary.

            jwilson3’s picture

            Issue summary: View changes

            added a "resources" section with links to twig best practices.

            jwilson3’s picture

            Issue summary: View changes

            make the resources into bullet list

            YesCT’s picture

            These are the ones that need profiling. The functions ones are low priority, the template to twig ones are higher priority.
            http://drupal.org/project/issues/search/drupal?status%5B%5D=8&issue_tags...

            cosmicdreams’s picture

            I added #1999214: Add a commented out debug line to all templates to show which variables are available to request adding a comment to each template about the dump(_context) techique for revealing all the variables available to a theme.

            cosmicdreams’s picture

            Issue summary: View changes

            Remove conversion spreadsheet links for now, no longer actively in use for .tpl.php conversions

            geoffreyr’s picture

            Issue summary: View changes

            Added #1964156 to twig engine issues at request of fabianx.

            Sean Charles’s picture

            Issue summary: View changes

            Updated issue summary.

            johannez’s picture

            Issue summary: View changes

            minor change

            joelpittet’s picture

            Issue summary: View changes

            ideas from Jen

            jenlampton’s picture

            Issue summary: View changes

            remove greens

            joelpittet’s picture

            Issue summary: View changes

            updated gist url to the pretty one

            jenlampton’s picture

            Issue summary: View changes

            bartik

            jenlampton’s picture

            Issue summary: View changes

            preprocess cleanup

            joelpittet’s picture

            Issue summary: View changes

            added attribute class issue

            joelpittet’s picture

            Issue summary: View changes

            add profiling

            Cottser’s picture

            Issue summary: View changes

            Work in progress on updating issue summary to "move on up" format ala @thedavidmeister :)

            Cottser’s picture

            Issue summary: View changes

            Bit more organization

            Cottser’s picture

            Issue summary: View changes

            Move up theme.maintenance.inc for another round of manual testing.

            Cottser’s picture

            Issue summary: View changes

            Update link to preprocess standards now that they are in 1354, move up forum issue for manual testing

            Cottser’s picture

            Issue summary: View changes

            Move aggregator up for manual testing

            Cottser’s picture

            Issue summary: View changes

            Move comment up for manual testing

            Cottser’s picture

            Issue summary: View changes

            Move common_test to RTBC-land

            Cottser’s picture

            Issue summary: View changes

            Move datetime to needs review

            joelpittet’s picture

            Issue summary: View changes

            Added TODO list, and shuffled around a couple.

            joelpittet’s picture

            Issue summary: View changes

            removed duplicate todo in favour of the todo page.

            joelpittet’s picture

            Issue summary: View changes

            shuffle around the todo

            joelpittet’s picture

            Issue summary: View changes

            cleanup format on uls

            joelpittet’s picture

            Issue summary: View changes

            locale to profiling

            joelpittet’s picture

            Issue summary: View changes

            toolbar and update to CNW

            joelpittet’s picture

            Issue summary: View changes

            rdf to needs work

            Cottser’s picture

            Issue summary: View changes

            Move locale back to RTBC

            joelpittet’s picture

            Issue summary: View changes

            form and menu moved to needs work

            joelpittet’s picture

            Issue summary: View changes

            added theme.inc, views and views ui theme functions from meta.

            joelpittet’s picture

            Issue summary: View changes

            Adding items to the fixed pile

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            joelpittet’s picture

            Issue summary: View changes

            moving items into needs work

            joelpittet’s picture

            Issue summary: View changes

            moved to needs profiling

            joelpittet’s picture

            Issue summary: View changes

            moved a couple more into needs profiling.

            joelpittet’s picture

            Issue summary: View changes

            moved some up to rtbc

            joelpittet’s picture

            Issue summary: View changes

            moving indent to needs review

            Cottser’s picture

            Issue summary: View changes

            Remove bartik theme_ function meta since it is no longer needed

            joelpittet’s picture

            Issue summary: View changes

            shuffle

            joelpittet’s picture

            Issue summary: View changes

            moving things into needs review

            joelpittet’s picture

            Issue summary: View changes

            moving views container to needs review

            joelpittet’s picture

            Issue summary: View changes

            moving views items up to needs review

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            joelpittet’s picture

            Issue summary: View changes

            Postponed section

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            joelpittet’s picture

            Issue summary: View changes

            shuffle

            Cottser’s picture

            Issue summary: View changes

            Add a new reroll

            Cottser’s picture

            Issue summary: View changes

            shorten up description

            Cottser’s picture

            Issue summary: View changes

            Add another reroll, fix markup

            Cottser’s picture

            Issue summary: View changes

            Add another reroll

            jenlampton’s picture

            Issue summary: View changes

            upate

            Cottser’s picture

            Issue summary: View changes

            Move issues up

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            joelpittet’s picture

            Issue summary: View changes

            Shuffle

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            joelpittet’s picture

            Issue summary: View changes

            moving

            joelpittet’s picture

            Issue summary: View changes

            moved menu to postponed

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            joelpittet’s picture

            Issue summary: View changes

            add re-roll search

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            Cottser’s picture

            Issue summary: View changes

            Move up link module

            thedavidmeister’s picture

            Issue summary: View changes

            Updated issue summary.

            thedavidmeister’s picture

            Issue summary: View changes

            Updated issue summary.

            thedavidmeister’s picture

            Issue summary: View changes

            Updated issue summary.

            thedavidmeister’s picture

            Issue summary: View changes

            Updated issue summary.

            thedavidmeister’s picture

            Issue summary: View changes

            Updated issue summary.

            thedavidmeister’s picture

            Issue summary: View changes

            Updated issue summary.

            thedavidmeister’s picture

            Issue summary: View changes

            Updated issue summary.

            jhodgdon’s picture

            For anyone working on these Twig conversions: Note that there had been an error in the documentation standards for Twig template files.

            We no longer want a line saying:

             * @see template_preprocess()
            

            in the documentation block.

            We removed this line from the standards, and removed the offending lines from existing Twig templates on
            #2013094: [policy adopted, patch needed] Stop saying '@see template_preprocess()' in every twig file
            but let's make sure no more creep in with future conversions. Thanks!

            jhodgdon’s picture

            Issue summary: View changes

            indent down to needs work

            jenlampton’s picture

            Issue summary: View changes

            cleanup

            catch’s picture

            Title: [meta] Convert core PHPTemplate files and theme functions to Twig » [meta] Convert core PHPTemplate theme functions to Twig
            Priority: Critical » Major

            Theme function conversions no longer block a release, so I think this can be 'major'. It'd be great to get as many in as possible (as well as the complete removal ones), but if we don't it just means we didn't get to them all.

            catch’s picture

            Issue summary: View changes

            major cleanup

            jenlampton’s picture

            Issue summary: View changes

            reorder

            jenlampton’s picture

            Issue summary: View changes

            dbw

            jenlampton’s picture

            Issue summary: View changes

            update

            jenlampton’s picture

            Issue summary: View changes

            reorder

            jenlampton’s picture

            Issue summary: View changes

            split profiling problems

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            Cottser’s picture

            Title: [meta] Convert core PHPTemplate theme functions to Twig » [meta] Convert core theme functions to Twig templates

            Just clarifying the title a bit.

            Cottser’s picture

            Issue summary: View changes

            Moving bits around

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            joelpittet’s picture

            Issue summary: View changes

            meter closed

            joelpittet’s picture

            Issue summary: View changes

            Updated issue summary.

            jenlampton’s picture

            Issue summary: View changes

            :

            jenlampton’s picture

            Issue summary: View changes

            reorg

            jenlampton’s picture

            Issue summary: View changes

            reorg

            jenlampton’s picture

            Issue summary: View changes

            reorg

            jenlampton’s picture

            Issue summary: View changes

            reorg

            jenlampton’s picture

            Issue summary: View changes

            .

            jenlampton’s picture

            Issue summary: View changes

            rtbc

            jenlampton’s picture

            Issue summary: View changes

            profiling

            Pages