Feature Proposal: Stop forcing skin authors to remove the final new-lines from tmpl files

Motivation

every release we have an issue like remove newlines from tmpl's to stop output beyond the end html marker.

This is entirely due to a legacy of using (some of) any text not inside a TMPL:DEF as the 'definition' of the output.

in twiki 4.x (i think it was) I re-arranged this mess from duplicated in each script.tmpl, and moved it into foswiki.tmpl, but this only simplified maintenance, without solving the root issue.

Yes, the patch below resolves the HTMLValidation errors we have atm.

Description and Documentation

Rather than continuing this 'definition by hard to read default' (in that the non TMPL:DEF text can be interleaved between TMPL:DEF 's) I propose we create a new named output definition region, such as completepage which replaces the fallthrough in foswiki.tmpl

%TMPL:DEF{completepage}%%TMPL:P{"htmldoctype"}%%TMPL:P{"head"}%%TMPL:P{"bodystart"}%%TMPL:P{"main"}%%TMPL:P{"bodyend"}%%TMPL:END%

and the code change to trunk would be

sven@quad7:~/src/foswiki/trunk$ svn diff core/templates/foswiki.tmpl core/lib/Foswiki/Templates.pm
Index: core/templates/foswiki.tmpl
===================================================================
--- core/templates/foswiki.tmpl   (revision 7521)
+++ core/templates/foswiki.tmpl   (working copy)
@@ -133,4 +133,5 @@
 }%%TMPL:END%
 
 %{ the bit that controls the output _MUST_ be last in the foswiki.SKIN.tmpl file. }%
+%TMPL:DEF{completepage}%%TMPL:P{"htmldoctype"}%%TMPL:P{"head"}%%TMPL:P{"bodystart"}%%TMPL:P{"main"}%%TMPL:P{"bodyend"}%%TMPL:END%
 %TMPL:P{"htmldoctype"}%%TMPL:P{"head"}%%TMPL:P{"bodystart"}%%TMPL:P{"main"}%%TMPL:P{"bodyend"}%
Index: core/lib/Foswiki/Templates.pm
===================================================================
--- core/lib/Foswiki/Templates.pm   (revision 7521)
+++ core/lib/Foswiki/Templates.pm   (working copy)
@@ -308,6 +308,11 @@
     $result =~ s/(%TMPL\:P{.*?}%)/_expandTrivialTemplate( $this, $1)/geo;
 
     $result =~ s|^(( {3})+)|"\t" x (length($1)/3)|geom; # leading spaces to tabs
+    
+    my $completepage = $this->expandTemplate('completepage');
+    if (defined($completepage) and ($completepage ne '')) {
+        return $completepage;
+    }
     return $result;
 }
 

by leaving the old non completepage TMPL code there, we should be able to support using these skin in foswiki 1.0 - will have to look at compatibility the other way..

Examples

Impact

I'd like to commit this to foswiki 1.1 - so could do with some feedback on its impact on other skins.

WhatDoesItAffect:

%WHATDOESITAFFECT%

edit

Implementation

-- Contributors: SvenDowideit - 24 May 2010

Discussion

Glad to see this done as a feature proposal.

The solution seems like the best path forward. Although it's a new feature, the first two solutions proposed in Item8846 were really new features also. So it is a feature to fix a bug (or at least, a shortcoming in the templating system).

I am hopeful we can see this in 1.1.

-- PaulHarvey - 24 May 2010

If nothing else, I think this would make the templates easier to understand. I'm not entirely sure, because I don't grok the templating system I like the explicit "start here" provided by completepage. I'll sleep on this, and maybe play with it in the next few days.

-- MichaelTempest - 24 May 2010

Main question in my mind is compatibility. At the moment we "select" the template to render based on the name of the script being run e.g. running "view" instantiates the pseudo-DEF at the root of "view.tmpl". You are adding an extra layer of indirection under this, which needs to be as simple to use as possible.

You might want to consider defining it thus:

If there exists a template definition called MASTER, then instantiate MASTER. Otherwise take the root content of the template read and call it MASTER

-- CrawfordCurrie - 24 May 2010

Sven's proposal is good, and it is more or less what Crawford rephrased in different words. The name completepage is okay too. We don't need several for view, edit and what so ever as the template system already alows to differentiate them sufficiantly.

The above seems to be backwards compatible already. So I see no problems for skins having to cope with newer and older engines at the same time.

I also like the fact that it explicitly names the template system's output def as this allows to override it in a rather late state of the expansion process. This wasn't possible before.

-- MichaelDaum - 24 May 2010

Just on the last day before the 14-day acceptance, I would like to have one thing confirmed as I am not sure myself.

An extension that uses templates such as CompareRevisionsAddon and HistoryPlugin - will they need to be updated to work with this? And if they are updated will they be compatible with 1.0.9?

The compatibility of extensions is my only concern. It would be bad if extensions have to be changed for 1.1 and no longer will work with 1.0.9. We all know many will spend a year or so on 1.0.9 before upgrading (because 1.0.9 is pretty stable and good) and they need to be able to download all extensions that use templates and use them.

I am less concerned about old skins in 1.1. There are few skins released in public and those that are not are made by people that can easily add the missing TMPL:DEF. It is only the non-skin extensions I am concerned about. If there is no issue with those, my concern goes away.

-- KennethLavrsen - 07 Jun 2010

gosh, my answer would be no, but suddenly i'm not 100% sure. Since i proposed this, alot of work has gone into the HTMLValidation tests (and fixes) I added, and so now I'd rather wait for 1.2 or 2.0 - I don't want to add more surprises.

-- SvenDowideit - 08 Jun 2010

Changing this to a Parked proposal. The initial developer has left.

-- Main.GeorgeClark - 16 Mar 2017 - 15:55