Let print use the theme-api : using preprocess methods / placing of templates are the core-needs to cover with this

template.php is not used at all in the process of handling the module's template, so it's not surprising that what you're doing doesn't make a difference.

You can edit the module's tpl.php files directly to do whatever advanced stuff you want to do...

João

Shouldn't preprocessing be added? It's what most themers would expect to find, and it's a little non-standard to avoid them.

Hi,

The clean and already rendered version of a node is provided by Drupal via the drupal_render() which is the function used by this module to obtain the node content.. Adding preprocess and alter hooks begin to sound like a replacement of PHPtemplate inside the module, and I for one don't have the resources to create a duplicate of Drupal core inside the print module.

João

PS: However, if someone is willing to prove me wrong and provides a patch for this, I am willing to accept it and integrate it.

You don't need to replace PHPTemplate to use the template processor. See, e.g., Views, which runs all of its templates through a pre-processor.

What #5 said. Modules constantly register preprocess functions. Putting a bunch of code in my tpl files gives me the willies.

I tried to look into this today, but I got a bit turned around due to lack of familiarity with this module's code. jcventura, you say your objection to this patch is that you do not want to duplicate Drupal's theme system in your module, but I see what looks like an awful lot of theme.inc in print.pages.inc. ;)

More or less, converting this would look like:

1. Register 'print' as an index in print_theme(), and give it a 'template' key that's the name of the base template file. See this snippet from node_theme() as an example:

function node_theme() {
  return array(
    'node' => array(
      'arguments' => array('node' => NULL, 'teaser' => FALSE, 'page' => FALSE),
      'template' => 'node',
    ),
    ...
  );
}

This effectively says "register a node.tpl.php template file, and give it these arguments."

Then, whenever you want node output to appear, you call it with theme('node', $node, $teaser, $page); I don't know this module well enough to know which arguments make the most sense for this template file, but I assume something like $format and $node; perhaps others.

2. You create a function called template_preprocess_print(&$variables) that massages the variables going into the template file. This is basically exactly what _print_var_generator() is doing now. The difference would be that instead of building a big $print array, you'd instead be building a big $variables array. Then instead of a themer having to deal with array indexes in their *.tpl.php file, like print $print['scripts']; , they could instead just do print $scripts; like they're used to in page.tpl.php.

You can check out http://api.drupal.org/api/function/template_preprocess_node to see how node module does this.

3. For the "wildcard" templates like print-node-$type.tpl.php and so on, you can populate the "template_files" index of the variables array, as shown at the bottom of http://api.drupal.org/api/function/template_preprocess_node:

  $variables['template_files'][] = 'node-'. $node->type;

I don't think I'll have time to work on this myself, but it would be really nice from both a developer experience and themer experience standpoint to have this issue addressed. Right now, this module is doing its own unstandard theming workaround instead of utilizing the tools in core as people would expect.

Well i have implemented it. For both, the node rendering (printnode) as well as for the page rendering (print_page) the themapi is used

theme('print_page', $print, $format, $node);

$content = theme('printnode', $node, $format);

This makes everybody able to use:

- preprocess methods for hook_preprocess_printnode and hook_preprocess_print_page
- Place there templates according to the theme api ( submodules .. subsubmodules ... modules ... )
- adjust the template search directories using the themeapi
- adjusting the choice of the the template file using the themeapi

And actually, it even shortens the code :)

Patchbase is 1.10

The patch

Rerolled patch, added some docs and cleaned up coding style

Project is now on github : http://github.com/EugenMayer/print/tree/usethemeapi
The master branch is the current 6.10.
The dev branch is usethemeapi + the current fix for field-size for wkhtmltopdf settings. This will be the branch where any patches will get applied to.

