Issue status update for http://drupal.org/node/16409 Project: Drupal Version: cvs Component: theme system Category: feature requests Priority: normal Assigned to: gordon Reported by: gordon Updated by: Steven Status: patch True, but the whole point of PHPTemplate is to make theming 'nicer': without the separate templates and easy variables, it's the same as a .theme file. Anyway it was just an idea I had. It's not critical, but I do think not having named variables will make templates a lot harder to make and read. Steven Previous comments: ------------------------------------------------------------------------ January 29, 2005 - 23:31 : gordon Attachment: http://drupal.org/files/issues/theme.inc_1.patch (889 bytes) Attached is a patch that I would like to get into 4.6. I didn't realise it but I am cutting this close. What this patch does is give the ability to theme engines to be able to handle all theme hooks with specifically having to code for them by creating a function {theme engine}_{theme hook} (eg. phptemplate_pager) What I have done to achieve this is I have created a new theme engine hook called theme_handler. This will check to see if the current theme has been set up to include this theme hook, and returns the function that it should called to handle this theme hook. Most the time this would most likely be a generic theme function. This function is then called with exactly the same arguements as the theme() was called with. (eg the theme hook as the first arguement) So in the case of the pager the function is called like so <?php phptemplate_generic("pager", "", 10) ?> The method that I have used, theme engines are not required to use this functionality, as some theme engines such as I think xtemplate will not be able to handle this, but it works extremely well in phptemplate. The reason that I have made this modification is that i can use a theme engine such as phptemplate to create the theme, and if I want to theme any additional features such as pager, menu_tree, I can without having to modify the theme engine from standard. So this gives me the best of both worlds. So with the phptemplate engine all I need to do to make use of the additional functionality is add template file to the themes directly like:- hook = hook.tpl.php, pager = pager.tpl.php, etc. It also means in the case of the phptemplate engine the theme creaters php skills can be limited, and still be able to expand the theme beyond that of the engines capibilities. ------------------------------------------------------------------------ January 29, 2005 - 23:32 : gordon Attachment: http://drupal.org/files/issues/phptemplate_0.patch (1.12 KB) and here is the patch for the phptemplate engine to make use of this functionality. ------------------------------------------------------------------------ January 30, 2005 - 18:10 : FactoryJoe@civicspacelabs.org It seems to me that this functionality belongs in PHPTemplate and not another theme engine. Are there any possible security issues raised with using such an open-ended patch? ------------------------------------------------------------------------ January 31, 2005 - 13:34 : Anonymous This was part of my original design for the new theme systes, but was removed because it was considered unneccesary complexity that only affected phptemplate. I think this is the right time to add it however, as it allows lots of really creative things. Also, most of the available template engines (smarty , phptal , phptemplate) are based on my original phptemplate code base, and as such will benefit from this patch. ------------------------------------------------------------------------ January 31, 2005 - 13:48 : adrian Bah. I wasn't logged in. Anyway, to answer joe's question .. this needs to be modified in core so that phptemplate can leverage it. It really is only a small change in theme.inc. ------------------------------------------------------------------------ January 31, 2005 - 21:10 : gordon Looking at the patch yes I do think there maybe a little security concern. But I am not exactly sure of the best method to employ. 1. I could just return from the theme_handler a true/false and then always call {theme engine}_generic and it can work it all out, which in the case of phptemplate is all that it needs to call. or 2. have the theme_handler return the hook, and then execute it as {theme engine}_{hook} which does give you the added flexibility, but it is not as much of a security issue. Also as said before, we change to core is required so that hooks that are not know by the theme engine have some way of being called. ------------------------------------------------------------------------ February 2, 2005 - 05:53 : gordon Attachment: http://drupal.org/files/issues/theme.inc_2.patch (908 bytes) I have made a change to the patch so that the process that is called is called {theme engine}_{returned hook} which I feel is more secure, but gives you more flexibility if the theme engine has the default handles split up for ease of use. ------------------------------------------------------------------------ February 2, 2005 - 05:54 : gordon Attachment: http://drupal.org/files/issues/phptemplate_1.patch (1.11 KB) and the patch for phptemplate ------------------------------------------------------------------------ February 3, 2005 - 03:37 : adrian Ok. I have (possibly) a spanner to throw in the works. The variables passed to the template appear in which format? I recall now why I didn't include this functionality. Essentially, the template handling code accepts an associative array, and passes each of the elements in the array to the template as a variable .. hence : <?php $vars = array( 'varA' =>'text 1', 'varB' => 'text 2', 'varC' => 'text 3); ?> Is available internally in the template as : <?php $varA = 'text 1'; $varB = 'text 2'; $varC = 'text 3'; ?> Because of how php (and most c based languages work), the called function has to name it's parameters, and with a generic function, the best you can do is numbered variables. Of course, $1 isn't a valid variable name, so the best that could be done is $var1, $var2 , $var3 or something similar. To combat this would probably require rewriting the theme() function to accept an associative array, but the amount of bloat and opportunity for error that causes is phenomenal. The primary reason I am not using a generic template passing function, is that I can't pass well named variables to the template. ------------------------------------------------------------------------ February 3, 2005 - 04:02 : gordon Yes I know that you cannot create any meaningful variable names which is why in for the phptemplate_generic() I only export a variable called $args which includes all the arguements that have been passed to the generic hook. This means that it can handle all unknown themeing functions without having to modify the theme engine which makes my life alot easier in the future when I have to upgrade client sites. Any ways anything beyond the current theme hooks that are provided by phptemplate do require some expert knowledge to make sure that they done stuff up. It also means that you can have the simplicity of the phptemplate engine to create your theme and be able to theme everything that you desire. An example of this is that I want to use phplayersmenu to format my navigation block. At this stage I have created a module called navtree which does this for me, but with this patch I am able to create a menu_tree.tpl.php and I can add in phplayersmenu in the theme and not have to have a little module that does it for me. ------------------------------------------------------------------------ February 21, 2005 - 03:59 : Chris Johnson +1 This seems like a good addition to me. It provides theme developers a lot more flexibility without having to become module authors as well. ------------------------------------------------------------------------ February 21, 2005 - 07:55 : bryankennedy +1 - i agree this would be super cool for 4.6 ------------------------------------------------------------------------ February 27, 2005 - 01:24 : Steven One possible way to get around the naming issue might be to store the parameters of all themable functions and name them the generic arguments that way. Of course it has to be maintained whenever themable functions change, but you could do this with a script which regexpes for theme_ functions and extracts the argument names: <?php global $list; function iterate($path) { $d = opendir($path); if ($d) { while ($f = readdir($d)) { $n = $path .'/'. $f; if (preg_match('/\.(inc|module)$/', $f)) { $data = file_get_contents($n); parse($data); } else if (is_dir($n) && ($f{0} != '.')) { iterate($n); } } closedir($d); } } function parse($source) { global $list; static $done = array(); preg_match_all('/function (theme_.*?)\(.*?\)/', $source, $functions, PREG_SET_ORDER); foreach ($functions as $v) { $name = $v[1]; if (isset($done[$name])) { continue; } $done[$name] = 1; preg_match_all('/\$([A-Za-z0-9_]+)/', $v[0], $args, PREG_SET_ORDER); $item = array(); foreach ($args as $arg) { $item[] = "'". $arg[1] ."'"; } $list[] = " '$name' => array(". implode(", ", $item) .")"; } } $list = array(); iterate("."); print "array(\n". implode(",\n", $list) .");\n"; ?> ------------------------------------------------------------------------ February 27, 2005 - 08:40 : gordon this is great, The biggest problem is keeping the list up to date, and for people who have written there own custom theme functions they will have to alter the theme engine themselves which is what I am trying to avoid with this patch. You could turn this into a standard function that can be called from the generic function. You would added caching and lower the overhead. But still in the end what is it really doing for you, instead of having $args[] you would have a the real variable name, which will make it pretter but no additional functionality. I do like this but other than creating a nicer set of varables for when calling generic theme functions what does this really give you. This seems like alot of overhead when you can just reference $args[0].