TOC 7.x not working yet?

hook_filter_FILTER_tips() is documented here: http://api.drupal.org/api/drupal/modules--filter--filter.api.php/functio...

The development snapshot of the TOC module for D7 only supports TOCs added through blocks currently. I hope to have a more functional version of the module available in the near future. Sorry I can't give a more definite timeline, I seem to be having as much trouble as everyone else tracking down why the other two methods for adding the TOC to pages aren't working. Patches are always welcome though!

"So when I click a link in the TOC, the page URL changes, but it doesn't jump down to the header." open a new bug/support request report please.

lol...

Should I have to say than title like "doesn't work" are not the best to categorize, follow issue and helping search engine?

subscribe.
Today, the filter is not working, but block is. However most of the extra block config settings don't seem to be implemented fully.
I'll be looking at it to see if I can get the back-to-top insertions going, though I feel that they work best as part of the filter process, and I suspect that the D7 filter process will be interesting to get my head around.

This looks promising:
http://plugins.jquery.com/project/pagemenu

Many others are available also
http://plugins.jquery.com/plugin-tags/table-contents

subscribe

i would build the TOC on node save with php and not via jquery.

subscribe

@BarisW : done !

Okay people, I know what's wrong and I have fixed it. Will sort out the patch presently (I hope - I've never been very good at generating patches - at worst I'll just zip up the replacement.)

What's the problem? Well part of the problem is the documentation and implementation of the Filter module in D7, it's (a) not explicit, it just pretends to be; and (b) it doesn't follow D7 principles which would have made it a lot simpler. It obviously just didn't get the love it needed.

The reason the TOC module doesn't work is because the necessary code file (tableofcontents.pages.inc) does not get loaded when it's needed. So it doesn't get executed. That's it. Nothing else.

I wouldn't have known about this but I just wrote a Wiki filter (all the existing Wiki filters are broken, or a nightmare [flexifilter, I'm looking at you]), and then I wanted to include the TOC module but bas we all discovered - it doesn't work.

Anyway my fix makes it work. I think the patch is okay.

adaddinsane,

Note that _mymodule_process() is in a separate file so that huge function doesn't get loaded each time when not necessary. If you have no [toc] tag anywhere, then you saved loading that file (which under D6 was about 42Kb.)

That's called optimization and if all the modules were to do that Drupal would be faster.

Although that supposes that you check the $text buffer with a fast strpos() function call for a '[toc' string. If present, then you include + execute the process function. If the strpos() does not find the '[toc' you skip on the include! (which means your patch works but it removes all the optimization work I have done in D6.)

Thank you.
Alexis Wilke

Alexis,

You seem to be taking this personally. All I see is a module that I want that's not functioning (and I do have some clue when it comes to coding, plus note the list of comments above) and needs fixing. So I fixed it.

I perfectly understand the concept of putting functions in other files. That was the whole point - the additional file was not getting loaded at the right time therefore the code was not running (the Filter module code checks for the existence of the function it's calling before executing, it doesn't find the TOC ones because the file's not loaded and doesn't).

If you actually look at my patch you'll see that all I did (more or less) was add hooks into the .module file to load the .pages.inc file and then run the required functions. I also cleaned up the naming of the filter_tips function so it actually got called as well (which it wasn't being).

As it happens TOC still has bugs. But at least it generates a TOC now which it wasn't before.

May i interrupt :)
@adaddinsane, thank you for you patch. I think AlexisWilke only wanted to give you some input of what needs to be optimized in that patch.

Of yours patches need to be discussed and optimized. If no one would do that all modules would end up as "gutter tape fixed with hack around this and that". And that would help no one.

Calling module_load_include() only when actually needing to run the appropriate HOOK_filter_prepare, HOOK_filter_process is an appropriate optimization measure, and looks like it was done correctly here. Especially as the problem in the ported module so far appears to be somewhat related to the loading.

