Item13905: changed implementation to #{...}#, unit tested more widely

foswiki · Jan 11, 2016 · 062ff5e · 062ff5e
1 parent b3c54f3
commit 062ff5e
Show file tree

Hide file tree

Showing 9 changed files with 104 additions and 66 deletions.
diff --git a/UnitTestContrib/test/unit/SkinTemplatesTests.pm b/UnitTestContrib/test/unit/SkinTemplatesTests.pm
@@ -742,4 +742,22 @@ sub test_TMPL_PREV {
     return;
 }
 
+sub test_comments {
+    my $this = shift;
+    my $data;
+
+    write_template(
+        'yview', '%TMPL:%{goway}%DEF{"junk"}%A
+
+%{die}%
+
+B%TMPL:END%%TMPL:P{junk}%-%TMPL:%{goway}%DEF{"clunk"}%C #{die}# D%TMPL:END%%TMPL:P{clunk}%'
+    );
+
+    $data = $tmpls->readTemplate('yview');
+    $this->assert_str_equals( 'AB-C  D', $data );
+
+    return;
+}
+
 1;
diff --git a/UnitTestContrib/test/unit/TopicTemplatesTests.pm b/UnitTestContrib/test/unit/TopicTemplatesTests.pm
@@ -231,6 +231,7 @@ sub test_templateTopicWithMacros {
     my $query = Unit::Request->new(
         {
             text => [<<'TEXT'],
+   * NOP: No%NOP%Link
    * DATE: %DATE%
    * TMPL:DATE: %TMPL:DATE%
    * GMTIME: %GMTIME%
@@ -240,9 +241,9 @@ sub test_templateTopicWithMacros {
    * URLPARAM: %URLPARAM{"purple"}%
    * WIKINAME: %WIKINAME%
    * WIKIUSERNAME: %WIKIUSERNAME%
+   * COMMENT: #{ ... comment }#
    * TMPL:IF: %TMPL:IF{"1=1" then="OK" else="BAD"}%
    * TMPL:P:%TMPL:P{"sep"}%
-   * NOP: No%NOP%Link
 %STARTSECTION{type="templateonly"}%
 Mither me not
 %ENDSECTION{type="templateonly"}%
@@ -268,7 +269,7 @@ TEXT
     $this->createNewFoswikiSession( $this->{test_user_login}, $query );
     my ( $responseText, $result, $stdout, $stderr ) =
       $this->captureWithKey( save => $UI_FN, $this->{session} );
-
+    print STDERR $stderr;
     my ($meta) =
       Foswiki::Func::readTopic( $this->{test_web}, 'TemplatedTopic' );
     my $text = $meta->text;
@@ -285,6 +286,7 @@ TEXT
     $this->assert( $text =~ s/^\s*\* NOP: NoLink$//m,                $text );
     $this->assert( $text =~ s/^\s*\* TMPL:IF: OK$//m,                $text );
     $this->assert( $text =~ s/^\s*\* TMPL:P: \| $//m,                $text );
+    $this->assert( $text =~ s/^\s*\* COMMENT: $//m,                  $text );
     $this->assert( $text =~ s/^TemplatedTopic$//m,                   $text );
     $this->assert( $text =~ s/^%TOPIC%$//m,                          $text );
     $this->assert( $text !~ /Mither me not/s, $text );

diff --git a/UnitTestContrib/test/unit/VariableTests.pm b/UnitTestContrib/test/unit/VariableTests.pm
@@ -70,6 +70,7 @@ sub test_embeddedExpansions {
     return;
 }
 
+# SMELL: this duplicates TopicTemplatesTests
 sub test_topicCreationExpansions {
     my $this = shift;
 
@@ -146,22 +147,24 @@ sub test_macroParams {
     # Check default given, given but null, not given
     # Check quotes and other standard expansions
     # Check override of standard macros
+    # Check comment #{...}# expansion in difficult sites
     $this->{session}->{prefs}->setSessionPreferences(
         ARFLE => '%BARFLE{default="gloop"}%',
         TING  => '%DEFAULT% %DEFAULT{default="tong"}%',
-        ALING => '\'%DEFAULT%\' \'%DEFAULT{default="tong"}%\'',
+        ALING => '\'%DEF#{i}#AULT%\' \'%DEFAULT{#{i}#default="tong"}%\'',
         TOOT  => '\'%NOP%\'',
         WOOF  => '%MIAOW{default="$quot$percent$quot"}%',
         Test  => '"%arg{default="%DEFAULT{default="Y"}%"}%"'
     );
     my $input = <<'INPUT';
-| gloop | %BARFLE{default="gloop"}% |
+| C | #{ ignore me }# |#{i}#
+| gloop | %BARFLE{def#{i}#ault="gloop"}% |
 | mong | %ARFLE{BARFLE="mong"}% |
 | %DEFAULT% tong | %TING% |
 | ding ding | %TING{"ding"}% |
 | '' '' | %ALING{""}% |
 | 'sweet' | %TOOT{NOP="sweet"}% |
-| "%" | %WOOF% |
+| "%" | %#{i}#WOOF#{i}#% |
 | p"r"r | %WOOF{MIAOW="p$quot()r$quot()r"}% |
 | Test | %Test% |
 | Test{"X"} | %Test{"X"}% |
@@ -172,6 +175,7 @@ INPUT
     $topicObject->text($input);
     my $result   = $topicObject->expandMacros($input);
     my $expected = <<'EXPECTED';
+| C |  |
 | gloop | gloop |
 | mong | mong |
 | %DEFAULT% tong | %DEFAULT% tong |

diff --git a/core/data/System/EditingShorthand.txt b/core/data/System/EditingShorthand.txt
@@ -1,4 +1,4 @@
-%META:TOPICINFO{author="ProjectContributor" date="1435796369" format="1.1" version="1"}%
+%META:TOPICINFO{author="ProjectContributor" date="1452527048" format="1.1" version="1"}%
 ---+ Editing Shorthand
 %STARTINCLUDE%
 %TABLE{}%
@@ -767,6 +767,13 @@ required
 ---++!! offset was 1, so offset is now 0
 </td>
 </tr>
+<tr>
+<td>
+=#<nop>{ ... comment ... }<nop>#=
+</td><td>
+*Comments:* %BR%
+Comments will be removed from content before the topic is displayed.
+</td>
 </table>
 <!-- %JQREQUIRE{"chili"}% -->
 

diff --git a/core/data/System/Macros.txt b/core/data/System/Macros.txt
@@ -1,4 +1,4 @@
-%META:TOPICINFO{author="ProjectContributor" date="1434650530" format="1.1" version="1"}%
+%META:TOPICINFO{author="ProjectContributor" date="1452527048" format="1.1" version="1"}%
 %META:TOPICPARENT{name="UserDocumentationCategory"}%
 %STARTINCLUDE%
 ---+ Macros
@@ -299,6 +299,10 @@ Most predefined macros return values that were either set in the configuration w
 
 %IF{"'%INCLUDINGTOPIC%'!='CompleteDocumentation'" then="See MacrosQuickReference for a complete list of predefined macros in your installation."}%
 
+---++ Comments
+
+You can add comments anywhere in macros (and outside them) using the standard =#<nop>{...}<nop>#= comment delimiters. Comments are removed *before* any macros are processed, so you can't use macros to generate comments. Any macros inside comments will be ignored.
+
 %STOPINCLUDE%
 
 ---

diff --git a/core/data/System/SkinTemplates.txt b/core/data/System/SkinTemplates.txt
@@ -1,4 +1,4 @@
-%META:TOPICINFO{author="ProjectContributor" comment="" date="1452097309" format="1.1" version="1"}%
+%META:TOPICINFO{author="ProjectContributor" comment="" date="1452527048" format="1.1" version="1"}%
 %META:TOPICPARENT{name="Skins"}%
 %STARTINCLUDE%
 ---+ Skin Templates
@@ -45,9 +45,10 @@ Template directives look a lot like standard [[macros]].
    * =%<nop>TMPL:END%= ends a block definition.
    * =%<nop>TMPL:PREV%=: returns the previous definition of the block being defined.
    * =%<nop>TMPL:P{"name"}%= includes a previously defined block.
-   * =%<nop>{...}%= is a comment. Whitespace either side of the comment
+   * =#<nop>{...}#= is a comment. Whitespace either side of the comment
      (newlines, spaces, tabs etc) is treated as part of the comment, and
-     removed when the comment is removed.
+     removed when the comment is removed. An alternate syntax =%<nop>{...}%=
+     is also supported.
 You can use a block before _or_ after declaring it. If you define the same
 block twice, only the second definition is used.
 
@@ -162,9 +163,12 @@ and then =* Set SKIN=crumbless,pattern=
 Remember: the template path contains the most specific template first.
 
 ---+++ Comments
-Comments =%{...}%= are removed from the templates as soon as the file is
-read, before any other template macros are evaluated. Whitespace either side
-of the comment (newlines, spaces, tabs etc) is also removed.
+Comments delimited by =#{...}#= are removed from the templates as soon as
+the file is read, before any other template macros are evaluated. 
+
+An alternative comment type =#{...}#= will automatically absorb whitespace
+either side of the comment (newlines, spaces, tabs etc). This type of comment
+only works in templates.
 
 #FindingTemplates
 ---++ Finding Skin Templates
@@ -254,6 +258,12 @@ Note that topics containing templates are checked for VIEW access using the
 normal [[AccessControl][Foswiki access controls]]. Any access control failure
 is silently ignored, and the template path expansion continues.
 
+---++ Predefined templates
+Most predefined templates are stored in =.tmpl= files. There are also a
+couple of standard topics that contain skin templates:
+    * WebCreateNewTopicTemplate - Page shown when you click on a %ICON{newtopic}% *Create New Topic* link. It provides a form requesting the necessary information to create a new, nonexistent topic.
+   * TopicDoesNotExistViewTemplate - Alert page shown when you try to view a nonexistent topic and usually used as a prompt to help you create this new topic. For this reason, the form of the WebCreateNewTopicTemplate is included and therefore shown, too.
+
 ---++ Developing new templates
 ---+++ Debugging
 When writing new templates, it can sometimes it can be hard to work out
@@ -328,12 +338,12 @@ see what PatternSkin defines.
 
 %STOPINCLUDE%
 
-%META:FILEATTACHMENT{name="oopsmore_diff_debug.gif" attr="h" comment="" date="1452097309" size="3205" user="ProjectContributor" version="1"}%
-%META:FILEATTACHMENT{name="default.css.gz" attr="h" comment="" date="1452097309" size="2072" user="ProjectContributor" version="1"}%
-%META:FILEATTACHMENT{name="base.css.gz" attr="h" comment="" date="1452097309" size="1331" user="ProjectContributor" version="1"}%
-%META:FILEATTACHMENT{name="oopsmore_diff_sidebyside.gif" attr="h" comment="" date="1452097309" size="1994" user="ProjectContributor" version="1"}%
-%META:FILEATTACHMENT{name="base_src.css" attr="h" comment="" date="1452097309" size="6780" user="ProjectContributor" version="1"}%
-%META:FILEATTACHMENT{name="base.css" attr="h" comment="" date="1452097309" size="5196" user="ProjectContributor" version="1"}%
-%META:FILEATTACHMENT{name="oopsmore_diff_sequential.gif" attr="h" comment="" date="1452097309" size="3464" user="ProjectContributor" version="1"}%
-%META:FILEATTACHMENT{name="default_src.css" attr="h" comment="" date="1452097309" size="11093" user="ProjectContributor" version="1"}%
-%META:FILEATTACHMENT{name="default.css" attr="h" comment="" date="1452097309" size="8209" user="ProjectContributor" version="1"}%
+%META:FILEATTACHMENT{name="oopsmore_diff_debug.gif" attr="h" comment="" date="1452527048" size="3205" user="ProjectContributor" version="1"}%
+%META:FILEATTACHMENT{name="default.css.gz" attr="h" comment="" date="1452527048" size="2072" user="ProjectContributor" version="1"}%
+%META:FILEATTACHMENT{name="base.css.gz" attr="h" comment="" date="1452527048" size="1331" user="ProjectContributor" version="1"}%
+%META:FILEATTACHMENT{name="oopsmore_diff_sidebyside.gif" attr="h" comment="" date="1452527048" size="1994" user="ProjectContributor" version="1"}%
+%META:FILEATTACHMENT{name="base_src.css" attr="h" comment="" date="1452527048" size="6780" user="ProjectContributor" version="1"}%
+%META:FILEATTACHMENT{name="base.css" attr="h" comment="" date="1452527048" size="5196" user="ProjectContributor" version="1"}%
+%META:FILEATTACHMENT{name="oopsmore_diff_sequential.gif" attr="h" comment="" date="1452527048" size="3464" user="ProjectContributor" version="1"}%
+%META:FILEATTACHMENT{name="default_src.css" attr="h" comment="" date="1452527048" size="11093" user="ProjectContributor" version="1"}%
+%META:FILEATTACHMENT{name="default.css" attr="h" comment="" date="1452527048" size="8209" user="ProjectContributor" version="1"}%
diff --git a/core/data/System/TemplateTopics.txt b/core/data/System/TemplateTopics.txt
@@ -1,26 +1,20 @@
-%META:TOPICINFO{author="ProjectContributor" date="1452097309" format="1.1" version="1"}%
+%META:TOPICINFO{author="ProjectContributor" date="1452527048" format="1.1" version="1"}%
 %META:TOPICPARENT{name="DeveloperDocumentationCategory"}%
 #TemplateTopics
 ---+ Template topics
 
-Foswiki uses several types of templates. __Template topics are topics that define the default text for new topics.__
+Foswiki uses several types of templates. __Template topics are topics that define the basic text for new topics.__
 
-(If you are looking for templates used to display existing topics, see SkinTemplates.  These are combined with DataForms
-for custom presentation of topic data (View Template) and custom editing (Edit Template).
+(If you are looking for the templates used for custom presentation of existing topics, see SkinTemplates).
 
 %TOC%
 
 %STARTSECTION{"PowerUserGuide"}%
 ---++ Overview
-There are four types of template topics:
 
-| *Topic Name:* | *What it is:* |
-| WebCreateNewTopicTemplate | Page shown when you click on a %ICON{newtopic}% *Create New Topic* link. It provides a form requesting the necessary information to create a new, nonexistent topic. |
-| TopicDoesNotExistViewTemplate | Alert page shown when you try to view a nonexistent topic and usually used as a prompt to help you create this new topic. For this reason, the form of the WebCreateNewTopicTemplate is included and therefore shown, too. |
-| WebTopicEditTemplate | Default text used in a new topic. |
-| &lt;MyCustomNamed&gt;Template | Whenever you create a topic ending in the word "Template", it is automatically added to the list of available template topics in the "Use Template" drop down field on the !WebCreateNewTopic page. |
+Whenever you create a topic ending in the word "Template", it is automatically added to the list of available template topics in the "Use Template" drop down field on the !WebCreateNewTopic page.
 
-When you create a new topic using the =edit= script, the system locates a template topic according to the following search order:
+When you create a new topic using the =edit= script, the system inserts default content taken from a template topic located using the following search order:
    1 A topic name specified by the =templatetopic= CGI parameter
       * if no web is specified, the current web is searched first and then the %SYSTEMWEB% web
    1 <nop>WebTopicEditTemplate in the current web
@@ -35,8 +29,6 @@ creation by prepending =TMPL:= to the macro name. For example,
 templated topic was created. SkinTemplates can also be expanded
 using =%<nop>TMPL:P{"templatename}%=.
 
----+++ Compatibility
-
 For compatibility with versions of Foswiki < 2.1, when the following macros are used in a template topic, they automatically get expanded when a new topic is created based on it:
 
 | *Macro:* | *Description:* |
@@ -67,6 +59,8 @@ This will restrict who can edit the template topic, but will get removed when a
 
 All other macros are unchanged, e.g. are carried over "as is" into the new topic, _unless they are contained within a %<nop>STARTSECTION{type="expandvariables"}% section._
 
+You can add comments to template topics using the =#<nop>{...}<nop>#= comment delimiters. These comments will be removed when the template topic is instantiated (they will *not* appear in the created topic.)
+
 ---++ Specifying a form
 When you create a new topic based on a template topic, you often want the new topic to have a form attached to it. You can attach a form to the template topic, in which case it will automatically be copied into the new topic.
 

diff --git a/core/lib/Foswiki.pm b/core/lib/Foswiki.pm
@@ -3315,6 +3315,8 @@ sub _processMacros {
     #no tags to process
     return $text unless ( $text =~ m/%/ );
 
+    #my $grunt = 1; uncomment lines mentioning $grunt for tracing
+
     unless ($depth) {
         my $mess = "Max recursive depth reached: $text";
         $this->logger->log( 'warning', $mess );
@@ -3328,6 +3330,9 @@ sub _processMacros {
     my $verbatim = {};
     $text = takeOutBlocks( $text, 'verbatim', $verbatim );
 
+    # Remove comments
+    $text =~ s/#{.*?}#//gs;
+
     my $dirtyAreas = {};
     $text = takeOutBlocks( $text, 'dirtyarea', $dirtyAreas )
       if $topicObject->isCacheable();
@@ -3340,42 +3345,34 @@ sub _processMacros {
 
     while ( scalar(@queue) ) {
 
-        #print STDERR "QUEUE:".join("\n      ", map { "'$_'" } @queue)."\n";
+  #print STDERR "QUEUE:".join("\n      ", map { "'$_'" } @queue)."\n" if $grunt;
         my $token = shift(@queue);
 
-        #print STDERR "UNQUEUE $token \n";
+        #print STDERR "UNQUEUE $token \n" if $grunt;
 
         # each % sign either closes an existing stacked context, or
         # opens a new context.
         if ( $token eq '%' ) {
 
-            #print STDERR " STACKTOP $stackTop\n";
+            #print STDERR " STACKTOP $stackTop\n" if $grunt;
             # If this is a closing }%, try to rejoin the previous
             # tokens until we get to a valid tag construct. This is
             # a bit of a hack, but it's hard to think of a better
             # way to do this without a full parse that takes % signs
             # in tag parameters into account.
             if ( $stackTop =~ m/}$/s ) {
                 while ( scalar(@stack)
-                    && $stackTop !~ /^%($regex{tagNameRegex}|)\{.*}$/s )
+                    && $stackTop !~ /^%$regex{tagNameRegex}\{.*}$/s )
                 {
                     my $top = $stackTop;
 
-                    #print STDERR " COLLAPSE $top \n";
+                    #print STDERR " COLLAPSE $top \n" if $grunt;
                     $stackTop = pop(@stack) . $top;
                 }
             }
 
-            if ( $stackTop =~ /^%\{.*\}$/s ) {
-
-                # Comment, ignore it
-                #print STDERR " IGNORE $stackTop $tag\n";
-                $stackTop = '';    # no new context
-                next;              # token
-            }
-
             # /s so you can have newlines in parameters
-            if ( $stackTop !~ m/^%(($regex{tagNameRegex}|)(?:{(.*)})?)$/s ) {
+            if ( $stackTop !~ m/^%(($regex{tagNameRegex})(?:{(.*)})?)$/s ) {
 
                 # Not a valid tag expr
                 push( @stack, $stackTop );
@@ -3386,16 +3383,14 @@ sub _processMacros {
             # SMELL: unchecked implicit untaint?
             my ( $expr, $tag, $args ) = ( $1, $2, $3 );
 
-            $stackTop = '%';        # open new context
-
-            #print STDERR " POP $tag\n";
+            #print STDERR " POP $tag\n" if $grunt;
             my $e = &$tagf( $this, $tag, $args, $topicObject );
 
             #Monitor::MARK("After $tag");
 
             if ( defined($e) ) {
 
-                #print STDERR " EXPANDED $tag -> $e\n";
+                #print STDERR " EXPANDED $tag -> $e\n" if $grunt;
                 $stackTop = pop(@stack);
 
                 # Don't bother recursively expanding unless there are
@@ -3411,7 +3406,7 @@ sub _processMacros {
                 next;    # token
             }
 
-            #print STDERR " EXPAND $tag FAILED\n";
+            #print STDERR " EXPAND $tag FAILED\n" if $grunt;
             # To handle %NOP
             # correctly, we have to handle the %VAR% case differently
             # to the %VAR{}% case when a variable expansion fails.
@@ -3458,7 +3453,7 @@ sub _processMacros {
 
     putBackBlocks( \$stackTop, $verbatim, 'verbatim' );
 
-    #print STDERR "FINAL $stackTop\n";
+    #print STDERR "FINAL $stackTop\n" if $grunt;
 
     return $stackTop;
 }