PHPTemplate: Map menu arg path to page template filename

+1 million from me for this idea. nice one Eaton

Dub

forgot to mention...I tried the patch and it works perfectly.

Dub

I like the generality of the approach, since it also means any module that creates a custom path can have a separately themed section.

Great idea, IMO. I didn't try it, but I don't see any potential problems with the code.

The patch code looks so obvious I'm surprised it doesn't already work this way!

One question, comparing your patch to the code on this handbook page:

http://drupal.org/node/45951

I'm wondering first if you need to special-case arg(0) == 'node'. Right now, I think your code would look for 'page-node.tpl.php' for every page with URL node/x, no?

+1 from me

with this one nobody ever needs to ask for an admin theme

only what will happen when you go to admin/settings or any path with more than one level... if that would work too it would be even cooler

Not to play captain grumpy here, but this patch is a demon we should avoid. I have enough problems getting one page.tpl.php right -- the idea of having a seperate page.tpl.php file for every arg(0) would be nightmarish from a theme development perspective. This is a great snippet of code, but it doesn't belong in the engine. People have enough trouble theming as it is, without then having to worry about maintaining 5 seperate page.tpl.php files.

Again, good snippet -- bad and uncessary default for the engine. What's wrong with just using the template.php file? My 2 cents.

Besides, most of the time, all you need to do is change the body ID of the page.tpl.php if you want wildly different looks. Remember?.

I don't think 5 files are required. If I understand the patch right, page.tpl.php will still be the default if no other temple is found. Thus, the patch will not break existing templeates, only allow a somewhat simpler mechanism for theming admin or other sections for those who care to try.

Well, that may be true, -- actually, wait no, I just tested the code and I was dead wrong. Did I mention I'm tired?

+1 from me

I still think that this method leads to a world of trouble ( with a capitol T )... but that's just my opinion -- and I'm a CSS snob and zealot.

Code works great, makes sense, and gives people more freedom. So I'm all for it.

If I'm understanding this correctly... would it make sections.module obsolete? And how could you get the same effect with some other engine, such as Smarty? Or is that even possible?

