Support for [ and ] and a few other bug/problems fixes.

Hi deviantintegral,

I like this module! Really cool!

Now, I have a long one here, most of which is fixed in the attached patch. 8-)

1. WYSIWYG problem

Solution

Now, I have one major complaint... I use a WYSIWYG editor and writing a comment is a real pain and it can be real slow too (because I need to switch between WYSIWIG and Source). But my main concern would be to explain to my customer (really totally end users) to write an HTML comment. Just forget it!

So I would really appreciate if you would apply my patch to support both: the comment and [tableofcontents...]. That would really rock and I'd bet a lot more people would start using your module like crazy (note that I did not invent the syntax, all the other filters I've seen so far make use of the square brackets.)

Another personal note: it is a lot easier to write [ and ] rather than <!-- and -->... 8-) [got to find good arguments, right?]

Potential Problems

Patch wise, the functionality is complete, but the help was not changed. If you accept this, then I will be happy to fix the help too. 8-)

Also, because I use my WYSIWYG editor:

1. The table finds itself inside a paragraph (<p>). I'm not too sure, but I think lists cannot be inside paragraphs.
2. By default, the Teaser is the [tableofcontents ...] tag and nothing else.

Again, I will be glad to look into fixing these problems if you accept the use of that syntax.

2. Search Pattern

Invalid use of period

Your search pattern uses (.*) within the comment. The problem is if you write two comments one after another, you will pickup both. Of course, an easy fix is to add a newline. But imagine two filters used back to back and the person does not want a newline...

The period (.) should be something like [^>] instead (anything but a closing angle bracket).

Invalid use of question mark (partial patch)

This one, I think you should also fix: the comment starts with <\!-- ?tableofcontents... I think that the question mark ought to be an asterisk. That way if someone puts two spaces it still works.

Similarly, the option name and value are separated with nothing or one single space. (i.e. ': ?' -> ': *').

Missing ';' at the end breaks everything

It would be neat to add a ';' if missing at the end of the string.

3. min/max problems

The maxlevel improper use (no patch)

There is a case that you do not handle properly. I have a partial fix for it, but it could be a lot better.

Say I write this: <!--tableofcontents minlevel:4>

(with my fix for the ending semi-colon, it works 9-) )

Now you have the default maxlevel set to 3 and the minlevel set to 4. You then have a test that says: if max is smaller than min, blame the user (he! he!). I think that if the user sets minlevel to 4, then maxlevel should be set to at least 4.

To do that, if you consider this a bug, you should set maxlevel to something like -1. If it is still -1 after parsing all the user options, get the default from the variable and if smaller than minlevel, set it to minlevel.

I'll provide a patch if you want.

The maxlevel invalid reset

The next problem with the maxlevel is that you set it to 3 if it is smaller than minlevel.

And if I just put minlevel to 4, setting maxlevel to 3 is bad. 8-)

Also the error message is wrong, since it mentions 3.

Error messages

Small mistake, the maximum may have been bumped by 1 at some point. The maximum minlevel should be 5 (although, 6 should work too, but that's okay) and the maximum maxlevel should be 6. At this time, these are 4 and 5 respectively.

4. Optimization

Outside of the fact that the theme function could be in a .admin.inc file, I think that the css & js can be added only when necessary. I would think that I'm affected by the cache and I did not see any difference between your way of putting the loads in the _init() function and the case 'process': just and only when a table is generated. This would be a lot better since 99% of my pages do not have a table of content and creating special Input Formats as an optimization is not very effective (especially for end users who have no clue.)

This appears in my patch, but please, make sure it works in all cases! I'm not 100% sure that my cache is being used.

Okay! That's it for this time. 8-)

Thank you for your time and patience with that one!
Alexis Wilke