+++ print.pages.inc
@@ -452,6 +445,7 @@ function _print_friendly_urls($url = 0) {
+<<<<<<< Updated upstream
  * Choose most appropriate template
  *
  * Auxiliary function to resolve the most appropriate template trying to find
@@ -497,6 +491,8 @@ function _print_get_template($format = NULL, $type = NULL) {

@@ -497,6 +491,8 @@ function _print_get_template($format = NULL, $type = NULL) {
 }
 
 /**
+=======
+>>>>>>> Stashed changes

ehem

+++ print.pages.inc
@@ -585,8 +581,8 @@ function _print_generate_node($nid, $cid = NULL, $format = PRINT_HTML_FORMAT, $t
+      dsm($content);

ehem

Powered by Dreditor.

thank you daniel.

corrected patch attachede

In general, this patch makes a lot of sense and would bring this module closer to what core does.

Some nitpicks below.

+++ print.module
@@ -94,10 +94,50 @@ function print_theme() {
+      'arguments' => array('node' => NULL,'type' => ''),

This needs some blanks in between.

+++ print.module
@@ -94,10 +94,50 @@ function print_theme() {
+      'arguments' => array('print' => NULL,'type' => '','node' => NULL),

This as well

+++ print.module
@@ -94,10 +94,50 @@ function print_theme() {
+ * Clones the theme regestry for nodes for print_node to make it work the same way in the same directries

This comment has a typo and I don't really understand what it tries to say. I don't do much theming, though.

Powered by Dreditor.

Thanks for reviewing. I guess those 2 first things can be corrected by the maintainer when applying, they are very minor. I think the comment is fine, its just one of the "magic" things of this patch actually and not to trivial i guess, when you dont know that theming internas.

It's trivial to fix.

Looks cool to me. However a little too much code than i'm able to review in my head without real checks ;-)

hmmh naming back and changing status - updated by concurrency somehow...

Hi,

I was very fond of this patch, so I'm trying to push into the code before I create the next release..

However, I am now thinking that this was entirely created as a patch and never actually tested. After applying the patch, I was presented with a blank screen. I traced this to two problems: the fact that the new theme('print_node') call is never actually implemented in the code and the second problem that the ouput of the theme('print_page') call is never sent to the user's browser, because the line "print $html" that does this was removed.

Taking a cue from node_view, I have inserted a call to theme('node'), and I thinking that maybe the solution is to create a theme_print_node() function consisting of the line "return theme('node', $node, $teaser, $page".
I've also maintained the template as print.tpl.php instead of the new print_page.tpl.php..

Please take a look at my version of the patch and tell me what you think about it..

João Ventura

Iam using this stable for more then a month and this patch has been tested. All i can think of are some theme_regestry issues with custom themes, please verify on garland.

Beside i have patched this module a dozen times more, iam not anymore interested in contributing here - iam sorry. I have forked off already and will rewrite it part by part using OOP, espcially the controller part. You did a good job in the past (there is a lot of effort in this module - pioneer work) but currently it seems like the module is growing wild - historical wise.

In the end, the process made in the issues is far to slow for my needs and of course, its hard for one person to hold the flag up for such an module. Maybe you might consider looking for co-maintainers and helpers - inviting people to this project.

Printing will become much more important with all the new distros like Open Atrium or Drupal Wiki (Drupal Commons) and a lot more upcoming.

If you want to help maintain it, sure. I would welcome it. I think it's better than forking it. As to the "patched this module a dozen times more", in github your last commit was the same patch as #12. It would be nice if you could at least share with the rest of us the dozen patches you've made, as they seem important. Also, in your patch you've deleted the drupal_final_markup() calls, thus re-introducing some security problems which forced a release some months ago.

As to my problem with this patch... My install is a fresh install of Drupal 6 using Garland, and very little else. I am using PHP 5.3, but that shouldn't be too much of an issue. I have verified that the theme registry works and is needed, but the code is simply incomplete.. How are you getting any output if you never print to the browser the result of the theme('print_page') call?

I agree with you that the controller part is indeed a bit dated, as it really hasn't seen anything radically new since the version for Drupal 4.7 :). However, short term I would like to concentrate on creating a print UI module to reduce the triplication of code that is currently 80% of each of the three modules.

João

OK. My fault, indeed the theme registry was probably not updated yet. I wasn't getting any output of the theme('print_node'), but it started working with no changes.

I've committed the patch to CVS. Thanks for the patch!

João

You're right..

That's why I started getting "output" of the theme('print_node').. There still isn't any, but during one of my tries, I started ignoring it :)

I'll get back on trying to fix this..

I've found and fixed the problem..

All that was needed was to add a call to template_preprocess_node($vars) in the print_preprocess_print_node() function.

It's in CVS now.

Zhgenti: nothing of what you said applies in the dev version. You must have applied the patch in #14 at some point.

One of the changes that I did in my patch was to remove the rename of the print.tpl.php file to print-page.tpl.php (which was unnecessary and would only create dozens of support issues).

Please get the latest dev, clean your Drupal cache and you should see that it is the print.tpl.php file that is used normally.

João

I got a report in #861916: Custom print.tpl.php not recognized even after clearing cache that the print.tpl.php wasn't working on the theme path, so I added the page's theme paths to the print_page theme paths.

It seems to work now.

Mhhh.. I'm getting

include(./sites/all/modules/print/print_node.tpl.php): failed to open stream: No such file or directory in /var/www/drupal-hfopi/includes/theme.inc on line 1066.

and no body content on the print page? tried to fix it but no dice, reverting this locally for now..
Can anyone confirm the error message?

Have you tried to run update.php or clearing the theme cache?

Sir, yes, sir! no idea what's wrong.

Are you using the latest dev version?

Is that question intended to insult me lol?

Actually, no.. I know that you've proposed several patches which weren't (yet) merged to the module, so I am guessing that you're probably running something not based on the current dev.

Basically, what I think it's wrong is that the module is unable to find the node.tpl.php file in your current theme directory. This may happen when you're running sub-themes..

João

I'm running the -dev version from 2 Aug. I like the direction of this. Harmonising with theme API and allowing preprocess hooks are a good step forward.

However in 6.x-1.x-dev, the print.tpl.php is still using invocations like print $print['scripts']; when it can now use print $scripts;

There are some quirks to be ironed out; print module is building its own print[css] array instead of just using $styles.

Actually, no..
was just kidding ;)
so I am guessing that you're probably running something not based on the current dev.
Well yes, but i am always carefully looking at the diffs applying them one by one. With this, what remained as cause for empty node content after manual 'bisecting' was this patch, obviously.
Basically, what I think it's wrong is that the module is unable to find the node.tpl.php file in your current theme directory.
Seems exactly to be the problem, my theme does not override the default one. Would it have to for making this patch function?

print module is building its own print[css] array instead of just using $styles.
Which is making sure the output is actually printable - most times something is printed you want it to be black text on white paper, not colour on colour.. hence the existence of the print module! many theme's CSS files have set media=all so these styles would not automatically surpressed, so there's good reason to use a reduced CSS set..

eMPee584: Thanks for pointing that out.. It seems that the code in print_theme_registry_alter is incomplete and it needs to include some more paths. Probably it should make sure to include also the default node.tpl.php.

jonathan_hunt: Indeed, now you have access to the $styles variable, but the default print.tpl.php is still using all the old variables.. This is because:

a) I'd like to maintain some backwards compatibility for now (now being until the official release of Drupal 7, so that I can move the Drupal 5 version of the module into bug-fixing only). I'm planning to break the print.tpl.php backwards compatibility in version 6.x-2.0 of the module, with a print.tpl.php containing a lot of the code that currently generates the print array.

b) as eMPee584 pointed out, the content of the $styles variable includes all the theme's CSS and one of the thing that this module does is to remove all the theme's CSS. In #871406: Print pages look bad there's an example of what happens when themes don't identify their CSS as theme-specific and the problems that creates for this module.

Subscribing

The latest dev already supports the default node.tpl.php.

The important change was:

-  $theme_registry['print_node']['theme paths'] += $theme_registry['node']['theme paths'];
+  $theme_registry['print_node']['theme paths'] = array_merge($theme_registry['print_node']['theme paths'], $theme_registry['node']['theme paths']);

By using array_merge instead of += the initial value of $theme_registry['node']['theme paths'] is preserved, and as that is the one pointing to the default node template, it now gets found.

Yes indeed! Now it works ;)
..and thx for explaining the code fix btw!

It would seem that the print template files must be in the same folder as the page.tpl.php template file.

In my case I put my templates in a hierarchy (theme/templates/pages, theme/templates/nodes, theme/templates/forms etc).
So the print templates must go in theme/templates/pages otherwise they are not discovered.

$theme_registry['print_page']['theme paths'] = array_merge($theme_registry['print_page']['theme paths'], $theme_registry['page']['theme paths']);

$theme_registry['print_page']['theme paths'] does not include the root theme path, hence the issue.

Answering #40, I guess this is the same with node template, isn't it?

Need help! I use print 6.x-1.x-dev (2010-Sep-06). I need to print not only content, but some regions from my page.tpl.php. I added

    'print' => array(
    'arguments' => array('print' => NULL, 'type' => PRINT_HTML_FORMAT, 'node' => NULL),
    'path' => drupal_get_path('theme', 'sibiryak') . '/templates',
    'template' => 'print',
    ),

to my template.php file. And also added a preprocess function, just for test I added a testing variable:

$vars['testing']='testing';

Then I added

<?php print $testing ?>

to my print.tpl.php file.

Cleared the cache. But there is no 'testing' ever printed at my print page. Where is the mistake?

PS. Sorry for my English.

Well, the first mistake is to re-open closed issues with not-directly related stuff.

After that, the reason why it's not working is that the theme function is actually called print_page, not print.

João