Posted by webchick on January 19, 2011 at 9:53pm
27 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | theme system |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | Extension system, Quick fix |
Issue Summary
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
#1
$THEME_NAME.module
#2
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.
#3
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...
#4
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!
#5
Marked #1044242: Change template.php to THEMENAME.theme as dup of this.
#6
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.
#7
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.
#8
Lets see if this works ...
#9
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.
#10
and GIT has rename commands :)
#11
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`.
#12
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 only require .info.yml files.
#13
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.
#14
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
#15
git format-patch -CMshould detect renames (and copies).#16
Fixing title.
#17
Try this again using `git format-patch -C -M 8.x`.
#18
The last submitted patch, 1033116-17-rename_template_php-eojthebrave.patch, failed testing.
#19
Oops. I mean .theme, not .engine ... trying this again.
#20
+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.
#21
The last submitted patch, 1033116-20-rename_template_php-brianV.patch, failed testing.
#22
Looks like this was missing one of the template.php files. Also updated for the new twig engine.
#23
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.
#24
Re-rolled against 8.x HEAD
#25
Strange. The test status isn't updating, failure was due to Git issue. Attaching again.
#26
The last submitted patch, drupal-1033116-24.patch, failed testing.
#27
#25: drupal-1033116-24.patch queued for re-testing.
#28
Tests pass. Bump? :)
#29
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 -Moptions forgit 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.
#30
Forgot to mark "needs work" for #29.
#31
Good call on the -C and -M options (those were new to me) :)
Re-rolled with suggestions from #29.
#32
The last submitted patch, drupal-1033116-31.patch, failed testing.
#33
#31: drupal-1033116-31.patch queued for re-testing.
Test pass locally for me so giving this another go just to see what happens.
#34
Tests pass :)
#35
Awesome, can't find anything to complain. :-)
#36
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.
#37
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 ?
#38
@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.
#39
@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?
#40
That would be #7269: Add .php extension to PHP files :-)
Let's get this in, nonetheless.
#41
#31: drupal-1033116-31.patch queued for re-testing.
#42
+++ 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...
#43
Changed 'node' back to 'post' as per @tstoeckler's comment.
Setting to 'needs review' for the testbot.
#44
The last submitted patch, drupal-1033116-42.patch, failed testing.
#45
The patch misses the renaming of bartik's and seven's template.php. Otherwise looks good.
#46
Doh, of course. That somehow got dropped from the patches after #25...
Renames included.
#47
The last submitted patch, drupal-1033116-46.patch, failed testing.
#48
See #29 for how to roll this with renames.
#49
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: Add .php extension to PHP files?
#50
It is same for windows as well. So +1 to 49.
#51
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: 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.
#52
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.
#53
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#54
Another approach could be to rename it to .theme for now, and then try to get #7269: 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.
#55
Moving back to needs review. It'd be nice to hear from a Windows user or two on this topic.
#56
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.
#57
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.
#58
Fine by me. lets commit this and then go for #7269: Add .php extension to PHP files
#59
@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.
#60
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:
$theme->ownerpoints to a theme's or theme engine's main PHP file (if any), as documented on _drupal_theme_initialize()._drupal_theme_initialize()is also where the magic include of the .theme PHP file happens.engineoftheme(instead ofphptemplate) declared in a theme's .info file, $owner is turned intothemename.theme, if a .theme file exists.enginedefaults tophptemplateif 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.
#61
@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.
#62
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).
#63
#64
Re-roll with old .theme file handling removed as per comment 60.
#65
Thanks!
@webchick was/is right with this in #54:
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: 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.
#66
#64: 1033116-64-rename_template_php.patch queued for re-testing.
#67
The last submitted patch, 1033116-64-rename_template_php.patch, failed testing.
#68
translation entity ui random failure?
#64: 1033116-64-rename_template_php.patch queued for re-testing.
#69
yeap
#70
#71
#64: 1033116-64-rename_template_php.patch queued for re-testing.
#72
Woot! template.php is dead :)
Committed b4cc0cc and pushed to 8.x. Thanks!
#73
This needs a change notice
#74
Created http://drupal.org/node/1967614
#75
I cant think of anything to add
#76
Hmm, did we ever get a response to @sun in #65? Maybe we continue that discussion in... holy low nid batman. #7269: Add .php extension to PHP files
#77
Spelling in the title.
#78
Automatically closed -- issue fixed for 2 weeks with no activity.