New template system

This is good stuff imho, at least in terms of the admin interface.
- This adds to the work that's been done recently on simplifying the admin menu and configuration pages.
- Switching between different styles of the same theme is easier and therefore quicker.
- The dupilcate logo image/slogan/mission settings we see with some themes would be reduced to a single instance.

I can see some people might complain that some theme specific settings are not visible, but they ought to be available under settings imo, it's simpler if all such things are together.

And you can still go in and tweak themes to your content, so no loss of power there.

Small patch attached fixes up chameleon so it finds common.css, without this it 404s and looks a little ugly. :)

Not going to commit this until it is complete (the settings are no more). Other than that, my main concern is that the table on the theme selection page is confusing. Unless you understand the underlying architecture of the proposed theme system (templates versus styles), you can't figure out what you are looking at or what you are doing when clicking the checkboxes. Otherwise, switching templates/styles works nicely.

I might be speaking too much for the other Adrian here but...

I think the point about settings is valid, theme specific settings need to exist, and need to go somewhere (presumably under the settings menu, in the way that module settings now do?) but I'm not sure where.

As for the the theme selection table, I think half the point is that you don't need to know the underlying structure of the theme system - you just select a particular style and you forget about all the rest. That said I agree that the table is a little confusing, too many 'Default' possibilities, "select the default theme, what? There's three different defaults". I'd suggest further simplification, instead of having each theme with a style called Default, the default style should be named after the theme itself. Thus this table would go from:

bluemarine
default
chameleon
default
marvin
pushbutton
default

to a simple list of styles:

bluemarine
chameleon
marvin
pushbutton

There is no real need to be aware which theme 'engine' underlies the style you're using (unless you want to modify it, but it's easy to look in the templates directory and figure out which is which).

I second this. From an administrator's perspective, you're not choosing a theme and a style, you're choosing a theme. From a designer/developer's perspective, you have many ways to create a theme: make a new stylesheet for an existing one, make a new template, install a whole new template engine. But the results are always a new theme.

The decision flow isn't "what theme?" followed by "what style?", it's just "what theme?"

Adrian: Thank you for the effort you have put into this patch. This patch and your install system are both changes I'm very hopeful will land in time for 4.5. I like the direction you're taking the theme system.

Here are a few comments:
* User theme selection (from "my account") doesn't work after the patch.
* I agree with adrinux that the "default" style for each template should be invisible, hiding behind the name of the template itself.
* With the exception of the word "default," I like the admin theme-selection as it is: I believe the admin should be aware of both which template and which style they're using, and you've done a good job of expressing that.

- get rid of the "type" column. users do not care.

- If you still need to present type in UI, I'd suggest to drop the confusing "theme-template-style" trinity and stick with 2 simple terms:

"theme": so-called skeleton
"variation": so-called skin

make it so that variations are grouped with themes (using a special table css perhaps,

This is locical for user, already-known (themes have been in Drupal for ages, now we even can have variations of these), avoids misinterpretation (the terms "style" and "template" are used widespread) and also makes a perfect English: "variations of the theme".

- get rid of the long and confusing help text. Just say it in 2 sentences:
"Themes set the look and feel of your site. If the theme is enabled, your users will be able to select it from their preferences and additionally /.../"

Some questions:

- how the theme thumbnail creation is proceed?
- will the themes/templates/styles have descriptions?

My first thought was that the selection table should be kept very simple to make changing themes quick and easy. But on further reflection it seems even the most fickle site admin probably doesn't change theme all that often, so having a descriptive entry for each theme seems like a better idea. This is one screen that can probably afford a little 'clutter' - better to say it can afford to be verbose - it shouldn't look cluttered :)

Commenting on the screenshot:
I'd like better differentiation of the name, perhaps mark it up strong?
The type column is confusing to me, surely even the standard look for each template is still a style? In that sense Kika is right to question the terminology, but I'm not sure about variation. And I definitely don't think describing a template as theme works. As Adrian put it to me earlier:
theme = template+style

My suggestion would be to lose the type column but introduce a 'description' field, theme Authors could then do this sort of thing:
bluemarine
"The classic drupal.org theme, based on the xtemplate template engine."
chameleon
"A clean and simple look originally intended for a blog style site, this is the standard style of chameleon template engine."
marvin
"Another clean and simple layout, now based on the chameleon template engine."
pushbutton
"Based on the xtemplate template engine this theme focuses on usability and readability."

Or perhaps use a vertical list of information, with screenshot to the left, enable and default buttons to the right of the name:
name: bluemarine
author: A.N. Other
template engine: xtemplate
description: The classic drupal.org theme.

