Problem/Motivation

Because the new default for hook_theme() is now 'template', all 'template' declarations can now be removed.

See #2226207: Make 'template' the default output option for hook_theme()

For example, node_theme() looks like this now:

/**
 * Implements hook_theme().
 */
function node_theme() {
  return array(
    'node' => array(
      'render element' => 'elements',
      'template' => 'node',
    ),
    …
  );

But it could look like this:

/**
 * Implements hook_theme().
 */
function node_theme() {
  return array(
    'node' => array(
      'render element' => 'elements',
    ),
    …
  );

Proposed resolution

Remove all 'template's from hook_theme() declarations.

This can be scripted/bulk search & replaced using a regular expression like this:

\s+'template' => '[a-zA-Z-]+',?

Remaining tasks

User interface changes

n/a

API changes

n/a

CommentFileSizeAuthor
#20 2246675-interdiff-19.txt1.46 KBrpayanm
#20 2246675-20.patch30 KBrpayanm
#17 2246675-17.patch30.64 KBrpayanm
#5 remove-template-from-hook-2246675-5.patch4.08 KBjohnstorey
#3 remove-template-from-hook-2246675-3.patch5.46 KBholly.ross.drupal
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mgbellaire’s picture

mgbellaire CreditAttribution: mgbellaire commented
Status: Active » Postponed
Issue tags: -API change, -Approved API change
Parent issue: » #2226207: Make 'template' the default output option for hook_theme()
star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Issue summary: View changes

Tweaking the issue summary a bit, thank you for starting this @mgbellaire :D

holly.ross.drupal’s picture

holly.ross.drupal CreditAttribution: holly.ross.drupal commented
Assigned: mgbellaire » Unassigned
Status: Postponed » Needs review
FileSize
remove-template-from-hook-2246675-3.patch5.46 KB

Removed all references to 'template' => in hook_theme() declarations.

Unsure if the reference to "template" on line 615 (and several others) is still relevant and need to be removed as well.

Status: Needs review » Needs work

The last submitted patch, 3: remove-template-from-hook-2246675-3.patch, failed testing.

johnstorey’s picture

johnstorey CreditAttribution: johnstorey commented
FileSize
remove-template-from-hook-2246675-5.patch4.08 KB

Re-rolled patch. Apparently since @holly.ross.drupal rolled the original one, two 'variables' declarations were removed, thus breaking the patch. The two declarations were:

- Around line 2594, in the 'maintenance_page' array
- Around line 2598, in the 'install_page' array

Once the patch was adjusted for this it applied cleanly on my local system.

dcam’s picture

dcam CreditAttribution: dcam commented
Status: Needs work » Needs review

Sending this to the Testbot.

Status: Needs review » Needs work

The last submitted patch, 5: remove-template-from-hook-2246675-5.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Needs work » Postponed

I think it probably makes sense to keep this postponed until #2226207: Make 'template' the default output option for hook_theme() is committed. It is scriptable. Thanks for the efforts @holly.ross.drupal and @johnstorey but it won't pass testing until we get the other issue in and we will just have to keep rerolling it. Trying to save time :)

Status: Postponed » Needs review

Saphyel queued 5: remove-template-from-hook-2246675-5.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 5: remove-template-from-hook-2246675-5.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Needs work » Postponed

#8 still applies.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Issue summary: View changes

Adding a potential regular expression to the issue summary.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Issue summary: View changes

Better.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Issue summary: View changes

Updating the regular expression slightly.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Issue summary: View changes

Scratch that, those ones are special cases anyway. Carry on :)

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Title: Remove all 'template's in hook_theme() declarations » Remove all unnecessary 'template's in hook_theme() declarations
Status: Postponed » Active

Let's do this, #2226207: Make 'template' the default output option for hook_theme() is in!

My suggestion is that a patch can be crafted here based on #2226207-149: Make 'template' the default output option for hook_theme(), but removing the changes that were committed in #2226207-182: Make 'template' the default output option for hook_theme(). git apply -R (reverse) might be helpful.

Or we can just re-script it.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
CreditAttribution: rpayanm commented
Status: Active » Needs review
FileSize
2246675-17.patch30.64 KB

Trying...

Status: Needs review » Needs work

The last submitted patch, 17: 2246675-17.patch, failed testing.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented

Thanks @rpayanm!

  1. +++ b/core/modules/config_translation/config_translation.module
@@ -43,7 +43,6 @@ function config_translation_theme() {
-      'template' => 'config_translation_manage_form_element',

+++ b/core/modules/system/tests/modules/twig_theme_test/twig_theme_test.module
@@ -17,15 +17,12 @@ function twig_theme_test_theme($existing, $type, $theme, $path) {
-    'template' => 'twig_namespace_test',

    These ones need to stay for now because they use underscores. The config_translation one it would be nice to open a small follow-up issue to rename that template but let's keep this focussed. 

  2. +++ b/core/modules/system/theme.api.php
@@ -49,7 +49,7 @@
- *    'template' => 'search-result',
+ *

    Extra blank line here ;)

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
CreditAttribution: rpayanm commented
Status: Needs work » Needs review
FileSize
2246675-20.patch30 KB
2246675-interdiff-19.txt1.46 KB

@Cottser thank you for review my code ;)
Here the patch :)

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Needs review » Reviewed & tested by the community

That looks very shippable to me.

Before patch:

ack "\s+'template' => '[a-zA-Z-]+',?" -h -c
129

After patch:

ack "\s+'template' => '[a-zA-Z-]+',?" -h -c
0

The only remaining 'template' lines I could find after patching all had underscores or periods in them.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Issue summary: View changes

Updating RegEx in issue summary, we can't remove template names that contain underscores.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Title: Remove all unnecessary 'template's in hook_theme() declarations » Remove all unnecessary 'template' lines in hook_theme() declarations
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Issue summary: View changes
Issue tags: +theme system cleanup

Nice diff stats!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Yay. :)

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 4724ae1 on 8.0.x 
    Issue #2246675 by rpayanm, johnstorey, holly.ross.drupal | mgbellaire:...

Status: Fixed » Closed (fixed)

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