Most people call things like page.tpl.php "template php files" (some call them "tipple fips," and those people need to be stabbed ;)).

Then there's this thing called "template.php" which is not a "template php file", but is "the template php file" and this is always totally confusing to new themers, especially since there's absolutely nothing in common between these two concepts.

Possible suggestions:
- overrides.php
- code.php
- customizations.php

Whatever. Just not template.php. :P

Thoughts?

Comments

catch’s picture

$THEME_NAME.module

webchick’s picture

Faaaaaaascinating! Hm!

That's essentially what it is, too. A lot of the same tricks you use in template.php (preprocess, *_alter, etc.) work in themes too.

catch’s picture

Category: feature » task

To expand on that a bit, it's quite possible to have a theme without template.php at all. However I have seen themes that have mountains of helper functions in template.php and loads of foo.preprocess.inc includes, much of which probably should have been in modules in the first place. This is especially true for custom themes where there's a division of labour between custom module developers and themers - and themers end up 'tidying up' and tweaking bits and pieces from custom modules in the theme - where committing to the module directly usually would have made more sense.

If we essentially ripped out template.php as a separate concept, allowed themes to include a .module file (and then we'd need to have subthemes with .module files work as dependencies I guess), that would make it more explicit what template.php/$THEME_NAME.module is really for. Also I can't think of anything off the top of my head that we allow people to do in the theme but don't allow in modules, I guess the ordering of preprocess execution is one difference but that would be easy to resolve.

Also we could then take the theme system out of drupal_alter() again http://api.drupal.org/api/drupal/includes--module.inc/function/drupal_al...

joachim’s picture

or $THEME_NAME.theme? A module extension inside a theme is going to look a bit weird...

Either way, having $THEME_NAME in the filename will make working with subthemes a lot easier! No more risk of uploading the wrong template.php to the wrong theme!

mattyoung’s picture

sime’s picture

Anything different would be great. I am working with a base theme stack that is 3 themes tall (base theme + corporate style theme + site-specific). I have three template.php files open in my text editor.

It feels most intuitive to use THEMENAME.theme. Mainly because it matches the directory it's in (sites/all/themes) just like .profile and .modules do. It's the holy trinity of Drupal. In some ways Drupal made this bed. We don't really need the naming pattern ".module" either - they could be called "module.php". That would be sad, we all love our .module extension so let's invite themes to the party.

eojthebrave’s picture

Agree, 100% that we need to change this to something ... anything other than template.php. I teach theming a lot and am often struggling with trying to explain the different between "template php files" and "template do php", or "template files" and "the template file".

So far my vote goes for $THEME_NAME.theme, I agree .module might be a little bit confusing inside of a theme directory.

eojthebrave’s picture

Status: Active » Needs review
StatusFileSize
new35.69 KB

Lets see if this works ...

droplet’s picture

It is a plain PHP file. don't force themer configure their editors before editing it. keeps it end with .php is easier for everybody.

droplet’s picture

Status: Needs review » Needs work

and GIT has rename commands :)

eojthebrave’s picture

Yes, .php would be easier and not require people to configure their editor. I'm going to argue for .theme though as it's consistent with how we name files everywhere else. .module, .engine, .test .. and so forth. All of which are "just PHP" files. Though, I'm happy with either $THEME_NAME.php or $THEME_NAME.theme ....

Will re-roll using `git rename`.

tstoeckler’s picture

I'm also for $THEME_NAME.theme. The file is the equivalent of a module file, it always gets loaded and it contains theme function implementations (vs. hook implementations). We then have a slight inconsistency in that themes without a .theme file are valid themes but modules without a .module file are not. That is, though, a bug on the module side, see #340723: Make modules and installation profiles only require .info.yml files.

eojthebrave’s picture

Hmm. Tried using `git mv ...` to rename the files and that didn't make any difference. Doing a `git status` on them prior to creating the patch in #8 was showing their status as 'renamed' already anyway. Am I missing something? Should I be doing something different when generating the .patch file? Right now I'm using `git format-patch 8.x` to generate the file.

kika’s picture

foo.theme.php convention should be in sync with bar.module.php, foobar.profile.php. It's a worthwile discussion to have but keep it separate. For now, my word goes to foo.theme

tstoeckler’s picture

git format-patch -CM should detect renames (and copies).

webchick’s picture

Title: Rename template.php to *anything* but template.php » Rename template.php to THEMENAME.theme to elimnate ambiguity around "the template file" vs. "template files"

Fixing title.

eojthebrave’s picture

Status: Needs work » Needs review
StatusFileSize
new3.86 KB

Try this again using `git format-patch -C -M 8.x`.

Status: Needs review » Needs work

The last submitted patch, 1033116-17-rename_template_php-eojthebrave.patch, failed testing.