A little bit of topic, but from a user point of view, I always wanted to be able to associate a template with my posts (node pages). Here is how I envision this to wok. A designer makes me several templates; in its simplest form that could mean various templates with different color scheme, or maybe with different images. Then, when I'm writing a post, a drop down menu is presented that lets me pick one of the available templates. So when I'm writing about Drupal, I'd select my 'Drupal template' and when I'm writing about 'Photography' I'd select my photography template, or when it is Christmas, I'd select my Christmas template (regardless of the fact I'm writing about Drupal or Photography). So as a writer, I get to choose the template.

Of course, this isn't what this patch is about but I wonder if this patch will make it harder or easier to accomplish that.

I haven't tried it, but doesn't the Taxonomy Theme module do just what Dires wants?

I'm not sure where this kind of module injects intself into the process, but it might be worth checking that this patch won't have the potential to break all the existing theme-switching modules.

Dries said:

A little bit of topic, but from a user point of view, I always wanted to be able to associate a template with my posts (node pages). ...........Of course, this isn't what this patch is about but I wonder if this patch will make it harder or easier to accomplish that.

I don't think the patch will affect your "select template for this node" idea, Dries.

As I understand it, the way the page.tpl.php or page-admin.tpl.php works is that it loads the Node content (your posts) and places it within the overall page. So the "select template for this node" option would still work. The patch just gives users a very simple option of controlling and managing the overall, full page layouts (not node layouts) by uploading a page-variation.tpl.php file.

The patch just simplifies that in the same way you can upload a node-forum.tpl.php file or a node-image.tpl.php file to automatically override the default node.tpl.php for specific node types.

incidentally, there is a new, very useful module called the Content Template Module by jjef that is very useful for specifying a bespoke node template for content types. I imagine it wouldn't be too difficult to tweak that module to offer the available templates as a drop down list when creating a node. So you can select a "photography" template or "general post" template as opposed to it happening automatically.

hope that makes sense. I think it's a good patch, but, I would wait until a phptemplate developer double checks it. For example, to ensure that the correct variables are still available to the page-variation.tpl.php file.

Dub

In terms of this approach vs. the previous, I'd be curious if there is a way to benchmark the relative performance? I think the most recent patch provides more flexibilty, but that's not worth a noticeable performance hit.

In terms of the last patch, a quick question about the naming of the function phptemplate_page_filename() . Would it be better to name it theme_page_filename()? I'm only making the suggestion since I usually name my theme functions like phptemplate_search_item() so that I can easily move/reuse them among phptemplate-based themes.

I wanted to be able to usea different template if it's the front page and the user is not logged in -- i.e. page_default.tpl.php (not logged in) versus page_front.tpl.php (logged in) if I was to avoid my own custom logic altogether (not that I should necessarily be able to, though a dedicated designer would appreciate not having to know the necessary PHP).

With Jeff's latest patch, while I still need logic to check if the user is logged in, I can include that logic in the page-front.tpl.php itself, thus keeping things cleaner.

+1

From browsing and reaponding to question in the forums, I thiink many people are overwhelmed trying to get a handle on how to use phptemplate. For this reason, I think using the simple approach is far preferrable. It would be much easier to explain to new users, and would probably capture the functionality needed by 90% or more.

I am very much for this.

I was almost on the virge of adding it myself a couple of times.

it's real nice, indeed.a much needed feature elegantly implemented in only so many lines of code.

This sounds great- sorry I have not had a chance to test it yet.

Two quick questions- I assume this walks the unaliased URL? For example, what happens if I alias my first forum (/forum/1) to /general-discussions?

Second, I don't know when this would be useful, but would this enable a separate page template for each node? I.e. page-node-100.tpl.php for /node/100?

Oh, wait, I see in the patch the code below for constucting the suggestions., so I guess the answer is no.

  while ($arg = arg($i++)) {
    $suggestions[] = $suggestion . '-' . $arg;
    if (!is_numeric($arg)) {
      $suggestion .= '-' . $arg;
    }
  }

Doesn't this need the base_path() before the path_to_theme() in there for compatability with weird setups?

Otherwise, Eaton, what's your address to mail some beer too??? :-D +1 indeed!

Actually, it WOULD make that possible. page-node-1.tpl.php is still one of the suggestions, just not page-node-1-edit.tpl.php.

Really? I thought that !is_numeric kills that.

- Update the in-code documentation.
- Don't use += on arrays.

phptemplate.engine_12.patch appears to contain two typos: $suggetions is used in two places instead of $suggestions

I'd love to hear Adrian's opinion about this patch.

Why do we need the $extension = '.tpl.php'? Looks like that can be removed/simplified.

*.tpl.php is a way to distinguish template files from other php files.. and *.php effectively hides the contents from unsuspecting directory hackers.

the only problem with it at the moment is the double extension in apache file uploads.

on the other hand, *.install, *.inc and *.module are already used, so maybe we can drop the extra extension anyway and don't worry about the false idea of security that should be handled by apache's .htaccess anyway in this case

This entire thread answers a number of questions I had, but I still would like an ID solution for the page such as Nick had suggested above. He wrote:

>

How do I do this? Let's say I want the user's blog edit form to have some custom tweaking only at the CSS level. THe path is

../drupal-4.7.2/?q=node/17/edit

How and where could i transpose this into a parent class that gets inserted into page.tpl.php file like this:

  <td class="main-content MY-class-HERE"     id="content-<?php print $layout ?>">

This second class will give me more than enough control over many thinks in CSS, without having to tamper with the current ID logic.

Somewhere, i have to decide the class, which I could decide with eaton's patch (I think). But then I have to assign it to a template variable which page.tpl can use (and I don't know how do make those either).

Guidance appreciated here.

Oops.. forgot what code works here. THe example above should have been

<td class="main-content MYCLASS_HERE" id="content-<?php print $layout ?>">

You lost me. Template.INC? (or php?)
What class am I getting via getcurrentpageClass? There IS no useful class yet.. is there?

What i meant was that I could derive a class name from some aspect of the path with a regex, and then create a page classname for the template.

Ok.. you meant I could write my own getcurrentpageclass() function, I assume. and yes, I could, but BASED ON WHAT? I need something generated from the URL, I assume like comments/add, node/edit, q=node/add/page, etc.

These woudl become classes like comments-add, and node-edit.

I would contruct this in a hacked phptemplate engine file and then assign the class to a new template variable that could be easily used in all subsequent template.phps

WOuldn't something like this work?

As a newb, I think this is something Drupal desperately needs. I think I should be able to theme a page without having to touch template.php... this patch makes that possible, right? :)

Now, I'm trying to apply the patch but I'm getting an error...

