Fixes #15

[veselov]

If I prevent @@full_outline from being reset in the middle of rendering, the ToC problem is fixed. However, the tests no longer work because the class variable is no longer changed between classes.

I tried to making it an instance variable, but that causes an NPE at line 396.

Things I don't understand:

• Why is there more than instance of the converter instantiated?
• I assume we can theoretically remove_class_variable @@full_outline, but at which point? May be when the conversion is done? Since there are apparently multiple converter instances, I don't quite understand when are we "done"

I modded the test code to make it easier to run a single test, or run HTML update by setting an env var, was quite handy.

[veselov]

OK, I think I fixed this now. (I squashed the commits, otherwise the interim changes to HTML files are too distracting).

A trivial fix would have been to just check, before (re)setting @@full_outline whether the document is nested?. Documents created from "a" table tags will have nested? true and parent_document set.

However, I really didn't like the idea of having a class-wide variable. If anybody attempted to convert things in parallel, AFAICU things would go terribly wrong. AsciiDoctor creates a new converter instance for each new document object that MultiPage creates, so AFAIU, this was the reason why a class variable was used in the first place - to maintain the state across multiple converter instances.

I'm not sure there isn't a better shortcut for fixing this, considering I neither well understand Ruby nor AsciiDoctor, but the idea should generally be the same - MultiPage needs a shared state to get its bearings around the structure, so instead of maintaining it in a class variable - let's have a "top-level" converter that can be discovered from any flying around document, so we can use that for stuff that needs to be shared throughout the entire generation process.

[owenh000]

Why is there more than instance of the converter instantiated?

It looks like you found the answer to this already. My understanding is that the convert_document() method is normally only executed once but in this case it is executed again each time a nested document is encountered in the document tree. Is that what you're seeing?

[veselov]

My understanding is that the convert_document() method is normally only executed once but in this case it is executed again each time a nested document is encountered in the document tree

Actually not. A new instance of MultipageHtml5Converter is created every time a new instance of Asciidoctor::Document is created in convert_section(), then that instance is asked to convert the sub-document (page).

In any case, this is fixed now by having @@full_outline made into an instance variable.

[owenh000]

A new instance of MultipageHtml5Converter is created every time a new instance of Asciidoctor::Document is created in convert_section(), then that instance is asked to convert the sub-document (page).

Okay, thanks. Even as the author I find the code path difficult to follow.

[owenh000]

This looks great, @veselov. Three things:

• Could you add some comments to the new code areas so we have a few more hints about what's going on?
• Can you split the test code change into a separate commit?
• This will still break if someone adds a section inside a table cell. (That it's possible is a bit absurd.) I suggest we merge these changes first. Further improvements could be done if anyone needs it.

[veselov]

@owenh000 OK, this PR now only has the issue fix and comments. Thank you! Test code changes went into an individual PR Agree on your 3rd point.

[owenh000]

Merged and released in v0.0.11. Thanks @veselov!

I opened issue #18 for the case of section headers inside a table cell, but I'm hoping no one has any reason to put a section header inside a table cell.