Book HTML export does not print bodies

Can you duplicate this with rc1?

This is a very quick while-waiting-to-leave-for-dinner-with-the-wife sort of patch, but I believe it should fix the underlying problem. It makes the book rendering mirror the node_view() process, with $node->content being built properly and all that.

There's an important change, though: anything that implements nodeapi op 'print' will need to move to the new $node->content based mechanism, just like things that use op 'view'. This isn't much of a hassle, though, since they don't work at *all* with the current code.

Set to 'code needs work' because I haven't even had a chance to run it yet, but the structure should be correct.

No, ignore that, a bit of an old patch snuck in. This version is the proper one.

We discussed a much better solution... which is less code actually.

Correcting title.

A slightly altered version, with a more fleshed-out comment. The $op parameter is actually useful in other contexts as well, and I don't think we should call it 'internal use only'...

But that's a minor issue, really.

Code looks okay, but I am not sure if we should be making API changes like adding the $op parameter at this point.

Code looks okay, but I am not sure if we should be making API changes like adding the $op parameter at this point.

Code looks okay, but I am not sure if we should be making API changes like adding the $op parameter at this point.

Two options: either we copy the contents of the node_build_content() function, duplicating it with a single changed line in the book module, or we add one optional parameter. If the former, http://drupal.org/node/100963#comment-170090 will work. I think the latter option is definitely better.

Also, please note that as $op defaults to view the change is BC.

Not sure about the patch. It smells a bit like a hack. I'd love to see the alternative patch so we can compare against that ...

(Also, the code comments are lacking. What are valid values for $op? It is non-trivial to find out as you have to chase quite a bit of code.)

Not sure about the patch. It smells a bit like a hack. I'd love to see the alternative patch so we can compare against that ...

It's a hack because book.module's original print-friendly-output code is (in relative terms) a hack. A clean and elegant solution to the underlying problem would be the ability to render nodes in a variety of different 'modes', like 'full' or 'teaser' or 'print' or 'feed'. That's something that I'd like to tackle in 6, as we take a hard look at the node loading system and the way we integrate CCK-style fields.

(Also, the code comments are lacking. What are valid values for $op? It is non-trivial to find out as you have to chase quite a bit of code.)

Any string is technically valid for $op, in the same way that any string is technically valid for $op when invoking nodeapi. The new parameter simply allows modules like book, which are building something *different* than a standard content view, to avoid much of the cruft added on by nodeapi $op view, and use an alternative: 'print'. It's definitely an edge case, but it's there.

We can do that by copying and pasting node_build_content() and making it book_build_content(), or we can add the optional param. I think the latter is very much the lesser of two evils, and makes this kind of application possible without lots of tremendously code duplication.

I can put together more extensive comments for the parameter, but I think we should decide whether the code itself is the direction we'd like to go before investing too much time in that.

Dries, the other option is http://drupal.org/files/issues/book_body_display_0.patch (#6) but that's a waste, it's a word-by-word copypaste of node_build_content.

Sure, it may be a waste, but it isn't an API change.

"... the ability to render nodes in a variety of different 'modes', like 'full' or 'teaser' or 'print' or 'feed'. That's something that I'd like to tackle in 6, as we take a hard look at the node loading system and the way we integrate CCK-style fields."

This isn't something we should be thinking about for this issue.

I'm okay with the copied code and a brief TODO.

I think that #6 makes for the better patch. Sure, it might duplicate some code, but it is more transparent and doesn't change the API. Maybe add a little TODO-comment to the patch in #6?

This isn't something we should be thinking about for this issue.

That seems a little silly; deliberately ignoring solutions that will help solve other problems, because that would imply a 'feature', seems counter productive. But, as trivial as it is, I suppose one could say that this changes an API. #6 it is.

Committed #6 to HEAD with a todo comment.

While the API addition was minor and potentially useful, I saw it as a tip-of-the-iceburg type thing. I don't want to see a API that begins to handle different node templates being abused. This probably is something worth looking at for Drupal 6 when we have a full development cycle to get it right.