eojthebrave’s picture

Status: Needs work » Needs review
StatusFileSize
new3.86 KB

Oops. I mean .theme, not .engine ... trying this again.

brianV’s picture

+1 - This is a really good move IMO - 'template file' and 'template files' has led to confusion in our team on several occasions, and we aren't the only ones.

Rolled to HEAD.

Status: Needs review » Needs work

The last submitted patch, 1033116-20-rename_template_php-brianV.patch, failed testing.

eojthebrave’s picture

Status: Needs work » Needs review
StatusFileSize
new26.06 KB

Looks like this was missing one of the template.php files. Also updated for the new twig engine.

brianV’s picture

Thanks for catching that. I saw the fail, intended to dig into it tomorrow ;)

Also needed to be updated for twig, which I see you've done.

c4rl’s picture

StatusFileSize
new25.32 KB

Re-rolled against 8.x HEAD

c4rl’s picture

StatusFileSize
new25.32 KB

Strange. The test status isn't updating, failure was due to Git issue. Attaching again.

Status: Needs review » Needs work

The last submitted patch, drupal-1033116-24.patch, failed testing.

c4rl’s picture

Status: Needs work » Needs review

#25: drupal-1033116-24.patch queued for re-testing.

c4rl’s picture

Tests pass. Bump? :)

tstoeckler’s picture

Some small nitpicks before this can go to RTBC:

+++ b/core/modules/php/php.module
@@ -105,7 +105,7 @@ function _php_filter_tips($filter, $format, $long = FALSE) {
-    $output .= '<li>' . t('Consider including your custom PHP code within a site-specific module or template.php file rather than embedding it directly into a post or block.') . '</li>';
+    $output .= '<li>' . t('Consider including your custom PHP code within a site-specific module or $THEME_NAME.theme file rather than embedding it directly into a post or block.') . '</li>';

I think that help text should rather be "a site-specific module or theme". That is more consistent, less confusing ("What the hell, is this $THEME_NAME thing?!") and also, I think the current text creates the expectation that I can just drop random code in a .theme named file and it will be loaded, which is not true, at least not necessarily.

Grepping for template.php I also found a comment in core/modules/system/lib/Drupal/system/Tests/Menu/BreadcrumbTest.php: "...due to template.php overrides." should be "...due to theme overrides."

Also can you roll the patch with git the -C -M options for git diff? That way, the patch file simply notes that the files were renamed without showing all the removed code and then the added code again. That would be awesome!

Edit: Fixed formatting fail.

tstoeckler’s picture

Status: Needs review » Needs work

Forgot to mark "needs work" for #29.

c4rl’s picture

Status: Needs work » Needs review
StatusFileSize
new4.31 KB

Good call on the -C and -M options (those were new to me) :)

Re-rolled with suggestions from #29.

Status: Needs review » Needs work

The last submitted patch, drupal-1033116-31.patch, failed testing.

eojthebrave’s picture

Status: Needs work » Needs review

#31: drupal-1033116-31.patch queued for re-testing.

Test pass locally for me so giving this another go just to see what happens.

c4rl’s picture

Tests pass :)

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

Awesome, can't find anything to complain. :-)

catch’s picture

Assigned: Unassigned » webchick

This makes some sense to me, except that old 4.x era themes like Chameleon used to ship with a .theme file iirc (and we still filter that out in .htaccess). i'm not sure if that's a problem at all but punting to webchick for a second opinion. Something about .theme feels weird to me but i don't have any concrete complaints against it either.

droplet’s picture

XYZ.php always better than themeName.theme a lot.

One example, assumed there're 30 new themers in your class,

Telling them to config an editor to support .theme with PHP highlighting and this is a PHP file

VS

Telling them that template.php (XYZ.php) is not a "template php file"

Which one take more time ?

tim.plunkett’s picture

@droplet, Actually, I just did this with a training class this week, and the answer is, changing the editor is easier :)

It's drupal, they already have to change that for other extensions. The confusion between *.tpl.php and template.php is very real.

c4rl’s picture

@droplet, In this case, for the sake of true consistency, we should open a separate issue that proposes converting:

* every theme template.php to themename.theme.php
* every modulename.module to modulename.module.php
* every filename.inc file to filename.inc.php
* every module.info to module.ini
* etc

And then get every contrib project to do the same. Right?

tstoeckler’s picture

That would be #7269: [meta] Add .php extension to PHP files :-)

Let's get this in, nonetheless.

tim.plunkett’s picture

#31: drupal-1033116-31.patch queued for re-testing.

tstoeckler’s picture

