Feature Proposal:

You can mix single and double quotes in a macro. For example,

%IF{"defined search" then='<a href="#" style="text-decoration:none;" onclick="location.hash=\'foswikiSearchForm\'; return false;">%ICON{down}% <span class="patternLinkLabel">%MAKETEXT{Modify search}%</span></a>'}%

No amount of escaping can get you away from the fact that some macros expand to HTML that includes quotes, which then SNAFU any macros they are embedded in. The inside-out evaluation ordering ensures that this problem will always exist so long as "quote generators" are permitted inside parameters.

One solution to this is to support "outside in" expansion order for selected macros, so that the inner macro doesn't get expanded until the containing macro has been evaluated. However this has been discussed in the past and rejected as it creates enormous complexity in the macro parser.

Another approach is to support "late expansion" for selected macros. There are three obvious approaches to this:

1. enable it for selected macros, e.g. %ICON
2. enable it for all core macros, through macro parameters e.g. lateexpand="on"
3. put macros under the control of some container macro - for example, %LATEEXPAND{ .... }% - that hoists rendering of those macros until later in the rendering process

Approach 1 has the attraction that it is a "no brainer" for users - it requires no additional syntax, and should just work for 90% of cases where such macros are used. It is also trivial to implement. It has the disadvantage that it makes these macros behave differently to other macros.

Approach 2 has the attraction that it requires explicit intervention by the user to make a macro "late expand" so they have full control. It has the disadvantage that it is quite unclear where the rendering is being postponed to - does it postpone just for the first containing macro? All the way to the outermost context? Or to some controlled level of expansion outside the current level e.g. lateexpand="2"? It is also very difficult to implement reliably.

Approach 3 has all the problems of approach 2, and also adds some lardy syntax.

On the assumption that "do nothing" is not a good approach, the rest of this proposal describes a simple solution to Approach 1.

NOTE: this is not intended as a hack to support a small number of troublesome wiki applications, but as a general approach to the inside-out-left-right rendering order problem.

Description and Documentation

The idea is to "cache" the rendered output of certain levels in the macro hierarchy and replace their rendered version with a "key" that can be picked up and substituted later. This is directly analagous to the takeOutBlocks approach used to support <literal> blocks.

In the patch below I elected to use a "custom HTML entity" for the key, though this could in fact be any string that will survive the rest of the rendering process.

Any macro can, rather than returning rendered HTML, return the result of a call to takeOutFragment. This will have the same effect as a literal tag for that fragment of HTML.

Because of the expansion ordering problems inherent in any "take out" strategy, this needs to be applied with care. As such I don't currently favour exposing this mechanism to plugins.

Implementation

Only implements late rendering of ICON. But it shows how (relatively) easy this is to do.

Index: lib/Foswiki/Render/IconImage.pm
===================================================================
--- lib/Foswiki/Render/IconImage.pm   (revision 16625)
+++ lib/Foswiki/Render/IconImage.pm   (working copy)
@@ -36,7 +36,7 @@
     $html =~ s/%HEIGHT%/16/g;
     $html =~ s/%ALT%/$alt/ge;
 
-    return $html;
+    return $session->takeOutFragment( $html );
 }
 
 1;
Index: lib/Foswiki.pm
===================================================================
--- lib/Foswiki.pm   (revision 16626)
+++ lib/Foswiki.pm   (working copy)
@@ -79,11 +79,15 @@
 # by Foswiki/Render.pm
 our $RENDERZONE_MARKER = "\3";
 
-# Used by takeOut/putBack blocks
+# Used by takeOutBlocks/putBackBlocks/takeOutFragment
 our $BLOCKID = 0;
 our $OC      = "<!--\0";
 our $CC      = "\0-->";
 
+# Hash used in _processMacros to cache fully rendered HTML for
+# substitution back into output at the end of rendering.
+our %fragmentCache;
+
 # This variable is set if Foswiki is running in unit test mode.
 # It is provided so that modules can detect unit test mode to avoid
 # corrupting data spaces.
@@ -3012,6 +3016,7 @@
 # than that.
 # $depth limits the number of recursive expansion steps that
 # can be performed on expanded tags.
+
 sub _processMacros {
     my ( $this, $text, $tagf, $topicObject, $depth ) = @_;
     my $tell = 0;
@@ -3032,6 +3037,8 @@
         return $text;
     }
 
+    local %fragmentCache;
+
     my $verbatim = {};
     $text = takeOutBlocks( $text, 'verbatim', $verbatim );
 
@@ -3153,6 +3160,11 @@
         $stackTop .= $expr;
     }
 
+    # Expand the fragment cache
+    $stackTop =~ s/&later(\d+);/$this->_putBackFragment($1)/ge;
+    # Make sure all fragments were expanded
+    ASSERT(scalar(keys %fragmentCache) == 0) if DEBUG;
+
     putBackBlocks( \$stackTop, $dirtyAreas, 'dirtyarea' )
       if $Foswiki::cfg{Cache}{Enabled};
     putBackBlocks( \$stackTop, $verbatim, 'verbatim' );
@@ -3162,6 +3174,35 @@
     return $stackTop;
 }
 
