BUG: toc macro recursion error

[00SS]

On tiddlywiki.com - Open tiddler: Contents and add the tag: tags: First

Make a: New Tiddler text:

<div class="tc-table-of-contents">
<<toc "Contents">>
</div>

Save: New Tiddler

Javascript Error: too much recursion

I had opened a pull request some months ago Fix recursion error in toc() macro in: toc.tid #3668 and recently closed it as very strangely I could not reproduce that recursion error of having given a non-existent tag.

I found this present error because I was trying to figure out how the toc macros call themselves recursively without going into an endless loop. I studied the code to the best of my ability and could not find any way this was being accomplished. The exclude and excluded variables appear to ALWAYS BE EMPTY - never actually filled up with any results of a <$list> filter.

I think ALL the macros that make table of Contents do not stop recursive calling.

The RECURSIVE ERROR does not show up simply because all the other macros have $reveal areas.

Following the same above, this time in the : New Tiddler text:

<div class="tc-table-of-contents">
<<toc-expandable "Contents">>
</div>

You can expand the link First and Contents for ever! The same can be seen with ALL the toc macros.

@pmario Please also add this to the top of Meta: Issues with Table of Contents Macros #3627

[Jermolene]

Thanks @00SS this is indeed a regression, and an embarrassing one!

Fix is upcoming, but what I realised while working through it is that arguably we don't actually need the recursion protection for the selectable TOCs (i.e. where the user has to click to disclose child entries). In those cases, there's no possibility of a JS recursion error. Cutting out the recursion detection makes the macros significantly simpler and therefore should improve performance.

[00SS]

@Jermolene There has been some discussion in Google Groups centered around using the exclude parameter to "exclude" certain tags from being included in a Table of Contents.

I imagine some resistance to removing this parameter.

[00SS]

I noticed the Commit today of: Restore exclude parameter for selective TOC macros

Please bear with me, as the toc macros are quite confusing for me to unravel. I may be way off base, but I will still try and give an opinion.

In the tiddler: $:/core/macros/toc In the macro: toc-body

In TW ver 5.13 there was one extra $list widget wrapping the recursive call:

<$list filter="""[all[current]] -[[$rootTag$]]""">
<$macrocall $name="toc-body" rootTag="""$rootTag$""" tag=<<currentTiddler>> sort="""$sort$""" itemClassFilter="""$itemClassFilter$"""/>
</$list>

From TW ver 5.14 onwards, this was removed so it is simply:

<$macrocall $name="toc-body" tag=<<item>> sort="""$sort$""" itemClassFilter="""$itemClassFilter$""" exclude=<<excluded>> path=<<path>>/>

Instead a variable exclude was set: excluded="""$exclude$ -[[$tag$]]""" and used in the filter as : $excluded$

In ver. 5.19 it is:

<$macrocall $name="toc-body" tag=<<item>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> exclude=<<excluded>> path=<<path>>/>

Now the variable set is: excluded="""[enlist<__exclude__>] -[<__tag__>]""" but in the filter is is used as: -[<__tag__>] -[enlist<__exclude__>]

I suspect in the filter it might just need to be: <__excluded__>

[00SS]

On further looking at the macro, what I wrote above is not correct, because in the recursive $macrocall, exclude=<<excluded>>

Maybe the place to examine is that back in v5.13, the recursive $macrocall was wrapped in the $list widget where the tags were being removed, whereas now it is not.

[00SS]

Well, changing: excluded="""[enlist<__exclude__>] -[<__tag__>]"""> to: excluded={{{ [enlist<__exclude__>] [<__tag__>] }}}> makes sense to me and fixes the recursion in THIS macro.

[00SS]

@Jermolene @pmario Please disregard the above.

Instead if you can take the code below and paste it over the toc and toc-body macros & repeat the exercise above. I believe it has the method of properly adding the correct tag tiddler title names into the exclude parameter on each recursion, and shows it happening on this example.

If you can also follow this by adding a tag title name to the exclude parameter when defining the Table of Contents, you can see that in action too, for example:

<div class="tc-table-of-contents">
<<toc "Contents" exclude:"Fourth">>
</div>

\define toc-body(tag,sort:"",itemClassFilter,exclude,path)
exclude1=<<__exclude__>>
<ol class="tc-toc">
  <$list filter="""[all[shadows+tiddlers]tag<__tag__>!has[draft.of]$sort$] -[<__tag__>] -[enlist<__exclude__>]""">
    <$vars item=<<currentTiddler>> path={{{ [<__path__>addsuffix[/]addsuffix<__tag__>] }}} >
     <$set name=excluded filter="[enlist<__exclude__>] [<item>]">
      <$set name="toc-item-class" filter=<<__itemClassFilter__>> emptyValue="toc-item" value="toc-item-selected">
        <li class=<<toc-item-class>>>
          <$list filter="[all[current]toc-link[no]]" emptyMessage="<$link><$view field='caption'><$view field='title'/></$view></$link>">
            <<toc-caption>>
          </$list>
exclude2=$exclude$
          <$macrocall $name="toc-body" tag=<<item>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> exclude=<<excluded>> path=<<path>>/>
        </li>
      </$set>
     </$set>
    </$vars>
  </$list>
</ol>
\end