+++ b/core/modules/php/php.module
@@ -105,7 +105,7 @@ function _php_filter_tips($filter, $format, $long = FALSE) {
-    $output .= '<li>' . t('Consider including your custom PHP code within a site-specific module or < code >template.php< /code > file rather than embedding it directly into a post or block.') . '</li>';
+    $output .= '<li>' . t('Consider including your custom PHP code within a site-specific module or theme rather than embedding it directly into a node or block.') . '</li>';

I had missed this before, but this inadvertently renames "post" to "node". That should be reverted. Using "post" there was a deliberate decision at some point. If we want to revisit that, that should be a separate issue.
If this still comes back green, I'll upload a patch with that.

EDIT: < code > inception via Dreditor...

brianV’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new3.66 KB

Changed 'node' back to 'post' as per @tstoeckler's comment.

Setting to 'needs review' for the testbot.

Status: Needs review » Needs work

The last submitted patch, drupal-1033116-42.patch, failed testing.

tstoeckler’s picture

The patch misses the renaming of bartik's and seven's template.php. Otherwise looks good.

brianV’s picture

Status: Needs work » Needs review
StatusFileSize
new23.29 KB

Doh, of course. That somehow got dropped from the patches after #25...

Renames included.

Status: Needs review » Needs work

The last submitted patch, drupal-1033116-46.patch, failed testing.

tim.plunkett’s picture

Status: Needs work » Reviewed & tested by the community
StatusFileSize
new4.31 KB

See #29 for how to roll this with renames.

ParisLiakos’s picture

XFCE thinks .theme is a panel launcher-.- i not only have to tell my editor that this is a php file, but my my OS too:P
since we are doing the rename, why dont we rename it to .theme.php so that we have less job to do in #7269: [meta] Add .php extension to PHP files?

jibran’s picture

It is same for windows as well. So +1 to 49.

webchick’s picture

Assigned: webchick » Unassigned
Category: task » feature
Status: Reviewed & tested by the community » Needs work

Hm. That sounds like a needs work. That is unfortunate, because the .theme vs. .module balance is really nice at a glance, theme.php / .module much less so. I'm not sure #7269: [meta] Add .php extension to PHP files is really going anywhere.

Re-classifying this as a feature, since it's more of a "nice to have" rather than something that blocks other work. I'm happy to commit this patch once agreement has been reached and we're below thresholds. Unassigning.

brianV’s picture

I don't really agree that the above warrant a 'needs work'.

1. In the case of XFCE, right click on the file and use the 'Open With' contextual menu. If the XFCE file browser doesn't support that, it's not really full featured. Ditto for Windows.

2. As for having to configure your IDE to support .theme files as PHP... well, you already do that for .module, .engine etc. Also, this is a once-per-project setting, or, with most IDEs I've used, something you can set globally if most of your work is Drupal.

ParisLiakos’s picture

well, it is i file that holds php code..its extension should be .php
thats the reason extensions exist, so that other programs know what to do with a file without me having to configure it..if we start having different extension for every freaking file,why do we even use them?

i might be able to do this once in my pc/enviroment, but i would hate to do with every vim/emacs/nano/<you name it> instance in each every different server i will have to work through ssh..thats where it starts getting annoying

webchick’s picture

Another approach could be to rename it to .theme for now, and then try to get #7269: [meta] Add .php extension to PHP files done, now accommodating for this extension as well. That'd at least keep the nice mirroring of filenames, and then if that issue doesn't happen (which it might not; it doesn't really seem to have a wide swath of buy-in around it atm), we need to suck up the consequences.

I seem to recall changing system-level assumptions about file extensions being a pretty big pain in the ass to do on Windows, but it's been a long time.

webchick’s picture

Status: Needs work » Needs review

Moving back to needs review. It'd be nice to hear from a Windows user or two on this topic.

tstoeckler’s picture

Windows doesn't recognize any files (that do not have the .php extension) as text/PHP files, that includes .module, .inc, etc. So the first time you open such a file on your system you have to pick the program to run it with and you will choose your editor of choice. I don't think it is much worse that you have to do the same for a .theme file, even if the "Choose the program you want to run this with" screen doesn't appear on its own, because Windows thinks it is a Windows theme. (Note that I'm not *currently* on Windows so I can't tell exactly what happens.) We're talking about developers, who should know or at least expect that there is PHP in that file. And since this is an operation that you do once (as in once in forever) I don't think we should worry about this.

cweagans’s picture

Category: feature » task

IMO, renaming crap is not a feature.

Also, once this lands, I'm going to tackle 7269. We won't have to deal with a non-php extension for very long, and since that's really the only objection here, I propose this goes back to RTBC. The patch in #48 still applies.

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

Fine by me. lets commit this and then go for #7269: [meta] Add .php extension to PHP files

sun’s picture

Status: Reviewed & tested by the community » Needs review

@catch's note in #36 is not historical - unless the code has been removed in the past days, then it still exists in HEAD.

I'd like to double-check that before this gets in.

sun’s picture

So yes. I was 100% sure that I stumbled over that Drupal 5.x code during my work on #1886448: Rewrite the theme registry into a proper service

This is what I can share with you:

  1. $theme->owner points to a theme's or theme engine's main PHP file (if any), as documented on _drupal_theme_initialize().
  2. _drupal_theme_initialize() is also where the magic include of the .theme PHP file happens.
  3. The $owner property is discovered and initialized in a completely different spot, _system_rebuild_theme_data().
  4. For the very special engine of theme (instead of phptemplate) declared in a theme's .info file, $owner is turned into themename.theme, if a .theme file exists.
  5. However, since Drupal 7, the engine defaults to phptemplate if omitted, which happens right within that _drupal_theme_initialize() function.

Consequently, we should remove that special handling for 'engine' == 'theme' in _system_rebuild_theme_data().

AFAICS, the previously intended logic will still work as-is, but I'm really not sure whether we still want to support that case any longer. The last time I've seen a pure PHP .theme-based theme was in Drupal 5. And apparently, when http://drupal.org/project/chameleon was moved into contrib, it got upgraded/migrated to the phptemplate engine.

eojthebrave’s picture

@sun, do you think we should remove the historical .theme processing in this issue then or in a separate one? If I'm reading the code properly there is nothing that would cause the above patch to not work, it's just confusion and historical cruft that should get cleaned up. Happy to do a quick re-roll and move the extraneous code from _system_rebuild_theme_data() if that should happen in this issue.

sun’s picture

I'm not 100% sure about the consequences of not removing it (and we obviously do not have any test coverage for that engine=theme case ;)), so I'd prefer to remove that logic with this patch. Otherwise, we can also create a major follow-up task to make sure to remove the support entirely (I don't know whether I catched all details/remnants).

sun’s picture

Issue tags: +Extension system
eojthebrave’s picture

StatusFileSize
new5.47 KB

Re-roll with old .theme file handling removed as per comment 60.

sun’s picture

Status: Needs review » Reviewed & tested by the community

Thanks!

@webchick was/is right with this in #54:

I seem to recall changing system-level assumptions about file extensions being a pretty big pain in the ass to do on Windows, but it's been a long time.

That is still the case - you need to register all of Drupal's custom extension names manually as "PHP" files, and of course, you also need to reconfigure your code editor (or IDE) to interpret those files as PHP.

In general, that's a pain to do and figure out, especially for newcomers, who have to bend their brain around gazillion of other things already, so additionally having to reconfigure all kind of stuff to make a weird Drupalism work is really not helpful.

Therefore, I'd really prefer to move forward with #7269: [meta] Add .php extension to PHP files "instead" — although this patch here requires and contains some additional adjustments that are specific to the theme system.

I'll leave it to @catch and @webchick to decide on how to move forward.

tim.plunkett’s picture

Issue tags: -Extension system

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1033116-64-rename_template_php.patch, failed testing.

ParisLiakos’s picture

Status: Needs work » Needs review
Issue tags: +Extension system

translation entity ui random failure?
#64: 1033116-64-rename_template_php.patch queued for re-testing.

ParisLiakos’s picture

Status: Needs review » Reviewed & tested by the community

yeap

xjm’s picture

Issue tags: +Quick fix
xjm’s picture

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Woot! template.php is dead :)

Committed b4cc0cc and pushed to 8.x. Thanks!

alexpott’s picture

Title: Rename template.php to THEMENAME.theme to elimnate ambiguity around "the template file" vs. "template files" » Change notice: Rename template.php to THEMENAME.theme to elimnate ambiguity around "the template file" vs. "template files"
Priority: Normal » Critical
Status: Fixed » Active

This needs a change notice

jibran’s picture

Status: Active » Needs review
Issue tags: +Needs change record
ParisLiakos’s picture

Title: Change notice: Rename template.php to THEMENAME.theme to elimnate ambiguity around "the template file" vs. "template files" » Rename template.php to THEMENAME.theme to elimnate ambiguity around "the template file" vs. "template files"
Priority: Critical » Normal
Status: Needs review » Fixed
Issue tags: -Needs change record

I cant think of anything to add

xjm’s picture

Hmm, did we ever get a response to @sun in #65? Maybe we continue that discussion in... holy low nid batman. #7269: [meta] Add .php extension to PHP files

star-szr’s picture

Title: Rename template.php to THEMENAME.theme to elimnate ambiguity around "the template file" vs. "template files" » Rename template.php to THEMENAME.theme to eliminate ambiguity around "the template file" vs. "template files"

Spelling in the title.

Status: Fixed » Closed (fixed)
Issue tags: -Quick fix, -Extension system

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