+=begin TML
+
+---++ ObjectMethod takeOutFragment($html) -> $key
+
+Used by macros which fully expand all the macros in their definition,
+and want to record the output generated using an entity key. These
+keys will be expanded to the saved HTML at the end of rendering.
+
+=cut
+
+sub takeOutFragment {
+    my ( $this, $html ) = @_;
+
+    ASSERT(defined $html) if DEBUG;
+    my $index = $BLOCKID++;
+    $fragmentCache{$index} = $html;
+    return "&later$index;";
+}
+
+sub _putBackFragment {
+    my ( $this, $index ) = @_;
+
+    my $html = $fragmentCache{$index};
+    ASSERT(defined $html) if DEBUG;
+    delete($fragmentCache{$index}) if DEBUG;
+
+    return $html;
+}
+
 # Handle expansion of a tag during topic rendering
 # $tag is the tag name
 # $args is the bit in the {} (if there are any)

-- Contributors: CrawfordCurrie - 26 Mar 2013

Discussion

How about an alternate macro syntax. %LATEEXPAND and lateexpand= are both a bit wordy. Using a %~ start delimiter to delay evaluation of any macro might work, for example %~ICON% ( %~anymacro% ) to delay evaluation of the macro. Your example above would become:

 <span class="patternLinkLabel">Modify search</span></a>'}%

vs.

%IF{"defined search" then='<a href="#" style="text-decoration:none;" onclick="location.hash=\'foswikiSearchForm\'; return false;">%LATEEXPAND{}% <span class="patternLinkLabel">Modify search</span></a>'}%

and

-- GeorgeClark - 26 Mar 2013

Yes, I thought of that but decided not to raise it as it complicates the parser quite a bit, and not all macros will support late expand. I found myself thinking "when would you ever not want to late-expand these specific macros?".

-- CrawfordCurrie - 27 Mar 2013

Do nothing is an option. Let me explain.

First, let me just put into perspective where this proposal comes from:

1. single quotes were added to the attribute parser
2. %ICON has been converted to generate HTML using single quoted attributes
3. on Foswiki trunk the WikiGroup applications broke due to the quote changes in %ICON

Point (1) I only recently became aware that you can write %MACRO{foo='bar'}%, that is using SINGLE QUOTES for Foswiki macros. The arising confusion for wiki apps is obvious now, last but not least due to the followup proposal that we have here.

Point (2) obviously generated backwards compatibility problems but we decided to take the risk. We loosely agreed on a guideline to use single quotes in HTML generated by macros to prevent clashing with Foswiki macros using double quotes in general.

Point (3): The new wiki apps in Foswiki trunk that broke now fail because of the use of single quotes in Foswiki macros and %ICON now generating single quoted HTML. These wiki apps in Foswiki trunk need fixing following the best practices further down. Nothing more needs to be done about it. It is by far not the case that these wiki apps are impossible to fix and we thus inevitably desperately require to extend the parsing process. Nor would it be aesthetically unpleasant to fix them.

On this base I am raising concerns about this proposal already.

But there's another problem in this proposal related to writing wiki apps: the same macro needs different evaluation order in different contexts. One wiki app would look better writing it with outside in evaluation in mind; On another occasion the very same macro requires to be evaluated as always. However the decision how a macro expands is configured globally thus affecting all macros equally all over the site. I am not sure which level of configurability this proposal was aiming at but you might even end up with incompatible wiki apps between Foswiki installations where the same macro expands differently on them.

Another point: incompatibility with TWiki.

All I can say is, play the game as we know it following these four simple rules:

1. internalize the inside-out-left-to-right evaluation order of the Foswiki markup language
2. always use double quotes for Foswiki macro attributes; ignore the fact that you could use single quotes as well
3. use single quotes for HTML attributes
4. use escaped double quotes in nested Foswiki macros

This will make your wiki apps most compatible, reusable and maintainable by others.

-- MichaelDaum - 27 Mar 2013

I am with MichaelDaum on this one. I have the feeling this is shouting for trouble down the road. As ugly as it is, we arranged our macros to cope with %ICON so far, as it is possible to produce valid output.

A much bigger problem in regards to the whole mix of double and single quote is the fact, that foswiki is not capable of producing reliably JSON compatible output. See EncodeMacroOutputForJavaScript. Without getting this additional problem properly addressed, I would keep my hands of the whole macro expansion logic.

-- AndreLichtsteiner - 27 Mar 2013

For now, i can't be bothered arguing any more. This is an old problem that has been rearing its ugly head regularly for many years. If you view Foswiki as being for users first and developers second, then the "do nothing" solution is fine. If you view it as for users and developers - and to me one of the great attractions was Foswiki's ability to turn the average numpty into a developer - then I believe it ought to be fixed.

Oh well, it's on the table. We will probably revisit it again in the future, at which time this patch will be available.

-- CrawfordCurrie - 28 Mar 2013

It is for developers too. In fact calling it a wiki is an unfortunate understatement. However your proposal is not a good one as far as I can see, no matter how technically matured your patch is.

-- MichaelDaum - 28 Mar 2013

Commenting is disabled while not logged in