forum.module - Convert theme_ functions to Twig

Per #1757550-44: [Meta] Convert core theme functions to Twig templates, retitling to indicate this issue applies to theme_ functions rather than templates. See #1898418: forum.module - Convert PHPTemplate templates to Twig for template conversion.

Updated issue summary.

#2: twig-forum-theme-only-1987400-2.patch queued for re-testing.

profiling this now

=== 8.x..8.x compared (519fe63632755..519feb8ccb3f5):

ct : 40,580|40,580|0|0.0%
wt : 377,694|376,751|-943|-0.2%
cpu : 335,787|335,069|-718|-0.2%
mu : 50,206,616|50,206,616|0|0.0%
pmu : 50,271,728|50,271,728|0|0.0%

Profiler output

=== 8.x..1987400 compared (519fe63632755..519febd624b4e):

ct : 40,580|40,580|0|0.0%
wt : 377,694|377,921|227|0.1%
cpu : 335,787|336,040|253|0.1%
mu : 50,206,616|50,206,712|96|0.0%
pmu : 50,271,728|50,272,256|528|0.0%

Profiler output

---
ct = function calls, wt = wall time, cpu = cpu time used, mu = memory usage, pmu = peak memory usage
The registry has been rebuilt. [success]
'all' cache was cleared in self

removing Profiling tag

profiling not run correctly. Needs to be redone

I'll profile this.

Stark theme. After a bit of digging, determined that admin/structure/forum/add/forum needed to be my home page. Allowed Anonymous to modify forums. Tweaked bbranches to restart apache after each test to clear APC cache.

Forgive my flakey little laptop, it's 8.x vs 8.x numbers have been consistently non-zero.

=== 8.x..8.x compared (51a27ce83196d..51a27d3f1f1ae):

ct : 38,632|38,632|0|0.0%
wt : 163,432|166,046|2,614|1.6%
cpu : 160,000|164,000|4,000|2.5%
mu : 11,223,208|11,223,208|0|0.0%
pmu : 11,337,400|11,337,400|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51a27ce83196d&...

=== 8.x..twig-forum-theme-only-1987400-2 compared (51a27ce83196d..51a27d81b1262):

ct : 38,632|38,720|88|0.2%
wt : 163,432|160,821|-2,611|-1.6%
cpu : 160,000|156,000|-4,000|-2.5%
mu : 11,223,208|11,275,312|52,104|0.5%
pmu : 11,337,400|11,389,952|52,552|0.5%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=51a27ce83196d&...

List of functions

Remove which preprocess function?

Thanks! Okay here is the re-rolled patch without "template_preprocess_forum_form" and I also removed a doc reference to template_preprocess_forum_form() in one of the twig files.

#13: twig-forum-theme-only-1987400-13.patch queued for re-testing.

manual testing is a novice task

re-rolled and removed @see template_preprocess() from the twig template.

Removed theme_forum_form

rerolled

Thanks @CaptainWonky

Manual testing looks good.

+<!-- BEGIN OUTPUT from 'core/modules/forum/templates/forum-form.html.twig' -->
 <div class="form-item form-type-textfield form-item-name">
  <label for="edit-name">Forum name<abbr class="form-required" title="This field is required.">*</abbr></label> <input aria-describedby="edit-name--description" type="text" id="edit-name" name="name" value="General discussion" size="60" maxlength="255" class="form-text required" required="required" aria-required="true" />
 <div class="description" id="edit-name--description">Short but meaningful name for this collection of threaded discussions.</div>
@@ -17,8 +18,10 @@
  <label for="edit-parent-0">Parent<abbr class="form-required" title="This field is required.">*</abbr></label> <select aria-describedby="edit-parent-0--description" id="edit-parent-0" name="parent[0]" class="form-select required" required="required" aria-required="true"><option value="0" selected="selected">&lt;root&gt;</option></select>
 <div class="description" id="edit-parent-0--description">Forums may be placed at the top (root) level, or inside another container or forum.</div>
 </div>
-<input type="hidden" name="form_build_id" value="form-LfltB-bkQb37R9Kgf3WklYCxLOA-lLv7I-3-x9CFb78" /><input type="hidden" name="form_token" value="vMOzncStTLdU8IxgvwUd69C8ixMZoS1xiOARLt9Cds8" /><input type="hidden" name="form_id" value="forums_taxonomy_term_forum_form" /><div class="form-item form-type-textfield form-item-path-alias">
+<input type="hidden" name="form_build_id" value="form-r00KvyqgPQyfySY4NdC9gRucGcFciYNhZ8vnDikg2oQ" /><input type="hidden" name="form_token" value="vMOzncStTLdU8IxgvwUd69C8ixMZoS1xiOARLt9Cds8" /><input type="hidden" name="form_id" value="forums_taxonomy_term_forum_form" /><div class="form-item form-type-textfield form-item-path-alias">
  <label for="edit-path-alias">URL alias</label> <input aria-describedby="edit-path-alias--description" type="text" id="edit-path-alias" name="path[alias]" value="" size="60" maxlength="255" class="form-text" />
 <div class="description" id="edit-path-alias--description">Optionally specify an alternative URL by which this term can be accessed. Use a relative path and don't add a trailing slash or the URL alias won't work.</div>
 </div>
 <div class="form-actions form-wrapper" id="edit-actions"><input type="submit" id="edit-submit" name="op" value="Save" class="button button-primary form-submit" /><input type="submit" id="edit-delete" name="op" value="Delete" class="button button-danger form-submit" /></div>