I know you could go on adding meta information for each theme all day, but I do like the idea of author name being shown.

theme descriptions: I don't believe we need them. Any theme descriptions I've written up to this point have been completely useless and unhelpful. Here are some examples: marvin is "A PHP theme", unconed is described as "UnConeD theme by Steven Wittens", chameleon is "A fast PHP theme with different stylesheets." In practice, it is difficult to appropriately describe a theme/template/style in a short passage of text. Also, when an admin is selecting a theme, they want to know what it looks like, not what someone wrote about it. For that reason, the screenshot basically eliminates the need for a description. On the other hand, I have no problem with displaying the theme author.

template selection screenshot w/style thumbnails: I like the screenshots! However, I miss the ability to see, at a glance, which templates are responsible for each style. I feel this information is important to admins so they can immediately find the appropriate template or style file if they wish to make modifications. I also tend to dislike the in-your-face differentiation between styles and themes. Perhaps the "type" column could be replaced with a "template" column... and name could be renamed to "style". For each template's default style, the "style" column could simply be blank.

mini-screenshots of each theme: Could we implement these in the individual users' selection of themes? I really like the screenshots, as themes are best selected with pictures rather than words (IMO).

I threw together a quick image showing my ideas for both the administrative interface and the "theme" section of "my account"... it's attached.

Template descriptions are useful, for instance:

"The Pushbutton template complies with the XHTML 1.0 (Transitional) standard,
accessibility standards Section 508 and WCAG Priority 1, and is designed to loose it's style elements gracefully in older browsers."

We could also give info on whether the template uses tables, or is pure CSS, and what mobile devices it's compatible with.

adrian: your last screenshot is close to perfect in that there are no confusing/unclear elements in your table! Everything is self-explanatory now - at least to me. :)

That page looks like the generic theme settings page because the text 'rubric/default' (while there in the help text) is close to invisible. I think this is best communicated through the page title and breadcrumb trail. Otherwise, it looks OK to me.

That patch appeared to apply cleanly to a fresh cvs checkout, but I get an error when visiting the site, even after wiping the db and creating a fresh one - visiting to create the first account gets me:

warning: init_theme(): Failed opening '' for inclusion (include_path='.:/usr/local/php/lib/php') in /Library/WebServer/Documents/drupal-template/includes/theme.inc on line 45.

warning: call_user_func(engine__block): First argument is expected to be a valid callback in /Library/WebServer/Documents/drupal-template/includes/template.inc on line 205.

warning: call_user_func(engine__page): First argument is expected to be a valid callback in /Library/WebServer/Documents/drupal-template/includes/template.inc on line 140.

Any idea's?

*Never mind*
Checked out cvs again, fresh db again, set up the admin account and *then* patched.
Working nicely. I guess the database files will need updating if this gets into core...

Looking good Adrian :)

I just spent the last THREE YEARS figuring out how to "do" themes correctly. I couldn't ever get the CMS engine the way I wanted it so I shelved it. Here are my recomendations and thoughts on the matter:

I wanted to approach "themes" from an entirely UNoriginal perspective: I went back to my high-school days and dug out all there is to know about Newpaper and Magazine Design. I even talked to Tim Harrower, author of Newspaper Designer's Handbook. Tim is largely considered THE guru on Newspaper layout and design. He recently redesigned the Wall Street Journal amoung other. You can find out more about it on his website.

After several conversations I came up with the following IMMUTABLE laws for internet publishing that almost EVERYONE ignores (except maybe high-end Zope, Bricolage, Vignette, etc. sites).

Law #1: There MUST be a seperate workflow for design/layout and content creation/publishing. The folks that do newspaper and magazine design/layout belong to the "production" department. The folks that actually generate the photos and articles belong to the "editorial" department. The product of the editorial department is handed off to the production department for design and layout.

HOW IT APPLIES TO DRUPAL: There should be a strong set of classes that seperate layout (template engine with one CSS), colour scheme (with a different CSS), navigation (global and contextual), and content (presumably XML).

Here's how I did it in my CMS:

First: The layout. I developed, per Tim's teachings, a site wide layout. I then develop module specific blocks (either wide or narrow) with their layout.

Implementation:
Layer 1: Generic SITE WIDE layout. This template, using a template engine, specifies three areas: top, middle, and bottom. The contents of each blob are pulled from the CMS engine.

Layer 2: Specifies the content (either static, generically dynamic, or module-based dynamic) of the top section and bottom section. Another template, is then nested in the middle section and sent off to the template engine. The middle section template specifies the columnar layout: two-col left, two-col right, three-col w/large center, one-col, etc.

