Item8234: DirectedGraphPlugin must never skip commonTagsHandler

The following "optimisation", which was added to the commonTagsHandler in one of the recent versions of the DirectedGraphPlugin, ought to be removed.

    if ( ( $_[1] ne $topic ) || ( $_[2] ne $web ) ) {
        &_writeDebug(
" SKIPPING commonTagsHandler  web = |$web|  topic = |$topic|  $_[2] $_[1] "
        );
        return;
    }

The code above prevents extensions like the GenPDFAddOn, which may process topics recursively, from properly rendering additional topics.

-- AndreasVoegele - 07 Aug 2009

I'll remove that test - Have you been running with this change in your local copy?

-- GeorgeClark - 14 Aug 2009

The more I ponder this, I recall an issue where the <dot> code would be expanded during the %INCLUDE process and it was premature - and attachments could end up in the wrong location. If I recall, the commonTagsHandler parses the topic multiple times, and by limiting it to the parent "including" topic, everything is still expanded, and this avoids some issues.

Could you possibly provide a simple example of including and included topics that doesn't expand correctly in with genpdf?

-- GeorgeClark - 14 Aug 2009

If the macro GENPDFADDON_RECURSIVE is set to "on" in a topic, the GenPDFAddOn extension adds all sub topics to the PDF document. When the sub topics are rendered the condition $_[1] ne $topic becomes true, since $topic holds the base topic's name while $_[1] holds the sub topic's name. According to Foswiki.pm $_[3] is true for included files. See SpreadSheetPlugin.pm for an example.

-- AndreasVoegele - 25 Aug 2009

Thanks Andreas, I'll have to work thought this a bit more when I have some time. I maintain both the GenPDFAddOn and DirectedGraphPlugin, so hopefully I can work it out. I think that one issue might be more how GenPDF does recursive rendering. I believe in this case each topic rendered recursively should be rendered in it's own context and not be aware of the parent rendering. It looks like at a minimum, the GenPDFAddOn should probably call pushTopicContext and popTopicContext surrounding each topic that it processes recursively. But this isn't enough because it does not re-init the plugins, so global variables don't get reset.

But as you point out, the 3rd parameter =true for included topics, so that should be used instead of the web/topic check in DirectedGraphPlugin. I had forgotten about that parameter. I had even checked in a comment change to the EmptyPlugin.pm, documenting it's use.

Commenting is disabled while not logged in