C:\wamp\www>patch -i phptemplate.engine_14.patch -p0
patching file `themes/engines/phptemplate/phptemplate.engine'
Hunk #1 FAILED at 46.
Hunk #2 FAILED at 63.
Hunk #3 succeeded at 215 (offset -4 lines).
Hunk #5 FAILED at 328.
Hunk #6 succeeded at 364 with fuzz 1 (offset -9 lines).
3 out of 6 hunks FAILED -- saving rejects to themes/engines/phptemplate/phptempl
ate.engine.rej

Please help! :)

*ouch ouch* — +1 — Looks good to me for what it's worth. Great feature. — Rob

Nice solution for a very often-used/requested feature. Would be great to get this committed.

As a self-proclaimed "drupal-theming-ninja" I love this patch.

i played with this. works as advertised. nice patch.

In the theme('page') section, $suggestions[] = $suggestion; looks like use of an uninitialized array. Please use $suggestions = array(...).

Making the patch with cvs diff -uF^f will make that sort of thing easier to verify.

What is the performance hit for stating one file per path argument?

@drumm - $arr[] = i a perfectly valid way to initialize an array. it seems that many people don't realize that, but the PHPDocs say so: "If $arr doesn't exist yet, it will be created. So this is also an alternative way to specify an array." from http://us3.php.net/manual/en/language.types.array.php. Further, this syntax does not even generate a NOTICE.

I leave this open because of neil's performance question.

i think eaton addressed the performance question, and i addressed the array initialization question. set to RTBC.

Committed to HEAD with some code style cleanup.

I did change the array initialization I mentioned earlier. I think it is slightly more expressive because it doesn't double as appending to an array.

I have "Warning: array_reverse(): The argument should be an array in ...\phptemplate.engine on line 339" (and line 340: foreach())

I've trace it and it IS array when values are "page-something" and "node-something", but it's string when it contains "field-something".

I use HEAD versions of Drupal and CCK.

Thanks!

Thank You, Eaton!
I solved this problem by simple adding '(array)' to variable (Line 339):

<?php
// Loop through any suggestions in FIFO order.
$suggestions = array_reverse((array)$suggestions);
?>

So will You please to formulate an issue to CCK project?

Great minds, etc . I stumbled on this same solution working with a hack to Smarty's template engine. In my case, I wanted the ability to restyle the layout based on the URL. I'm using URL rewriting so that my pages look like:

cms.foo.bar/resume
cms.foo.bar/house
cms.foo.bar/dogs
cms.foo.bar/dogs/auggie
cms.foo.bar/dogs/jack
etc.

I wanted the "/resume", "/house" and "/dogs" pages to have different banners, colors, even different layouts based on the URL. That is, all '/resume' pages could have one look, all '/dogs' pages could have another layout. In addition, I wanted to be able to fine tune the layout to the second and third levels, if necessary, so that each of my dogs' web sites (yes, my dogs have their own web sites -- so shoot me) could each have slightly different layouts, if necessary.

What's nice about the _smarty_callback function is that you can (as noted above) pass it a "suggested" template resource. If it's not available then the software will default to the standard 'page.tpl'. What a nice safety net. You won't generate a Smarty exception if you forgot to create that "suggested" page template.

I haven't built a patch file yet but this mod is pretty easy to insert into smarty.engine with a non-corrupting text editor. Line 223 of the box stock smarty,engine is:

  if ((arg(0) == 'node') && is_numeric(arg(1))) {
    $variables['node'] = node_load(arg(1));
  }

  return _smarty_callback('page', $variables);
}

This is where I added the patch:

  if ((arg(0) == 'node') && is_numeric(arg(1))) {
    $variables['node'] = node_load(arg(1));

    // PATCH(SHM)
    if (isset($variables['node']->path)) {
        $path_elements = split('/',  $variables['node']->path);
        $variables['path_elements'] = $path_elements;
        return _smarty_callback('page', $variables, 'page-' . $path_elements[0]);
    }
    // END PATCH(SHM)
  }

  return _smarty_callback('page', $variables);
}

If the first element of path is /house then Smarty will try to load the template page page-house.tpl in your theme's root directory. Failing that it will load the default page.tpl as usual.

In addition, the path elements are broken out into an array and stuffed into Smarty's variable namespace as 'path_elements' so you can use it to fine tune the page template using Smarty ops, i,e.

$path_elements: array(0 => dogs, 1 => auggie);

Your Smarty template can use this to, for instance, insert a different photo in the banner.

{if isset($path_elements[1])}
 img src="{$path_elements[1]}.jpg">
{else}
....
{/if}