Layer 3: The CMS, returns the content to the template engine for each column based on the configuration stored in the DBMS.

Layer 4: Each module returns its content to the CMS using context data passed to it.

On the surface, since I haven't really dug into the code yet, it looks like Drupal is doing this, only merging layer one and two and merging layer 3 and 4. I don't have to tell you there is a significant memory/processor cost to doing things this way. That is why I favour being able to cache periodic and static data in generated pages that are pulled directly from a file based cache in the same way Bricolage and Vignette do.

Second: The colour scheme and textual styles.

Per Tim's handbook and a couple other Magazine books: Drupal.org (the community) would be wise to develop a design taxonomy of tags. These tags will be used consitantly throughout the site. It would then be a no brainer to create an admin module where you could choose your predefined "look and feel", or have a tabular form where you can edit the colour, typeface, point size, border, background image, etc. for each text "style".

Implementation:

Layer 1: The site wide skin is applied as a CSS file.

Layer 4: Each module can have it's own unique styles with a configurable CSS file.

NET EFFECT:

You can now have a site that is consistant, where the layout is independent of the look and feel. You now have a website that can be "reconfigured" at the whim of the production manager/art director. You now have a website where content authors and the editorial staff don't have to care about the presentation and can focus on quality content.

I'd like to hear if anyone is interested in this. I would be willing to contribute the layout/look-and-feel classes I already have. I used the model-view-controller and factory design patterns for the classes. I never implemented the admin side because I got bogged down in the CMS engine.

Devin.

Also, can you expand on the 'develop a design taxonomy of tags' thing in a theme forum post please Devin? What exactly does that mean?

Adrian,
Thanks for helping me out. I'll place this in the Themes Forum. You are correct in your assessment of how much I have delved into Drupal's code base. I haven't. I will when I get time, I just haven't yet. I didn't mean to suggest that there be some kind of either/or situation with classes vs. procedural calls/includes. I don't have a bias one way or the other as both have their strengths and weaknesses.

I'll through this post over in the Themes Forum and see if I can elaborate the "taxonomy tags" a little better. I could probably post a sample CSS that would illustrate it better.

Devin.

- Theme Chameleon and theme Marvin display 'Array' instead of the primary links.

- Enabling the pushbutton theme results in the Xtemplate engine throwing parse errors.

- It is easy enough to switch themes so one might argue how useful these screenshots are. They are mostly eye candy. Either way, I'd try to make better screenshots: show a fraction of a page so we get to see some more detail. For consistency, maybe we should use the default welcome page?

- The 'Display node information on:' form group: drop the ':', use form_group()'s $description parameter to provide some help (it is unclear) and display the node names rather than the module names.

- The 'Toggle display' form group's help text needs to be clarified. Also, write 'posts' instead of 'nodes'.

- Avatars don't seem to work?

- The description of the 'Path to custom logo:' setting is incomplete.

- Using Bluemarine, there automagically appear some primary links, even though I haven't configured any. This is inconsistent with the Chameleon and Marvin. This might also be slightly confusing.

- The titles of the theme configuration pages read 'bluemarine/default' (for example) rather than 'bluemarine'. This is inconsistent with the titles shown on the overview/selection page.

Added some discussion here.

Cheers,

Devin.

I tried out the latest version of the patch, and I like the direction it's headed. It appears that user theme selection is still inoperable, however. Please spend a moment looking into this. The init_theme() function in includes/theme.inc no longer reads the value of $user->theme... instead, it looks at the value of a global $custom_theme variable, which, as far as I can tell, is set nowhere else.

Also, the "engines" directory seems pretty ambiguous to me... I'd prefer that it be a subdirectory of "themes" or perhaps that it be named "theme_engines".

- The explanation in engines/drupal/drupal.engine is something but it doesn't make me go "Aha, this is how it works!". The name 'drupal engine' is very confusing, but that is a minor nitpick for now.

- Part of the patch no longer applies due to the filter module changes.

- Some of the form description could be better, but we can worry about that later.

I'd like other people to test and review this code. My opinion is that the GUI and configuration is _much_ improved (good job) but that, unfortunately, the underlying code and - Drupal theming in general - will become _much_ harder to grasp.

If I had more time, I'd try to take Adrian's GUI/configuration changes and port them to the current theme system.

I worked on the patch a bit and tried to draw up a compromise between completely changing the way the theme system works and implementing templates. My patch is attached. You can test the patch out live (for a limited time only) on my test site. You should have full access to the theme admin interface as a registered user.

