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

Files: 
CommentFileSizeAuthor
#20 2246675-interdiff-19.txt1.46 KBrpayanm
#20 2246675-20.patch30 KBrpayanm
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,072 pass(es). View
#17 2246675-17.patch30.64 KBrpayanm
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,928 pass(es), 80 fail(s), and 0 exception(s). View
#5 remove-template-from-hook-2246675-5.patch4.08 KBjohnstorey
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch remove-template-from-hook-2246675-5.patch. Unable to apply patch. See the log in the details link for more information. View
#3 remove-template-from-hook-2246675-3.patch5.46 KBholly.ross.drupal
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch remove-template-from-hook-2246675-3.patch. Unable to apply patch. See the log in the details link for more information. View

Comments

mgbellaire’s picture

Cottser’s picture

Issue summary: View changes

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

holly.ross.drupal’s picture

Assigned: mgbellaire » Unassigned
Status: Postponed » Needs review
FileSize
5.46 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch remove-template-from-hook-2246675-3.patch. Unable to apply patch. See the log in the details link for more information. View

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

FileSize
4.08 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch remove-template-from-hook-2246675-5.patch. Unable to apply patch. See the log in the details link for more information. View

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

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.

Cottser’s picture

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

Status: Needs review » Needs work

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

Cottser’s picture

Status: Needs work » Postponed

#8 still applies.

Cottser’s picture

Issue summary: View changes

Adding a potential regular expression to the issue summary.

Cottser’s picture

Issue summary: View changes

Better.

Cottser’s picture

Issue summary: View changes

Updating the regular expression slightly.

Cottser’s picture

Issue summary: View changes

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

Cottser’s picture

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

Status: Active » Needs review
FileSize
30.64 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,928 pass(es), 80 fail(s), and 0 exception(s). View

Trying...

Status: Needs review » Needs work

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

Cottser’s picture

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

Status: Needs work » Needs review
FileSize
30 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 79,072 pass(es). View
1.46 KB

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

Cottser’s picture

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.

Cottser’s picture

Issue summary: View changes

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

Cottser’s picture

Title: Remove all unnecessary 'template's in hook_theme() declarations » Remove all unnecessary 'template' lines in hook_theme() declarations
joelpittet’s picture

Issue summary: View changes
Issue tags: +theme system cleanup

Nice diff stats!

webchick’s picture

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.