Convert theme_input() to Twig

Let see... Not sure about it but it's a try :)

+++ b/core/modules/system/templates/input.html.twig
@@ -0,0 +1,17 @@
+<input {{ attributes }} />{{ children }}

There a space between input and attributes that should be removed.

I remove the space as suggested in comment #3.

Patch attached.

4: 2152219-convert-theme-input-4.patch queued for re-testing.

2: 2152219-convert-theme-input.patch queued for re-testing.

8: drupal_core-convert-theme-input-2152219-8.patch queued for re-testing.

4: 2152219-convert-theme-input-4.patch queued for re-testing.

8: drupal_core-convert-theme-input-2152219-8.patch queued for re-testing.

This last patch fixes the new line problem and removes the unnecessary space.

1. +++ b/core/includes/form.inc
   @@ -2167,33 +2167,18 @@ function form_process_autocomplete($element, &$form_state) {
   + * Preprocesses variables for input templates.
     *
     * @param array $variables
   

   Missing the
   * Default template: input.html.twig.

2. +++ b/core/includes/form.inc
   @@ -2167,33 +2167,18 @@ function form_process_autocomplete($element, &$form_state) {
   +  $variables['children'] = (!empty($element['#children'])) ? $element['#children'] : '';
   

   #children key is defined already in drupal_render(), don't do the empty() check which likely breaks printing 0s

3. +++ b/core/modules/system/templates/input.html.twig
   @@ -0,0 +1,17 @@
   + @todo: remove this file once http://drupal.org/node/1819284 is resolved.
   + This is identical to core/modules/system/templates/container.html.twig
   

   I don't think this @todo is relevant.

Here's a patch and an interdiff with the changes joelpittet suggested.

Perfecto, thanks @mdrummond. Adding tags back on this one.

Here's a patch and interdiff with the changes Cottser suggested.

This patch is green. Manual testing and profiling up next?

Manually testing, and I'm surprised that removing the new Attribute() actually worked!! FAPI's render element's got some magic I am not aware of... magic-- $variables = $element somwhere...

So what's next. Profiling?

Manual Testing first, then profiling. Just need markup comparison screenshots or diff files. I usually turn twig debugging on so I can see that it's rendering the template first and second so I can remove the markup before and after on both with and without the patch to compare the two a bit easier.

I was looking at this this morning and spun up a simpletest.me instance. Input fields are working fine but I suspect there's more to testing this than just patching, hitting a page and viewing source.

What sort of report/testing should be done to verify issues like this? Is there a page outlining how to test and report the results of Twigifying things?

@Gold, hi! For a second I didn't think we had anything formal:P http://drupalladder.org/lesson/05820dfa-a09d-9014-511f-5779d9915adc

and https://drupal.org/node/2048869

@joelpittet, cheers. Spinning up a clean copy now. Will create a content type with all input fields and grab the source, apply the patch and repeat.

Have updated the issue with details from the link you provided also.

This has not worked as expected unfortunately.

The source of a page with as many different input fields as are available in a standard profile install, both before and after the patch here: http://dropbucket.org/node/1140

There is also a diff there.

Looking at the screencaps though it appears to be causing issues with;

• Title
• checkboxen
• Entity Reference
• File
• Number fields (all types)
• Text
• Submit

Did you clear cache after applying the patch? The only way I can reproduce #33 is by applying the patch and then not clearing cache before testing - if I browse to /admin/config/people/accounts without clearing cache, I also see missing textfields, radio buttons, etc., but then if I clear the cache then all the input fields appear as expected.

edit: realised you will need to use Drush or the rebuild.php script to clear cache, as the button on the performance page disappears when the patch is applied!

@longwave, I will retest.

I'm newish to the d8 testing process and didn't know about the rebuild.php script. Here at work I'm also tied to the current stable drush (which doesn't operate with D8) so couldn't cc all. Will try and fix that today too.

@Gold feel free to ask any questions in #drupal-twig on IRC if you'd prefer. There is usually a couple of us hanging out there for any troubles you have. Thanks again for looking at this issue. Drupal 8's version of drush removed the 'drush cc all' in favour of 'drush cr'.

A nice way to use drush is have it on your computer from github and switch branches if you need an older version but then you can keep it up to date too with a git pull.

@longwave, you rock. :)

After using the rebuild.php script and checking again all the fields look as they did before patching.

I've not bothered with a diff this time. Looking at it, the only items that fell out were the dates in the value attributes for the date & time fields. The reset were form_build_id and url tokens in the css/js in the head.

This is looking good for me now.

Failed to update the status with #37. Or does this need more testing before this status is reached?

We need profiling before we can RTBC this, at least as far as I understand the process.