+
+<!-- END OUTPUT from 'core/modules/forum/templates/forum-form.html.twig' -->

Btw that ^^ is a diff between 8.x and this branch. The only diff being the form_build_id and a newline character extra at the EOF.

Updated issue summary.

Going to profile this on the plane tomorrow :)

Scenario:

• Forum module turned on
• Frontpage set to admin/structure/forum/add/forum
• Administer forums permissions turned on

=== 8.x..8.x compared (523bd0c5929e9..523bd1075886b):

ct  : 47,754|47,754|0|0.0%
wt  : 324,917|326,667|1,750|0.5%
cpu : 292,222|293,061|839|0.3%
mu  : 14,855,544|14,855,544|0|0.0%
pmu : 14,945,824|14,945,824|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=523bd0c5929e9&...

=== 8.x..1987400-twig-theme-forum compared (523bd0c5929e9..523bd15c0308e):

ct  : 47,754|47,842|88|0.2%
wt  : 324,917|327,140|2,223|0.7%
cpu : 292,222|294,689|2,467|0.8%
mu  : 14,855,544|14,908,288|52,744|0.4%
pmu : 14,945,824|15,006,088|60,264|0.4%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=523bd0c5929e9&...

Here's a minor re-roll too that I needed to do before profiling.

Patch looks good it is ready to fly. Minor issue.

+++ b/core/modules/forum/templates/forum-form.html.twig
@@ -0,0 +1,13 @@
+ * @see

Should be @file can be fixed at the time of commit.

Thanks at @jibran

#31: 1987400-31-twig-forum-theme.patch queued for re-testing.

Postponing this on #2092343: Consolidate forum.module and remove call to _theme_table_cell() within template_preprocess_forum_topic_list()

Discussed with @markcarver don't want to send another single variable template conversion through to core.

Once that one get's sorted this can be closed as duplicate or won't fix.

Update conversion table

31: 1987400-31-twig-forum-theme.patch queued for re-testing.

Re-roll. Maybe consider just removing this?

I tried that and it failed on both attempts locally.

diff --git a/core/modules/forum/lib/Drupal/forum/Form/ForumFormController.php b/core/modules/forum/lib/Drupal/forum/Form/ForumFormController.php
index ae16f90..3c267ef 100644
--- a/core/modules/forum/lib/Drupal/forum/Form/ForumFormController.php
+++ b/core/modules/forum/lib/Drupal/forum/Form/ForumFormController.php
@@ -54,7 +54,7 @@ public function form(array $form, array &$form_state) {
     $form['parent']['#tree'] = TRUE;
     $form['parent'][0] = $this->forumParentSelect($taxonomy_term->id(), $this->t('Parent'));

-    $form['#theme'] = 'forum_form';
+    $form['#theme_wrappers'] = 'form__forum';
     $this->forumFormType = $this->t('forum');
     return $form;
   }

and

diff --git a/core/modules/forum/lib/Drupal/forum/Form/ForumFormController.php b/core/modules/forum/lib/Drupal/forum/Form/ForumFormController.php
index ae16f90..3c267ef 100644
--- a/core/modules/forum/lib/Drupal/forum/Form/ForumFormController.php
+++ b/core/modules/forum/lib/Drupal/forum/Form/ForumFormController.php
@@ -54,7 +54,7 @@ public function form(array $form, array &$form_state) {
     $form['parent']['#tree'] = TRUE;
     $form['parent'][0] = $this->forumParentSelect($taxonomy_term->id(), $this->t('Parent'));

-    $form['#theme'] = 'forum_form';
+    $form['#theme'] = 'form__forum';
     $this->forumFormType = $this->t('forum');
     return $form;
   }

Maybe you want to take a stab at this @Cottser?

Re-rolling based on #39. This seems to be working locally, let's see what test bot has to say

@jerdavis just noticed that this is missing the twig template. Would you mind adding in?

@Cottser, I think we should push this one in and we can still work on that other issue for consolidation of these.
#2265991: Replace theme_*_form() with theme suggestion for #theme => form.

I think no reason to consolidate removals by allowing module maintainers to review the changes.
Suppose better to re-title this to "remove theme_forum_form()"

+++ b/core/modules/forum/src/Form/ForumForm.php
@@ -54,7 +54,7 @@ public function form(array $form, array &$form_state) {
+    $form['#theme_wrappers'] = 'form__forum';

should be array

interdiff

@andypost this seems like a good solution... just can't seem to test it out. I'd assume you could just go to admin/structure/forum/add/forum after installing the forum module and see the form. or admin/structure/taxonomy/manage/forums/add

But none of them produce this form from what I can see. I wanted to just see if the seven or bartik template override of form.html.twig as form--forum.html.twig would pick up. Any idea?

reroll

this form used in forum.add_forum route /admin/structure/forum/add/forum

no difference

before

after

Thanks for the re-roll @andypost and for confirming the path for me. I was looking for the string "Short but meaningful name for this collection of threaded discussions." But that didn't show up, but either something else is broken or it's getting overwritten someplace else, nevertheless out of the scope of this.

I gave this a test and it works like a charm:

1. Added a copy of form.html.twig to: /themes/seven/templates/form--forum.html.twig
2. Turned on twig debug.
3. Rebuilt cache.
4. Went to /admin/structure/forum/add/forum

Just a note, the before and after screenshots you posted don't give much detail though they are appreciated, showing that nothing has changed can yield false positives.

Committed 5a07eb9 and pushed to 8.x. Thanks!