Item13909: Support expansion of standard macros during topic creation from template.

pencil
Priority: Enhancement
Current State: Waiting for Release
Released In: 2.2.0
Target Release: minor
Applies To: Engine
Component:
Branches: Item13909 Item13905 master Item14288 Item14380 Item14537
Reported By: JohnRouillard
Waiting For:
Last Change By: CrawfordCurrie
I am using the comment plugin to provide a form to create a new page.

As part of the output template I have a %IF that checks to see if a particular url parameter was passed from the input form. If the param is passed, it adds an embedded %SEARCH% to the page.

The code in the output template for the comment is:
%EDITTABLE{}%
| *See Also*  | %URLPARAM{"SeeAlso"}% %STARTSECTION{type="expandvariables"}% %IF{"defined EmbeddedSearch" then="$percentSEARCH{search=$quotNagSvc*$quot excludetopic=$quot$percentTOPIC$percent$quot regex=$quoton$quot scope=$quottopic$quot format=$quot$topic$quot nosearch=$quoton$quot nototal=$quoton$quot separator=$quot, $quot}$percent" }% %ENDSECTION{type="expandvariables"}%  |

I had to add the SECTION{type="expandvariables"} setting to get the %IF macro to be evaluated. However that causes the search to be evaluated as well. So rather than having an embdded search in the new page, I have the output of the search written to the page.

I have tried using
sea%NOP%rch
but I end up with SEA<nop>RCH in the generated page. I have also tried $nop e.g. $per$nop()centSEAR$nopCH .... But the $nop is removed during the expandvariables and I still end up with the results of the search in the new page.

I have also tried using $dollarpercentSEARCH but that put $percentSEARCH in the created page.