@mdrummond Cheers. Knocked the Needs manual testing tag off at least. :)

@Gold Indeed! Great work! One step at a time.

Here's the info on profiling: https://drupal.org/contributor-tasks/profiling

The basic idea is to test the performance for before the patch is applied and after the patch is applied. Ideally performance is pretty close after the patch.

I found it challenging to get my computer set up to do profiling, but I did get it working eventually. Making sure my laptop was plugged in, and that I had all programs except for Terminal closed, after a reboot, was helpful. What took me a while was getting consistent performance over multiple tests for the before patch scenario. That has to be nearly identical in order to have a valid benchmark to measure the change in performance due to the Twig conversion. If you want to try profiling, hope those tips help!

+++ b/core/includes/form.inc
@@ -2167,33 +2167,18 @@ function form_process_autocomplete($element, &$form_state) {
-  $variables['attributes'] = new Attribute($element['#attributes']);

I'm surprised this worked, unless there is some magic I don't know about this should be:
$variables['attributes'] = $element['#attributes'];

Don't need the instantiation but still need to variable pass along from $element to $variables.

Yeah... Between my projects at home and the development environment here at work I doubt I have a system that is stable/consistant enough for profiling.

Hmm... Another project comes to mind. very cut down RaspberryPi running just the barebones for D8 profiling... That could work. :)

That would be a nice tool for profiling, I bet the results would be more accurate than my laptop. We should fix or figure out #43 before we start profiling though.

reroll
I'm not sure how to make the interdiff

@rodrigoaguilera you don't need to do an interdiff for a re-roll though, just the changes you make that weren't part of the original.

Full instructions on how to do an interdiff: https://drupal.org/documentation/git/interdiff
The gist of it is a diff between commits, and the shortcut in git is: git show > interdiff.txt

The slightly longer one would be
git diff HEAD~1..HEAD > interdiff.txt

And if you would like a workflow to understand try mine:) http://pittet.ca/drupal/sprint/patch

Patch Needs profiling , I'm not so good at that !!

I'm just curious what ends up in children, the docs are vague so maybe the tests will give hints.

Here's the fixes mentioned in #57 + a couple more.

So though I'd like to just remove that, if they're is a template that needs {{ children }} they could just pass them through the preprocess or write them directly in the template.

But I don't want to break anything and this is just a conversion so maybe we can just RTBC #59 or add a better comment then RTBC;)

Thanks for the reference, I still think if need be contrib could add in that to preprocess and a template but that's for another issue.

+++ b/core/modules/system/templates/input.html.twig
@@ -0,0 +1,15 @@
+ * - children: The rendered child elements.

Can we just change this to: "Optional additional rendered elements."?

Cool, that's changed. Profiling next, stop me if we don't need it for this?

Time for profiling and a bit monkey work.

Profiling data

Run 538ce4363dd76 uploaded successfully for drupal-perf-steinmb.
Run 538ce46e85e63 uploaded successfully for drupal-perf-steinmb.

=== 8.x..8.x compared (538ce4363dd76..538ce46e85e63):

ct  : 28,621|28,621|0|0.0%
wt  : 245,677|245,486|-191|-0.1%
cpu : 223,421|223,704|283|0.1%
mu  : 18,718,744|18,718,744|0|0.0%
pmu : 18,784,568|18,784,568|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=538ce4363dd76&...

Run 538ce4363dd76 uploaded successfully for drupal-perf-steinmb.
Run 538ce49a96f74 uploaded successfully for drupal-perf-steinmb.

=== 8.x..2152219-twig-theme_input compared (538ce4363dd76..538ce49a96f74):

ct  : 28,621|28,905|284|1.0%
wt  : 245,677|247,614|1,937|0.8%
cpu : 223,421|225,372|1,951|0.9%
mu  : 18,718,744|18,738,400|19,656|0.1%
pmu : 18,784,568|18,804,272|19,704|0.1%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=538ce4363dd76&...

Markup compared and diffed

--- /Users/steinmb/Downloads/foo.html Mon Jun  2 16:05:36 2014
+++ /Users/steinmb/Downloads/foo.html Mon Jun  2 16:12:28 2014
@@ -1,6 +1,7 @@
 <div class="form-item form-type-textfield form-item-title-0-value">
       <label class="form-required" for="edit-title-0-value">Title</label>
       <input type="text" aria-required="true" required="required" placeholder="" maxlength="255" size="60" value="" name="title[0][value]" id="edit-title-0-value" aria-describedby="edit-title-0-value--description" class="text-full form-text required">
+
           <div id="edit-title-0-value--description" class="description">
       The title of this node, always treated as non-markup plain text.
     </div>

Committed 406da31 and pushed to 8.x. Thanks!