My patch looks nearly identical on the surface, but the way in which things are implemented is considerably different. Any and all feedback is appreciated. I still have a bit of hope that this might make it into core before the feature freeze... but time isn't exactly on my side. If nothing else, some code is ready for early in the 4.6 development cycle.

To test the patch on your local site, I recommend that you rename (move aside) your themes directory and put the new one (from the tar) in its place.

A couple benefits of my patch:
* old themes continue to function, with no changes (although they don't take advantage of the unified interface without changes)
* template themes can also override theme functions if necessary through their .theme file... alternately, the theme file can simply be a small placeholder telling Drupal to start the xtemplate engine (see bluemarine.theme or pushbutton.theme)
* more Drupal-esque design (i.e. drupal_get_theme_setting function rather than huge $theme global variable)

Theme system directory layout:

drupal
|\-themes
| | \-engines
| |    \-xtemplate (contains xtemplate.engine and xtemplate.inc)
| |\-pushbutton (contains pushbutton.xtmpl, pushbutton.theme, pushbutton.css, images)
| |\-bluemarine (contains bluemarine.xtmpl, bluemarine.theme, pushbutton.css, images)
| |\-chameleon (contains chameleon.theme, chameleon.css, images)
| |   \-marvin (contains marvin.css, images)
|  \-example (contains example.theme)
|\-includes (contains theme.inc, etc.)
 \-modules (contains system.module, etc.)

I hoping Dries likes your code better than he liked Adrian's, or that this work on the theme system is at least working towards a compromise :) 'cos I'd really like to see this make it into 4.5.

Commenting your patch:
Nice job on carrying over the UI changes, clearly only minor differences from Adrian's excellent job.
Styles have no description - this is not so good, half the point of the UI changes was to make the difference between a theme and a style transparent to the user, reading down the list of options in /admin/themes it now looks like some are just missing descriptions.

Moving engines into themes - that seems tidier.

Style and stylesheet names - this alternate approach seems logical, and you seem also to have catered for the functionality Adrian included in phptemple, with seperate layout, module etc stylesheets with the ability to ignore them.

Being able to override theme functions - Adrian had mentioned adding this functionality too, and it's definitely a good thing. The last couple of days I've been working on re-building the phptemplate default template from scratch, and finding I need this functionality if I want to truly customize a design.

As far as the code goes I'm probably not in a position to comment, I know Adrian has been thinking not simply of an improved theme system, but also how to enable install functionality. This patch looks ok in itself, but only Adrian can say whether it will fit in with his install system design...

Attached is a newer version of the patch. This version also includes an upgraded theme selector for site users (basically a stripped-down version of the admin interface... screenshots included) and an upgrade path for those currently using xtemplate or chameleon themes. (no change is necessary for other themes)

Once again, move your existing themes directory elsewhere and replace it with the themes directory from the tar file. You can then copy back your contrib themes. This should be done in this way to guarantee that you do not have two copies of xtemplate or chameleon competing with one another and giving you "function is already defined" errors.

A more technical description of the changes is available over at my test site.

adrinux: Thanks for your time reviewing the patch. As for the inconsistency with descriptions, you'll notice that I intentionally left descriptions off of both the chameleon and bluemarine themes. On a fresh drupal 4.5 install, no descriptions will appear other than for example.theme. Making descriptions for styles seems like it would unnecessarily complicate things... right now, styles consist of .css files... and nothing else. Sure, I could make the description a comment at the top of the CSS file and make PHP parse it, but that seems like a lot of effort for very little return. Perhaps we could instead look at making the description text less obtrusive so it won't appear inconsistent? Then again, I anticipate that most themes won't really need a description now anyway with the screenshot functionality.

I share your hope that this might end up in 4.5... but if not there's always 4.6, I guess.

User selection is another good feature to add, nice work.

re: descriptions for styles.
I think there is just some confustion between us about what a style is. I was refering to the whole template/stylesheet combination as a style, and thinking of the theme engine as the theme. But it probably makes more sense to do as you've done and think in terms of:
theme engine
themes (can be different templates using same engine)
styles (different css stylesheets)

In that sense you have it right, a stylesheet probably doesn't need a description right now.
Though I can see that it might be useful - suppose you have a large text stylesheet variation for visually challenged users, I'm not so sure it would be all that obvious from the screenshot what the difference is, a description would be nice. But we can leave that to the future I think, keep things simple for now.

I've very interested to see what Adrian and Dries have to say about this patch. Looks good to me though, except I had minor problems applying both your patches btw - might just of been me but... - are you reversing new and old when you diff?

I've attached yet another newer version of the patch. The only changes are a couple of minor typos I fixed. This is my last update... I promise. ;)

adrinux: I double-checked and verified that this patch successfully applies to a clean CVS checkout. Here are the commands I used to apply the patch:

cd drupal
mv themes themesold
tar -xjf ~/templates-20040815.tar.bz2
patch -p0 < templates-20040815.patch

Here are my initial thoughts without looking at the actual code:

• When I set the default theme to a theme which is not enabled, I got a PHP error. (Note: I was using the previous tarball, not the one you uploaded less than an hour ago.)
• Some theme descriptions are missing and those that are available are not consistent: just display the paths so people can figure out where to modify a particular theme.
• On the 'my account - edit' page, the theme selection is awkward: in particular, the 'Default theme' part. Remove that and simply select the current theme.
• I think that the per-theme overwriting of global theme settings is not well communicated by the GUI. The same is obviously true with Adrian's patch but I figured I'd point it out nonetheless. I don't know how to solve this (yet).
• 'Site mission' -> 'Mission statement'?
• Try not to use the word 'node': use 'post' instead.

I'll look at the code next.

I like the simplicity of TDobes code compared to Adrian's. A couple remarks:

• system_theme_listing() is a complex function, it seems. I wonder if there is room for simplification here.
• We used generic names for the styesheets and templates because it make it easy for people to create new themes. They can copy an existing theme and start modifying it. With your patch, we're back to the old model where files (and functions?) need to be renamed.
• What is the $custom_theme stuff for?

> When I set the default theme to a theme which is not enabled, I got a PHP error.
Oops... I have code in place to force-enable the default theme, but it makes the change too late... I'll fix this.

> Some theme descriptions are missing and those that are available are not consistent:
> just display the paths so people can figure out where to modify a particular theme.
So you suggest that I remove descriptions entirely? Okay.

> On the 'my account - edit' page, the theme selection is awkward: in particular, the 'Default theme' part.
> Remove that and simply select the current theme.
Done.

> I think that the per-theme overwriting of global theme settings is not well communicated by the GUI.
> The same is obviously true with Adrian's patch but I figured I'd point it out nonetheless.
> I don't know how to solve this (yet).
I made no changes to the GUI other than the addition of the user theme selector. I will attempt to write some coherent context help for that page explaining the process.

> 'Site mission' -> 'Mission statement'?
Okay.

> Try not to use the word 'node': use 'post' instead.
Very well.

> system_theme_listing() is a complex function, it seems.
Agreed. I'll try to split it up a bit, perhaps simplifying system_module_listing() in the process.

> We used generic names for the styesheets and templates because it make it easy for people to create new themes.
> They can copy an existing theme and start modifying it.
> With your patch, we're back to the old model where files (and functions?) need to be renamed.
I'm really not sure if there's a way around this... the filename is used to identify the file to the theme system. Users creating a separate stylesheet must simply copy the existing stylesheet to a new name. Users creating a new template must copy the present template's files to their new template's name, in addition to changing, for example, bluemarine_engine to mynewtheme_engine. I'm open to any suggestions on how this might be simplified.

> What is the $custom_theme stuff for?
The $custom_theme global variable is read by init_theme(). If it is set to a string identifying an enabled theme, init_theme will use this theme instead of the presently selected user theme or default theme. This allows for the creation of a module which can, in its _init hook, modify the current site theme. (I'm thinking of a contrib module which could allow admins to set different themes for different sections of their site, based on path matching like block.module.)

I'll whip up another patch in an hour or so addressing the issues you mentioned.

Attached is yet another new version of the patch.

This one addresses all of Dries' concerns in the methods I explained in my last post.

I have split up system_theme_listing into several smaller functions, one of which I was able to share between system_theme_listing and system_module_listing. All of the minor changes were made as well.

It's worth noting that the php error caused by selecting an non-enabled default theme also existed before my patch. We might want to fix that in the 4.4 branch as well.

The only remaining issue is that the patch requires template writers to rename files when creating a new theme and update a couple of strings in a .theme file. Honestly, I don't feel like that's too complex... as long as we provide clear instructions in the handbook (which I'll look into updating upon commital of the patch).

Latest patch applied cleanly to fresh cvs checkout but my update php is broken - it may have been broken before...
warning: Invalid argument supplied for foreach() in [fpath to]/update.php on line 84.

Not a very helpful error message.

make that the patch before the latest...

Sorry about that, adrinux. It appears my updates.inc script had a bug if you were using a theme other than xtemplate or chameleon. The attached updated patch takes care of the problem. (and has all the other fixes too)

As far as I can see the TDobes patch/system is neat and small. However, I feel it takes away the whole purpose of the system by adrian.

AS I see it, TDobes system uses parts of Adrians system to fix some bugs/features/issues of the current theme system. Adrians System has one other huge advatage. It does more than only some UI improvemtents and some centralisation of configuration (the abovementioned bugs/features/issues). It creates a way and method to make easy designs and easy installation procedures. IT does this on a fairly low level, so that finally all the silly differences between smarty, phptemplate and xtemplate are really hidden away. Not only for the administrator. Tdobes system only does that: It removes the differences for administrators and users only. But it does not remove the differences for themedevelopers.

I feel that group has been greatly negleted. Partly due to different engines and non-standard theme systems. If drupal would stick to one system, that allows for differnent techniques of theme-development, themedevelopers can jump on the drupal train and help us formward.
If we continue with diffentent, non-co-operating systems the avarage theme developers will become confused: after all: they are not developers in the sense of programmers!

The proof for this all, is that we have hardly seen any big themes contributed in the last year. Another proof is the overwhelming amount of slightly tweaked bleu-ish-xtemplate-using-sites out there. If It is as simple as opening a texteditor, adding and removing some CSS the curve of learning themeing for drupal becomes incredibly smaller. Look at the amount of contributed themes for phptemplate. You will see that there are nearly as much of those as there are "normal" themes. The reason for this, is that it is simple to create themes, styles and tweaks.
Adrian took this idea a step further by making this concept into a theme system.

So to summarise all, and to come to a conclusion I would like to say:
PLease, please consider adding this system in core after the branching. I bet that if we use, rather then only see the improvements and possiblities, we all will be happy and start themeing right away :)

Ber

Ber: TDobes extracted the part that is obviously good. If it gets committed, most people will be happy (good), and it is likely going to simplify the remainder of Adrian's patch (again, good). This remainder (the structural changes and added indirection) got me confused more than once so I'm not convinced yet that the added complixity improves the theming experience. It didn't work for me, but it looks like it did work for you.

adrian: I am working on another version of my patch to address your concerns. It will include:
* a standardized naming system for templates and styles
* a call to a default function for templating engines such as phptemplate
* an admin interface which only shows options for the features any particular theme/theme engine supports

Templates will not need a .theme file whatsoever, although it will be possible to have one if a template designer wishes to redefine themed functions outside of their template engine.

Engines will be responsible for locating their templates and reporting their names and paths back to the theme system. My goal here is one central theme/template/style selector... not one selector for themes and another for templates.

I will post it in a few hours.

A new patch is attached. I'm a bit sleep-deprived at the moment, so I'll be brief in explaining the changes:
* fixed a couple of bugs in the upgrade path
* styles must now be named style.css... theme system shows pathname in UI, not filename
* $theme_ignore_stylesheets is no more (no need due to above change)
* engines now report back to theme system all their templates using $engine_templates function, which returns an array in the style of file_scan_directory... actual existance of the files in this array are not checked... they are merely used to extract a filename + directory name for the database.
* as a result, no need for .theme files whatsoever for templates
* $theme_engine is no more
* if desired, a hybrid .theme file / template theme can be created... but it now the responsibility of the .theme file to include_once the appropriate engine and run the $engine_init hook with appropriate parameters... .theme files, if they exist in the same directory, always override templates
* added an $engine_init hook similar to the one in adrian's patch... if it exists (recommended), it is passed the $theme variable from init_theme (which includes the database filename + name for use in choosing the proper template)
* $theme_features or $engine_features now decides which options are drawn in the admin interfaces specific to each theme... the global settings page always shows all options
* theme() attempts to call $engine_default($function, $args) before resorting to theme_$function($args) for use with advanced template engines
* xtemplates now must be named xtemplate.xtmpl... theme system shows pathname in UI, not filename
* screenshots are now named screenshot.png... not screenshot_$theme.png anymore

Dries: this should take care of the last of the issues you mentioned. Users can now simply one template/style directory to another and have a duplicate, working copy of a template/style. Any other concerns?

adrian: I'm pretty sure I addressed the issues you mentioned in your first couple of comments (let me know if I missed anything)
with regards to the three points in your last post:
1. just report one of the files (page.tpl.php, perhaps?) to Drupal. The engine calls these files anyway, so this should not be a problem.
2. Honestly, I can't think of a good solution for this either... I tend to agree with you that the only way (without a major restructure) will be to try and provide clear documentation.
3. Styles are stored in the $user->theme and default_theme variables just like they were in your patch. (i.e. chameleon, chameleon/marvin) The styles are applied using drupal_set_html_head and theme_stylesheet (which I also borrowed from your patch). I suppose we could provide a way for themes/theme engines to get items one-by-one drupal_get_html_head, but unless someone has a simple suggestion on how that could be implemented, that seems more like a 4.6 feature to me. :)

Okay... I've been up testing code for way too long. It's time for some sleep. As always, any comments on the patch are welcomed. My test site is once again open for theme administration to anyone who registers.

I see people mention that avatars aren't working yet in this patch. Has the patch been applied to CVS, because they stopped working for me over a week ago?

Note that the patch no logner applies.

I also wonder why none of the theme designers (but a select few) have tested/reviewed this patch.

What do you guys think about this patch?

adrian: I guess I'm not sure what the problem with my present _default hook would be. I was expecting that engines themselves would decide whether they were going to implement a theme function or not (based on a file_exists for the appropriate template or whatever). If they did not implement a given theme, they (the engines) would call the appropriate theme_$function function themselves.

I also considered using a "trigger" value (i.e. FALSE) to tell the theme() function that the engine does not implement a certain function in its _default hook. I decided against this because I couldn't be certain that a theme function somewhere doesn't already return this value for some other reason. If we're certain that no other theme function return (or ever will return) the value FALSE, we could use this method instead.

I guess my question is: why would my method not work? And... how would you keep the _default hook in the engines without a call from theme()?

leafish_dylan: no changes were made to the avatar with the patch other than surrounding it with an if statement so themes can individually switch avatars on and off. I will try to do a test with avatars to determine whether they work for me, but I won't be able to do this till tomorrow as I'm not home at the moment. (If they're broken for some other reason, I'll see if I can't create a patch to fix that as well.)

Dries: I've attached an updated patch which applies successfully to CVS HEAD after the recent changes (i.e. addition of upload.module). I've also taken adrian's suggestion and removed the engine_default capability.

...and the updating continues...
The attached version keeps the patch up-to-date with latest changes to CVS. (and uses capital letters for all header titles as per Stefan's patch)

This also integrates jhriggs' one-line fix for file_scan_directory. It seems that file_scan_directory now sets $file->path instead of $file->filename... If there's a reason for this, I can do a search/replace in the patched system.module so the theme system uses ->path instead. Let me know.

We are currently reviewing this patch in #drupal. Feel free to join.

Here's an update after the review on #drupal. Changes:

- Replaced

with in theme listing.
- Shortened the code for theme toggles.
- Renamed 'Information' column header to 'Name'.
- Show path for every theme, and show path relative to Drupal root.
- Removed example.theme, will be moved to contributions/examples.
- Removed display of the path on 'edit my account': this is admin info, not for users.
- Fixed configure tab: global vs theme-specific settings are now subtabs of the configure tab.
- Improved help text for per-theme settings a bit.
- Allowed configuration options per styles, not just per skeleton.
- Changed placement of title/slogan in pushbutton so that it's closer to the original.
- Changed placement of title/slogan in chameleon/marvin so that it's closer to the original.
- Added border to screenshots and set display: block; on them.
- Screenshots now show part of a page (gives you a better idea of the theme).

Screenshots created using:
* Log in as administrator user.
* Enable the following modules on top of a clean install (for some extra menu items):
aggregator, blog, node, page, story, tracker
* Create the following story node:
title 'Donec felis eros, blandit non.'
Morbi id lacus. Etiam malesuada diam ut libero. Sed blandit, justo nec euismod laoreet, nunc nulla iaculis elit, vitae. Donec dolor. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Vivamus vestibulum felis nec libero. Duis lobortis. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Nunc venenatis pretium magna. Donec dictum ultrices massa. Donec vestibulum porttitor purus. Mauris nibh ligula, porta non, porttitor sed, fermentum id, dolor. Donec eu lectus et elit porttitor rutrum. Aenean justo. Phasellus augue tortor, mattis nonummy, aliquam euismod, cursus eget, ipsum. Sed ultricies bibendum ante. Maecenas rhoncus tincidunt eros.
* Look at the node, and make sure the tabs are visible. Take a screenshot.
* Cut out a piece about 420x254 resized to 150x90 (35% zoom). Try to show useful page elements (menu, tabs, title, links).
* Applied a plain 'sharpen' filter to the thumbnail if it's not crisp enough.

Crap. Forgot about HTML tags ;).

- Replace <h2> with <strong> in theme listing.
- Shortened the code for theme toggles.
- Renamed 'Information' column header to 'Name'.
- Show path for every theme, and show path relative to Drupal root.
- Removed example.theme, will be moved to contributions/examples.
- Removed display of the path on 'edit my account': this is admin info, not for users.
- Fixed configure tab: global vs theme-specific settings are now subtabs of the configure tab.
- Improved help text for per-theme settings a bit.
- Allowed configuration options per styles, not just per skeleton.
- Changed placement of title/slogan in pushbutton so that it's closer to the original.
- Changed placement of title/slogan in chameleon/marvin so that it's closer to the original.
- Added border to screenshots and set display: block; on them.
- Screenshots now show part of a page (gives you a better idea of the theme).

Screenshot guidelines:
* Log in as administrator user.
* Enable the following modules, for some extra menu items:
aggregator, blog, node, page, story, tracker
* Create the following story node:
title 'Donec felis eros, blandit non.'
Morbi id lacus. Etiam malesuada diam ut libero. Sed blandit, justo nec euismod laoreet, nunc nulla iaculis elit, vitae. Donec dolor. Class aptent taciti sociosqu ad litora torquent per conubia nostra, per inceptos hymenaeos. Vivamus vestibulum felis <a href="#">nec libero. Duis lobortis</a>. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Nunc venenatis pretium magna. Donec dictum ultrices massa. Donec vestibulum porttitor purus. Mauris nibh ligula, porta non, porttitor sed, fermentum id, dolor. Donec eu lectus et elit porttitor rutrum. Aenean justo. Phasellus augue tortor, mattis nonummy, aliquam euismod, cursus eget, ipsum. Sed ultricies bibendum ante. Maecenas rhoncus tincidunt eros.
* Look at the node, and make sure the tabs are visible. Take a screenshot.
* Cut out a piece about 420x254 resized to 150x90 (35% zoom). Try to show useful page elements (menu, tabs, title, links).
* Applied a plain 'sharpen' filter to the thumbnail.

After a bit of testing, Steven and I found that Opera had an issue with the pushbutton theme in the previous patch. Here's a zip of the fixed (Opera-compliant) post-patch pushbutton theme.

Thanks for all your help, Steven!

And here's how it looks altogether:
http://acko.net/dumpx/themes.png
http://acko.net/dumpx/themes2.png

Thanks guys. Committed it to HEAD!

Note that the 'search box' setting for Marvin and Chameleon do not appear to work.

I accidentially put 'toggle_search' in the array of settings chameleon supports. The search box is not currently implemented for chameleon. The attached patch makes this change.

Also in the attached patch, I merged Steven's latest changes (from only a couple of hours before this patch was committed) to xtemplate/default/xtemplate.css into bluemarine/style.css.

Oops... my last patch doesn't apply anymore. Here's an update (attached). Summary:
- removes 'toggle_search' from the features array in chameleon.theme
- merges changes to bluemarine from just a couple of hours before template patch was committed

also... follow-up to leafish_dylan: The problem with avatars was an unrelated issue and is now fixed.

there is a bug in chameleons stylesheet import order. it currently is

@import "misc/drupal.css";
@import "themes/chameleon/ax/style.css";
@import (via link) "themes/chameleon/common.css";

that way, i cannot overwrite rules from common.css. the specific chameleon stylesheet should come last, ie.

@import "misc/drupal.css";
@import "themes/chameleon/common.css";
@import (via link) "themes/chameleon/ax/style.css";

TDobes: committed your patch.

I commited a fix for the stylesheet inclusion order, as well as some minor fixes.

If anyone still has issues with the template system, please reopen this issue or submit a new bug report.

I found another theme system mini-bug. There's a small typo in the code for displaying theme-specific settings (the _settings hook). A patch is attached. Thanks to jluster for letting me know about the bug.

Committed. Thanks.

Oops... yet another little bug.

Themes that use a _settings hook are improperly identified as "theme engines" in the UI... this patch ensures that they are correctly identified as "themes".

Hopefully this is the last bugfix for this patch. :)

Looks like I spoke too soon.

The attached patch supercedes the previous one and fixes another minor bug.
Enabled styles whose "parent theme" was disabled would not function correctly. This (and the _settings help text for themes) is corrected in this patch.

Sorry about all the noise.

Committed to HEAD. Thanks TDobes.

Here's another little bugfix.

User theme selection does not respect the "status" bit, listing even themes that are disabled in the admin interface.

The attached patch also allows the $custom_theme variable to specify a disabled theme. (Use case: the admin might want to allow a module to activate a theme on certain pages, but not their users)

Applied to HEAD/CVS.