\define toc(tag,sort:"",itemClassFilter:" ",exclude:"")
<$set name=excluded filter="[enlist<__exclude__>] -[<__tag__>]">
exclude0=$exclude$ excluded0=<<excluded>>
<$macrocall $name="toc-body"  tag=<<__tag__>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> exclude=<<excluded>> />
</$set>
\end

[Jermolene]

Hi @00SS are you saying that the changes in 6e81122fa3ac0b74764b86347f84e2805f0ece34 don't fix things? It seems to work for me; if I visit the prerelease and add the tag "Discover TiddlyWiki" to HelloThere then I can't recursively open "Discover TiddlyWiki" in the sidebar.

[00SS]

@Jermolene

Sorry for nit-picking, but it appears that the Table of Content macros are used extensively, specially so by new users, and I believe they should not misbehave in any way at all. I am concentrating now on the easiest of them all. I believe once I nail down the exact logic and behaviour expexted of them, I can later look at the others as well.

The latest version you have provided does stop the recursion, but the macro is not doing everything it is meant to be doing.

• It is ignoring the circular tag, instead of listing it once and then not again.
  • I believe it is not for the program to disallow circular tags, only to disallow endless recursions.
• Its behaviour is different from the other toc macros in that it does not allow an initial exclude value the way the others do.
  • Also, on further scrutiny, I think in the emptyMessage: <$link><<toc-caption>></$link> should be used instead of <$link><$view field='caption'><$view field='title'/></$view></$link>

So my revised suggestion (now I have removed the markers showing parameter values at different stages) is:

\define toc-body(tag,sort:"",itemClassFilter,exclude,path)
<ol class="tc-toc">
  <$list filter="""[all[shadows+tiddlers]tag<__tag__>!has[draft.of]$sort$] -[<__tag__>] -[enlist<__exclude__>]""">
    <$vars item=<<currentTiddler>> path={{{ [<__path__>addsuffix[/]addsuffix<__tag__>] }}} >
     <$set name=excluded filter="[enlist<__exclude__>] [<item>]">
      <$set name="toc-item-class" filter=<<__itemClassFilter__>> emptyValue="toc-item" value="toc-item-selected">
        <li class=<<toc-item-class>>>
          <$list filter="[all[current]toc-link[no]]" emptyMessage="<$link><<toc-caption>></$link>">
            <<toc-caption>>
          </$list>
          <$macrocall $name="toc-body" tag=<<item>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> exclude=<<excluded>> path=<<path>>/>
        </li>
      </$set>
     </$set>
    </$vars>
  </$list>
</ol>
\end

\define toc(tag,sort:"",itemClassFilter:" ",exclude:"")
   <$set name=excluded filter="[enlist<__exclude__>] -[<__tag__>]">
      <$macrocall $name="toc-body"  tag=<<__tag__>> sort=<<__sort__>> itemClassFilter=<<__itemClassFilter__>> exclude=<<excluded>> />
   </$set>
\end

[pmario]

The exclude parameter as it is at the moment, isn't meant to be exposed to users. ... Some users found out about it and started to "misuse" it, which leads to confusions.

I'm pretty sure the [enlist<__exclude__>] is a result of this confusion. ... It hasn't been meant that way. ....

[pmario]

• It is ignoring the circular tag, instead of listing it once and then not again.
  • I believe it is not for the program to disallow circular tags, only to disallow endless recursions.

That's right. exclude is only meant to remove endless recursions. ... Tiddlers "must be" shown in several branches, if they are tagged that way. If users want tiddlers to be in one branch only, they need to fix the problem. ... Remove the wrong tag, or use my tocP macros

[pmario]

I just had an idea about variable naming. We could expose the "exclude" parameter for users and the internal var could be named "block-list" or "block" or something similar.

[00SS]

The path variable is used in the other toc macros in the title attribute of the <$qualify> widget. It does nothing in this macro and needs to be removed.

The main differences in logic is (I believe) the version I am proposing

• adds the <<currentTiddler>> (via <<item>>) to the excluded variable instead of the slightly earlier in the tree structure <<__tag__>> being added presently
  • This allows the first extra layer of recursion, but not subsequent ones.
• The invoked macro toc will include the exclude parameter with a safety check to remove the <<__tag__>> value from its list
• The link will properly display wikitext from a caption

[pmario]

The path variable is not really active at the moment, but it's needed if we want to implement the "auto-expand" feature. .... Which is a problem with the existing tagging structure.

[00SS]

The toc macro is expanded by default. It has no $reveal widgets, so the path variable is redundant in this particular macro.

I looks like the one exclude parameter (with its accompanying excluded variable) can be both used:

• by the user to exclude specific branches
• for the macro to keep recursion in check.

If there is a reason to separate this function, I haven't understood it yet.

Once we can finalize the way the tree lays out in this simple macro, how much recursion to allow (one level in my suggested code), and ensuring endless recursion cannot take place, then the same logic can be, hopefully safely, applied to all the other Table of Content macros.

[pmario]

I'm completely refactoring the TOC macros atm. The code should be simpler, but the functionality is better then ever. At the moment I'm focusing on functionality. Perfermance will be a second step. (It should be slightly better already. ... but I didn't do measurements.)

@00SS Your discoveries here where very helpful.

The recursion problem is fixed. The "new" exclude parameter is implemented for simple toc macro. ...

I think I "only" need to add it to all the other possibilities.

[pmario]

tracking is still done at: https://github.com/Jermolene/TiddlyWiki5/issues/3627