Loading the inc file only when (probably) needed will take care of basic optimization for a large number of normal page loads.
Right now, the improvement is from "doesn't work" to "works", so that's great.
Extending this approach to *also* have a look-ahead for the keystring as Alexis describes could be a further possible performance improvement if neccessary. Premature optimization tweaks should not be a blocker on a working result however.

Just FYI, I've found that in D7 (very different to D6), a table of the available hook functions are cached (once) - it's much harder to lazy-load hooks when they are required. This has caused some problems with D7 ports. As long as the actual hooks are defined in the main module file it should be fine, but take care not to dynamically load any inc file that contains your other hooks.
What this means is that a few reasonable 'optimization' methods developed for D6 modules may not work the same in D7.

Look, guys, I am not talking out of my arse, I am a professional Drupal developer - it's my day job, I get paid for it. I've been doing it for maybe five years (I've been coding for 40 years), I've worked on and created major Drupal sites (UK govt, NBC, Comedy Central and many others) in D5, D6 and D7.

I'm going to apologise because I know I can be prickly: Sorry, Alexis, for being prickly.

You mention your strpos() optimisation well, there is only 1 strpos in the TOC.module file, and it has nothing to do with loading files. Maybe you meant to commit that code but didn't? For whatever reason, it isn't there.

@e-anima I don't do "gutter tape" fixing.

(EDIT: typo)

adaddinsane,

Hmmm...

There is one strpos() in the D6 module which checks the TOC class.

The test of "[toc" is indeed in the .pages.inc itself. I guess that's because you still need/want to check the headers (h1 to h6) even if there isn't a [toc] because the table of contents could automatically be added if you have more than a certain number of headers.

tableofcontents/tableofcontents.admin.inc:  if (strpos($node->body, '<!--break-->') !== FALSE) {
tableofcontents/tableofcontents.module:        if (strpos($node->content['body']['#value'], 'class="toc"') === FALSE) {
tableofcontents/tableofcontents.pages.inc:  if (empty($_tableofcontents_block_toc) && strpos($text, "[toc") === FALSE) {
tableofcontents/tableofcontents.pages.inc:    if (strpos($text, "[vtoc") !== FALSE) {
tableofcontents/tableofcontents.pages.inc:        $url = strpos($file->fid, 'upload') === FALSE ? $file->filepath : file_create_filename($file->filename, file_create_path());

Anyway, we have a similar problem in the spam.module which requires some other files to be loaded even before the hook_init() function is called! So I understand what you are talking about.

As for remaining bugs, the worst one right now is the double numbering on headers. This can happen... I still have to look closer why the second pass doesn't detect that a first pass went through!

Thank you.
Alexis

Okay the 22a patch handles a bug in the tableofcontents.pages.inc file (variable name conflict); and the 22b patch handles a function naming problem which I introduced on the settings submission.

These patches would have to be applied in addition to the one at #15. Turns out I committed the first change before adding the second which means I had to do two patches. I'll get the workflow straight eventually. (Only just started submitting Git patches.)

Might be the same issue as #21 but I turned on auto-numbering and it died spectacularly. To be honest while this module has a lot to commend it (it exists!) I think it needs a radical rethink and rewrite, at least for D7.

Dammit forgot to attach the files - see next comment...

The patches...

I've just uploaded a completely hacked about D7 version here. It's not official, untested by anyone but me, and does not do any kind of upgrade from previous versions. However it works and is appropriate for D7. It uses the same core code as the standard module.

If the maintainers want to include it as a new version that works for me. If not, I don't mind either.

Thanks to everyone for the TOC mod.

I couldn't get the official current D7 TOC to work for me. This new version from #24 does work for me so far. So thanks for working on that.

subscribe

Use the "Follow" button at the top of the issue :-)

(Sorry guys, but I'm tied up with a new contract at the moment - and also producing a steampunk web series: contributing there will make me very enthusiastic about working on this :-) http://igg.me/p/93585?a=405734

We now return you to your normal schedule.