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
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
Members fund testing for the Drupal project. Drupal Association Learn more

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.