The OTHER wiki has %EOTC__ that I can use so I could use %EOTC__IF{... to get just the IF variable expanded without expanding the variables in the then clause.

I could see this being handled a few ways:
  • provide an optional levels parameter for the expandvariables type that limits how many levels of expansion occur. In the case above, level would be one to expand only the %IF{...} outer level. But given the eval order left to right inside to out, this may be tough to make work.
  • Provide the %EOTC__ prefix mechanism. Requested here and documented here.
  • Provide a mechanism where the exact variables to be expanded are passed into the expandvariables section. E.G. SECTION{type="expandvariables" operateon="IF"} so only IF's are expanded.

Sadly this is a blocker for me as it is the preferred solution to a significant usability problem with the wiki application I am trying to get backing for at work.

Thanks.

From irc chat:
(2015-12-31 17:06:55) gac410: I've not run into the EOTC__ syntax before. You might want to hit
                              up CDot or MichaelDaum after the New Years
(2015-12-31 17:12:56) gac410: You can further delay $percnt by using $dollarpercnt. ...  but tbh
                              by the time I'm considering that I'm so lost, as to not have a clue.
(2015-12-31 17:21:00) gac410: rouilj1:  This appears to be a very simple change to implement that
                              EOTC__ function,  but tbh, I think that needs review  by others before
                              incorporating it.  Michael / CDot might have preferred ways to implement.
(2015-12-31 17:21:40) gac410: If you open a task, Enhancement priority,  I can post a patch for review.

-- JohnRouillard - 01 Jan 2016

The following patch implements the feature. It compiles, but I don't have a test case yet. We need to decide:
  • Do we want to do this
  • Is there another preferred implementation options.

diff --git a/core/lib/Foswiki.pm b/core/lib/Foswiki.pm
index a2a9789..c42638d 100644
--- a/core/lib/Foswiki.pm
+++ b/core/lib/Foswiki.pm
@@ -3538,6 +3538,14 @@ sub _expandMacroOnTopicCreation {
     # brace-matching.
     return '' if $_[0] eq 'NOP' && defined $_[1];
 
+    # You may want to expand arbitrary tags on topic creation.
+    # By prepending EOTC__ (EOTC stands for Expand On Topic Creation), you
+    # can achieve that.
+    if ( $_[0] =~ /^EOTC__(\w+)$/ ) {
+           $_[0] = $1;
+           return $this->_expandTagOnTopicRendering(@_);
+    }
+
     # Only expand a subset of legal tags. Warning: $this->{user} may be
     # overridden during this call, when a new user topic is being created.
     # This is what we want to make sure new user templates are populated

Crawford, Michael ... any comments or objections? Other than being a bit arcane, I don't see any significant issues with this, and it would be nice to retain compatibility when reasonable.

I know we are in feature freeze, but this seems minor enough that we could add it to 2.1.

-- GeorgeClark - 01 Jan 2016

I don't think that this is a good addition to the syntax. Above patch seems not quite right either only allowing a certain kind of TML to be prefixed with it.
%STARTSECTION{type="expandvariables"}%
is the right approach. The fact that code inside a topic template is also executed when visiting it does not seem to motivate such a syntax change sufficiently. At least it does not convince me.

To prevent code in view- and edit templates from being executed use this construct

%{<verbatim>}%

...
%{</verbatim>}%

Use this construct to add something similar to topic templates (not tested but should work).

%STARTSECTION{type="templateonly"}%<verbatim>%<nop>ENDSECTION{type="templateonly"}%
...
%STARTSECTION{type="templateonly"}%</verbatim>%<nop>ENDSECTION{type="templateonly"}%

Btw I would not embed a formatted %SEARCH into each page created. That's an anti-pattern: the wiki text area should be used foremost for net data created by content authors. Application logic of wiki apps - such as formatted searches and the like - belong into view templates applied on top of the net data. This prevents you from creating massive redundancy in all topics created by your approach. Instead you will have exactly one view template for all topics. Any maintenance of this wiki app only has to deal with this one view template instead of having to sift thru hundreds of topics each having the same (probablly buggy) %SEARCH in it each.

-- MichaelDaum - 01 Jan 2016

Michael,

There seems to be a misunderstanding about my issue.

I don't care about the code being executed when viewed. That's not a problem I already have the template in a verbatim block, so viewing it is not an issue. The problem is that the expansion of the code is too aggressive when the template is evaluated. I can't generate valid macro references from the expansion.

Every macro/variable inside an expandvariables block is expanded even if only one (outer macro) layer needs to be expanded.

When the output template code is expanded by the CommentPlugin to create a new topic, I can't create the formatted search. I always get the results of the SEARCH rather than the macro to execute the search.

Each created topic will have different searches. In fact there may be multiple searches or no searches for a given topic.

One thing that may have confused you is that my example uses a hard coded search string. This was to simplify the example while I was trying to make it work. In actual use the SEARCH would have a parameter
search=$quot%<nop>URLPARAM{"EmbeddedSearch"}%$quot
rather than a hard coded value. The EmbeddedSearch parameter is supplied by the user as they fill in the form to create the new page of documentation.

This "application" is a knowledge base where the subject matter experts fill in a form that generates the documentation. One of the items they can fill in is a search to reference existing pages related to the new page they are creating. (There is a naming convention that can be used to search for related pages.) A static list of similar pages will quickly be out of date so the search has to be dynamic.

My issue is that I can't expand the comment's output template so that it will generate an embedded user defined search. If I don't use expandvariables, the %IF macro isn't executed leaving the %IF... in the created topic. Without the IF (just using SEARCH), I end up with an empty search string if the user doesn't define the EmbeddedSearch parameter.

So I need the IF evaluated and the result of the IF should be a SEARCH variable/macro with the user supplied parameters, or nothing if the paremeter isn't defined. So far no amount of %NOP % variables, $nop $dollar $percent etc. escaping lets me generate a valid SEARCH macro.

I am not in love with the mechanism used by the other wiki but I need some way to have only some macros/variables expanded in the template even if the macros are embedded within each other.

If there was a
TMPL:IF{ "defined !EmbeddedSearch" then=...}
option it would solve my use case as I wouldn't need the expandvariables at all since the TMPL:IF would execute without any special markup.

Hmm, arguably that may be a better markup TMPL:XXX where XXX is interpreted as a normal MACRO, but the TMPL: prefix says this should be expanded/run at the template expansion level.

-- JohnRouillard - 01 Jan 2016

If the search arguments (RelatedPages) were entered into a formfield in the topic, you might be able to conditionally expand the search as part of a topic view template.
The related pages are displayed in a form. The value of the cell is a mix of:
  • direct page references (with comments)
  • results from one or more embedded searches
  • Full URL's to documents in sharepoint, mediawiki and the filesystem.
All of them are displayed in a single table cell that is changed using the row edit capability of edittable.

I suppose I could try to figure out how to do this in a formfield and then just reference the field value in the right place using %IF to display nothing if the field wasn't set.

I would prefer to not do that as I see usability issues since you can't edit the field using edit table which is used to edit the other items in the table.

-- JohnRouillard - 02 Jan 2016

Michael, the patch, for better or worse, is a c/p from TWiki.pm. It fits unmodified.

-- GeorgeClark - 02 Jan 2016

I agree with Michael that the EOTC_ syntax stinks. However I think the concept is actually quite good, it just hasn't been thought through.

One of the most annoying problems I have come across is the divide between "skin" templates and "topic" templates. This is a frequent source of confusion for the unwary user, and a source of major confusion in the code. Currently there are three "types" of macro:
  1. Skin template macros / pragmas (TMPL:P, TMPL:INCLUDE, TMPL:DEF..TMPL:END, TMPL:P, TMPL:PREV)
  2. Topic template macros (the subset of normal macros that always expand on topic template instantiation, and the control mechanisms like expandvariables and templateonly)
  3. "Normal" macros.
In several places developers have tried to shoehorn the "normal" macro functionality into "skin" and "topic" template expansion
  • selective macro expansion on topic template instantiation e.g. DATE, WIKINAME
  • the TMPL:P{condition} mechanism (which I deeply regret)
  • the expandvariables section
This has left us with a bit of a mess, but it isn't irredeemable. Let's say we accept that there are two "expansion domains" *. The first domain applies when a template - "skin" or "topic" - is being instantiated. The second applies when a topic is being viewed. If we think of "skin" and "topic" templates being essentially the same thing, we can see that these expansion domains already exist, i.e.
  • TMPL: at the start of a macro puts it in the "template" domain e.g %TMPL:INCLUDE.
  • No TMPL: means the macro is expanded in the topic domain e.g. %INCLUDE.
This is something that has been in the back of my mind for a long time, perhaps it's time has come. There are clearly limitations - a condition using URL params during a "skin" template expansion probably wouldn't be a good idea - but these can be resolved by identifying those macros that can be expanded during skin template expansion, and those that have to be left unexpanded in the output. For example, a macro might fire a NotAllowedDuringTemplateExpansion exception. I can see this leading to greater things:
  • A %P{...}% macro that instantiates a %DEF{...}%
  • Rationalisation of TMPL:P, IF, TMPL:INCLUDE and INCLUDE to eliminate duplication / overlap
  • Template precompilation

If this is agreed as a good end-goal, then starting with a TMPL: syntax for forcing macro expansion during template instantiation is a good first step.

Bottom line, I'd favour:
  • A Development proposal based on the idea of rationalising template syntax as described above, in stages
  • Applying the patch, but using TMPL: instead of EOTC_

* it might be better to call them "phases" instead of domains, and retain three "phases" i.e. skinexpansion, topictemplateexpansion and topicexpansion. The first two would effectively be the same in code, and only exist for documentation purposes.

-- Main.CrawfordCurrie - 02 Jan 2016 - 09:30

There is an issues using TMPL: (or at least EOTC:) that is described in the original TWiki development discussion. The macros have already been parsed out and are being individually considered at that point in the code. So while the original proposal objected to the EOTC__ syntax, it was retained for practical purposes, as the macro needs to match the macro regex, and TMPL: doesn't match as a macro.

For 2.1, we are really in feature freeze, so I don't think we can pick up anything more complex than a "merge for compatibility purposes" type change. This is probably one of those situations where we choose to not retain TWiki compatibility, and I'd rather not rush in something more complex. The other option is to merge their feature "as-is" as an assist for sites converting from TWiki, but omit it from the documentation, or document it as a "deprecated feature" ... to be replaced by a future TMPL: mechanism.

I'll copy the above discussion into a feature proposal for 2.2. Do we have any interested developers to be on the "Committed" list?

-- GeorgeClark - 02 Jan 2016

And as a side note, I've had to replace the % with the &#37; encoded entity in the above discussions - it seems the renderer breaks when used in %{ ... }% even with the embedded <nop>

-- GeorgeClark - 02 Jan 2016

Thanks George - I don't have access to the TWiki site to read their discussion, but what you describe is a minor point that I'm sure can be catered for in the implementation.

I would argue against a merge. There is no value in picking up ill-thought-out features.

-- Main.CrawfordCurrie - 02 Jan 2016 - 17:04

TMPL and macros are quite different in a lot of respects, last but not least TMPL:DEFs are cached and overrideable, one of the best features of Foswiki. Other products - such as Jive - have a similar templating system that let you redefine parts or modules of a page.

I agree with Crawford that TWiki's new EOTC feature is not particularly well thought out. Therefore we better don't adopt it. The only place this kind of stuff belongs to is the TWikiCompatibilityPlugin.

-- MichaelDaum - 04 Jan 2016

I changed the title from "Support TWiki %EOTC_ prefix to expand individual wiki MACROS during topic creation from template"

Proposal is in SupportAllMacrosInTemplateTopics

-- Main.CrawfordCurrie - 07 Jan 2016 - 14:12

Was this release in 2.2.0? If so shouldn't it have some state other than Waiting for Feedback?

-- JohnRouillard - 26 Jun 2016

Actually it has been implemented in the Item13909 branch, which needs merge into the master branch. I've changed the proposal status to "Ready to Merge", and this task to "Needs Merge".

-- GeorgeClark - 26 Jun 2016

Waiting to merge from the Item13905 branch, using "CREATE" (argue about that one later. I like it)

-- Main.CrawfordCurrie - 20 Feb 2017 - 20:25

Merged. trunk.foswiki.org now running with this.

-- GeorgeClark - 02 May 2017

Please do not add double-negotiations in configuration parameters:

$Foswiki::cfg{DisableEOTC} = $TRUE;

Better call it EnableSomething. Btw what does EOTC stand for? Could you name it in a way it is more intention revealing?

-- MichaelDaum - 02 May 2017
 
Topic revision: r35 - 31 Jan 2018, CrawfordCurrie
The copyright of the content on this website is held by the contributing authors, except where stated elsewhere. See Copyright Statement. Creative Commons License    Legal Imprint    Privacy Policy