Item12196: SetVariablePlugin should pick up its macros from the view template if there is no %TEXT%

Currently, SetVariablePlugin behaves in a way that somewhat confusingly differs from the way pages are rendered.

Normally, a view template will include a %TEXT% template token to include the topic text in the output. Certain topics, though, especially very form-driven ones, might not have any useful topic text and thus a view template might opt to not include a %TEXT% token. In that case, the view template will simply be rendered without the topic text.

In SetVariablePlugin, its macros such as %SETVAR% are processed while saving a topic. To allow for more interesting applications, it will process these within the view template, too... but if the view template doesn't contain a %TEXT% token, instead it will completely ignore the view template and only look at the macros in the topic text.

This is somewhat unexpected since it differs from the behaviour during viewing: if we assume that we're not rendering the topic text because there is none, it doesn't make too much sense to have the plugin look at macros in there.

It might make more sense to

• look at macros in the view template only, or
• look at the view template and the topic text separately.

Here's a patch for the first variant:

diff --git a/lib/Foswiki/Plugins/SetVariablePlugin/Core.pm b/lib/Foswiki/Plugins/SetVariablePlugin/Core.pm
index f185274..0298917 100644
--- a/lib/Foswiki/Plugins/SetVariablePlugin/Core.pm
+++ b/lib/Foswiki/Plugins/SetVariablePlugin/Core.pm
@@ -280,8 +280,8 @@ sub handleBeforeSave {
 
   if ($tmpl && $tmpl =~ /\%TEXT%/) {
     $tmpl =~ s/\%TEXT%/$text/g;
-    $text = $tmpl;
   }
+  $text = $tmpl;
 
   # Disable most macros in the text... we only care about those that probably bring in a [GET,SET,UNSET,DEL]VAR
   # TODO: can we perform all INCLUDEs and DBCALLs only before disabling everything else?

And here's one for the second variant:

diff --git a/lib/Foswiki/Plugins/SetVariablePlugin/Core.pm b/lib/Foswiki/Plugins/SetVariablePlugin/Core.pm
index f185274..27e81ab 100644
--- a/lib/Foswiki/Plugins/SetVariablePlugin/Core.pm
+++ b/lib/Foswiki/Plugins/SetVariablePlugin/Core.pm
@@ -281,6 +281,8 @@ sub handleBeforeSave {
   if ($tmpl && $tmpl =~ /\%TEXT%/) {
     $tmpl =~ s/\%TEXT%/$text/g;
     $text = $tmpl;
+  } else {
+    $text .= $tmpl;
   }
 
   # Disable most macros in the text... we only care about those that probably bring in a [GET,SET,UNSET,DEL]VAR

-- JanKrueger - 26 Oct 2012

I see what you mean. Alas, both patch variations don't cut it as they both significantly change semantics.

The first one will nuke any %SETVAR in topic texts. We need them from both: the topics text and from the template.

The second one will potentially induce a different ordering among %SETVAR macros. The order of the SETVARs is crutial to the order the rules are collected and finally applied during save. The topic's text is basically assumed to be somewhere in the middle of the view template with part of the view template being expanded before and after. Moving all of the topic text to the start alters semantics heavily in that respect.

-- MichaelDaum - 26 Oct 2012

After some thinking it over: patch nr1 is just fine. Let's see later what breaks .

-- MichaelDaum - 26 Oct 2012

Commenting is disabled while not logged in