Feature Proposal: Support comment syntax in topics and topic templates

Motivation

Comments are very important in provided context, explanation and recording. Currently the only way a comment can be inserted in a topic is using HTML comments, which means that the comments end up being transmitted to the browser, wasting bandwidth and processing time in the renderer. It would be far better to have a comment syntax which can be removed as soon as possible, c.f. the %{nop>{...}% syntax in skin templates.

Description and Documentation

For a long time it's been a source of frustration to me that I could embed comments in skin templates, but not topic templates or even topic source. There is an HTML comment syntax, but that has the problem that sections inside HTML comments may still be vulnerable to further processing by rendering modules, and the comments consume bandwidth when rendering the page.

It would make sense to support a comment syntax in topics.

The proposed syntax is #{...}#

Details:

• Comments will be removed at the start of skin template processing, i.e. handled exactly like %{...}% (which will be retained for compatibility)
• Comments will be removed at the start of topic template instantiation i.e. a comment in a topic template will not survive into the created topic.
• Comments will be removed at the start of macro processing for normal topics i.e. comments will override macros and all TML rendering constructs except <verbatim>

-- Contributors: CrawfordCurrie - 11 Jan 2016

Discussion

(The discussion below up to 9/1/2016 relates to the original proposed syntax %{...}% which is rejected for the reasons discussed)

Few remarks (as usually, these aren't concerns) - just asking:

1. how will this affect the constructions like %{<verbatim>}%
2. allows be nested? %{ here %{ another }% one}%
3. is this legal? %WEB%{ some text here }%TOPIC%
4. what will happens inside of the <verbatim> %{text}% </verbatim>
5. what happens when someone enters the %{text}% into the attachment description at upload? e.g. it gets into the %META fields?

• of course I understand, than the syntax %{...}% is an legacy, but you sure know also, than from the view of the theory of writing language-parsers for the Foswiki macro-language the plain %{ wasn't an good choice. IMHO, would be much better to use something
  1. what isn't collides with the normal macro markup components - e.g. without the % and {} characters
  2. e.g. for the comments let introduce something totally new and different tag-pairs what can be safely removed as soon as possible without the fear than it will break something
  3. or at least use something as %#{ which is better parseable (not good too, but better as the plain %{ )

What is the problem with the users original idea using the <comment> ?

-- JozefMojzis - 08 Jan 2016

Templates already use %{ ... }% as comments. So this feature just extends a syntax that is already well established. See for example, System.WebCreateNewTopicTemplate I don't see any reason to invent a new comment when we already have a comment syntax.

-- GeorgeClark - 08 Jan 2016

OK - as I said - I'm only curious. Will see the implementation.

-- JozefMojzis - 08 Jan 2016

Good questions, Jozef.

Remember that the pipeline looks like this:

1. Skin template expansion
2. Macro expansion
3. TML rendering

The syntaxes used in these three phases are distinct; phase 1 uses %TMPL:, phase 2 uses %MACROS% and phase 3 uses pseudo-html tags, such as <sticky>

1. sticky, verbatim and the like are phase 3 constructs and as such are handled by the renderer. Rendering only starts when all macro expansions are done. Thus %{...}% comments will comment them out,
2. Yes. The syntax is a natural extension of the macro syntax and obeys all the same rules.
3. See 1
4. See 1
5. See 1. This may require some adjustment to the escapes.

I don't understand what you have against the syntax; I chose it because it fitted very naturally into the parsers and indeed, the implementation in Foswiki.pm couldn't be much simpler.

I don't have a problem with block comments, but they need to fit into the phase 2 syntax, and not the TML rendering syntax. If you delay comment processing until after macros are expanded, you run the risk of processing macros that are subsequently elided by comments e.g.

<comment>
%SEARCH{"every single topic 100 times"}%
</comment>

Thus %SECTION{type="comment"}% would be a natural choice (albeit a bit clumsy).

-- Main.CrawfordCurrie - 08 Jan 2016 - 23:31

For the 1.) - exactly that i tried to tell. In the template topic's me often using an construction like: %{<verbatim>}%

, e.g.:

%{

}%
%INCLUDE{"SomeTopic"}%
%{

}%

e.g. for the templates, the <verbatim> is hidden, (for the TMPL it is commented out) - so, the INCLUDE will work, but when viewing the Template Topic in normal topic view, it shows the content of as in the verbatim block. (as above) - thus the include doesn't gets expanded.

When the %{ will work for the normal topic's view too, all such constructs will stop work, e.g. in the above will show the INCLUDE error about the nonexistent SomeTopic, because the <verbatim> get removed (will be commented out) before the phase3...

If this is the goal - it really give a reason for raising concern. BTW, in the F.O. is also used the %{<verbatim>}%

construction often.

-- JozefMojzis - 09 Jan 2016

Exactly. It appears that ages ago, SvenDowideit checked in a SkinTemplateViewTemplate which disables any ability to view skin templates. I could not figure out what jomo was talking about because all our templates display a hidden div.

Delete SkinTemplateViewTemplate, and then visit a topic like WebCreateNewTopicTemplate. Once the view template has been removed, the template is quite viewable without resorting to raw view. Our templates make extensive use of this feature, and we probably ought to remove the VIEW_TEMPLATE setting from templates that are annotated in this way.

So this proposal will break those topics, although they are already broken by the view template. I wonder if a syntax like %#{...}% might make sense as it's very similar but allows template comments to be visible when rendered as a topic.

-- GeorgeClark - 09 Jan 2016

In my "hacked" version of Foswiki me using only "line based comments".

%# This is a comment
%# e.g. 3 char sequence at the beginning of the line "%" plus "#" and "space"
%#this is not a comment - missing the space
%# It honors verbatim (so comments in verbatim block are shown untouched)

of course, it isn't an good solution, but DWIM (mostly).

The diff is this:

diff --git a/core/lib/Foswiki.pm b/core/lib/Foswiki.pm
index 7a38d0d..1ba3159 100644
--- a/core/lib/Foswiki.pm
+++ b/core/lib/Foswiki.pm
@@ -3112,6 +3112,8 @@ sub innerExpandMacros {
         WEB   => $webContext
     );
 
+    $$text =~ s/^%# .*$//gm; #MYHACK# comments
+
     # Escape ' !%VARIABLE%'
     $$text =~ s/(?<=[\s\(\.])!%($regex{tagNameRegex})/&#37;$1/g;

-- JozefMojzis - 09 Jan 2016

Now I'm totally confused. Jozef, when you say "in the template topics" do you mean "topic templates", or "skin templates" that are stored in topics? I'll assume the latter, because your description makes no sense in "topic templates".

OK, it looks like so you are using the existing "skin template" comment syntax to comment out constructs that you want expanded when the "skin template" topic is viewed normally, but excluded when a topic is rendered based on that skin. Is that correct? If so, you are explicitly leveraging the fact that %{ only works in skin templates, and I'm wasting my time here.

I am trying to rationalise macro syntax across the different rendering phases, and %#{ introduces a new syntax when I'm explicitly trying to reduce them. So I don't want to do that.

Maybe I'm trying too hard to collapse the "skin template" syntax and the "topic template" syntax ( Tasks.Item13909). Perhaps instead we should be concentrating on making the syntaxes processed by the different phases explicitly different. In which case the use of TMPL: in Tasks.Item13909 is a mistake. and it should be something like: TT: (for "topic template"). Then the introduction of a %SECTION{type="remark"}% and a simple macro %REMARK{"this is a comment"}% (homage to BASIC) should suffice to address the requirements of both the "topic template" and "normal view" expansion phases.

-- Main.CrawfordCurrie - 09 Jan 2016 - 13:36

The %{ comment syntax that only works in templates was exploited by ArthurClemens back in May 2012 in a total of 2 of our Skin Template Topics.

• FAQViewTemplate and
• WebCreateNewTopicTemplate.

In Nov. 2012, that feature was hidden behind a view template. It was never in Foswiki 1.1, and is invisible in Foswiki 2.0 unless users (like Jomo) have discovered and exploited it. I can't find where it's documented. It appears to be a side effect of the Template comment feature that was actually added to help control accidental white-space in templates that caused broken HTML.

So this seems to be a rather accidental, undocumented feature, and I don't see any strong reason to preserve it exactly as is. Stepping back a bit.

%{ ... }% comments:

• Are stripped from templates when processed as a template. Surrounding white-space is also removed, resolving an old template issue where extra lines show up after the body.
• Renders as noise while a template is viewed as a topic
• Permits a <verbatim> to be inserted into the noise, so that a template is more readable when viewed as a topic.
• And is rendered moot in our shipped templates, because of a custom VIEW_TEMPLATE set in the meta preferences of every topic completely hides the contents.

And the underlying requirements are:

• A mechanism to hide topic content from any transmission to the browser during normal rendering
• Some way to view a template topic in a way readable by mortals ... a "modified raw" view if possible.
• Avoid adding more macros or syntax. Keep things consistent with existing syntax where possible.

Given that the current "hidden verbatim" is rather less than ideal - it sprinkles in extraneous %{ and }%, is there some way to build upon the SkinTemplateViewTemplate so that it can do a better job of displaying a Skin Template? Maybe something along the line of the INCLUDE doc handler, which formats a template, displaying comments as wiki text and anything outside of the comments into a verbatim block?

So I'd recommend amending this proposal to:

1. Implement the %{ ... }% syntax as originally proposed.
2. Add a Foswiki::IncludeHandlers::tmpl handler that can format any template in readable format.
3. Rework the SkinTemplateViewTemplate to format the topic via the include handler.
4. Add a TemplateDoc topic which formats files in the template directory similar to the PerlDoc topic.

-- GeorgeClark - 09 Jan 2016

As usually, me causing a lot of confusion. I would have done better if I kept silent.

So, step-by-step. First:

• yes, as you correctly identified - I'm babbling about the skin templates.
• I just tried to tell - the proposed syntax will breaks the %{<verbatim>}% , which is often used in some SKIN TEMPLATES.
• myself managing only few foswiki webs therefore i'm able change the "skin template" topics, so if you want extend the comment syntax - by me no concerns.
• again, I just wanted to point out that this can cause problems for other users, who likes the %{<verbatim>}% trick.
• that's all.

Second:

• exactly just because we have 3 different rendering engines (different "languages") which each uses different syntax and does different things, IMHO we need different comment syntax too, because the parser can't identify "to who belongs" the particular comment (to which phase).
• Example: I can imagine that someone would want to insert comments into the newly created topics. Well, he will put those comments into the "template topic" hoping get the "comments" into the newly created topics. (and now i'm talking about the template topics). But since we will have a unified syntax for the comments for both phases, he is unable to do this.
• of course, this isn't an important feature - only a demonstration why the unified comment syntax could hurt. (where the parser could not distinguish to whom really belongs the comment (to which phase - e.g. to which parser))

Third - about the unified syntax as all:

• suprisingly I totally agree with you about than "we need an unified syntax - for the templates and for the macros".
• i do not commented your initiative about the TMPL:.. thing ( Tasks.Item13909 ) - just because it is a good idea
• just, (again IMHO) for the "final solution" this needs much more, honestly, for the true unification we need some more powerful templating language, for example
  • which knows true "variables"
  • which could done true encapsulation (where the variabled gets expanded not as now - every SET is expanded in the caller context and not in the called)
  • which allows register paired tags (so we could be able much more elegantly solve paired tags like %TAB% ... %ENDTAB% -but for this we need parse the whole topic to AST first...
  • which allows replace blocks (as the current TMPL:DEF) - but more generally
  • and so on... more and more...
• talking about these "needs" are not belongs into the comments in this topic. (and nor to the SupportAllMacrosInTemplateTopics, because foswiki probably will never change the templating system to something more powerful - even if the "backward compatibility problem" is solvable by an thin layer, which could automatically translate the old template & macro syntax to the new (hypothetical) one.
• and moreover probably nobody share my above thoughts

Finally - the result:

• i'm fully supporting ANY of your initiative, because ANY of your initiative will make foswiki better and more powerful
• me only point out some possible problems with this proposal
• but I'm NOT RAISING ANY CONCERNS.
• so DO it.

-- JozefMojzis - 09 Jan 2016

So in all that, is there any way to insert a comment into a topic when it originates from a template. That does seem useful. Rather than a new syntax, some sort of escape mechanism when the template is "expanded on topic creation" ?

-- GeorgeClark - 09 Jan 2016

As i told in the IRC chat - i'm perfectly happy with any proposed syntax.

-- JozefMojzis - 09 Jan 2016

But I'm not - Jozef's points are sound, and on reflection conflating the "skin template" and "topic template" syntax is as dangerous and undesirable as the current conflation of the "topic template" and "normal topic". It just remains to find the right syntax, and document it correctly.

• Currently we have two comment syntaxes - %{...}% in "skin templates", and HTML comments ( =<?...> and <!-- ... --> )in "normal topics".
• We want to be able to express comments that are removed at any of the three stages of the pipeline - "skin template" expansion, "topic template" expansion, "normal topic" expansion. Comment removal in stage N should support stage N+M comment creation.
• Parser capabilities are pretty much irrelevant here, the priority is to find user-friendly syntaxes.
• %{...}% is fairly nice to work with, but suffers from a nesting problem when employed in complex template expansions (you can't write %<nop{%{}% and expect it to work in a "topic template" or "normal topic"),

Personally I think comments are comments, and should be removed as soon as possible from source text. That was certainly my intention - the verbatim trick is something I hadn't foreseen, and unfortunately we need to keep it around for compatibility, but I'd rather play it down and implement a single comment syntax - #{...}# - that works everywhere. This would be removed at the earliest possibly opportunity i.e.

• Before skin template macro expansion
• before topic template macro expansion, either for view or for generation of a new topic
• Before INCLUDE of a topic
• Before normal topic macro expansion

-- Main.CrawfordCurrie - 10 Jan 2016 - 07:20

The #{ ... }# is nice. Just we need? (probably) ensure than we will not remove blindly anything from a topic containing some patch-code, e.g. like from the following hypothetical topic: Start of the topic

---

Here are some lines form the DBCachePlugin

./DBCacheContrib/lib/Foswiki/Contrib/DBCacheContrib/MemArray.pm:    $#{ $this->{values} } = $count - 1;

and this too

./DBCachePlugin/lib/Foswiki/Plugins/DBCachePlugin/Core.pm:        $temp =~ s#\)#${TranslationToken}#g;

---

End of the topcic

Note,

• the 1st verbatim containing the sequence #{
• and the second the }#.
• Both are form the real codebase, so we probably could'nt remove everything between the #{ ... and ... }# simply "blindly".

It is very hard to find any two-character combination for the context-sensitive texts as our topics, which could contain

• any user-entered free texts (in such case the user probably could correct his "entry"
• macros (and their arguments - quoted or unquoted)
• patches for perl, css, javascript or any language - because the "patch" code is generated, isn't possible edit them later to "remove" the "colliding" pairs (like above) in special blocks like <verbatim>
• etc...

and will NOT collide with the content - for removing "anything between the comment-pairs" - very early.

-- JozefMojzis - 10 Jan 2016

I'm not sure, whether the following fits into this discussion, but I'm looking for a syntax, that removes part of a topic just before it hits the wire. I don't want some Foswiki internals to get sent to the browser. Let's say ALLOW settings or alike, that foswiki recommends zu hide with ordinary HTML comments.

This would offer us the possibility to finally retire IfDefinedPlugin in our installation.

-- Main.AndreLichtsteiner - 10 Jan 2016 - 13:26

How will topic comments fit into the rendering order. If they are "blindly removed", then they will break content, as in JozefMojzis ' verbatim example. If they follow the Foswiki rendering order, a "null macro" for example, then following the inside-out order, a %SET macro inside a #{ comment would be evaluated then commented out.

I think comments need to be stripped after verbatim blocks are removed but prior to any further macro expansion or rendering.

-- GeorgeClark - 10 Jan 2016

Jozef, we already handle verbatim blocks, avoiding macro expansion in them. So long as comments are handled at the same point in the pipeline as macros, there shouldn't be a problem.

Andre, not sure what you mean by "just before it hits the wire". Do you mean you want to remove content just before content is sent to the browser? If so, then none of the discussion above is applicable. There's currently no way to do what you describe - mainly because HTML comments already exist to do it.

-- Main.CrawfordCurrie - 10 Jan 2016 - 20:52

Crawford, George

I was thinked on some

• Set COMMENT_REMOVE = off

to disable the comment removal in the given topic, but if the comments will be removed, after the verbatims are handled, im happy with the proposed #{ ... comment ... }# . <--- this will be removed, after FO get's the new comments removal installed.

Just for a curiosity, what will be the result of the following?

#{ < --- start of the comment
<verbatim>
some content here
</verbatim>
end of the comment --> }#

-- JozefMojzis - 10 Jan 2016

Our use case would be fine with markup, that behaves as proper wiki %-macro, as we do today with IfDefinedPlugin:

%IFDEFINEDTHEN{"'X' = 'U'"}% 
.. some in Topic SECTIONS to be INCLUDEd 
.. some Settings
.. some Comments
%FIDEFINED%

-- Main.AndreLichtsteiner - 11 Jan 2016 - 07:07

Andre, your questions/points don't really relate to this proposal; can you take them elsewhere, please?

Jozef, your curiosity would be removed. I don't want comment removal to be optional - the more control options we have, the more confused we all get. You already have %IF to handle optional inclusions, use that instead.

George, I had in mind to remove them at the beginning of Foswiki::processMacros, which is effectively the site you describe (i.e. Set has already been processed and verbatims are removed. However after reviewing the code, I see that is not possible. verbatim is a rendering construct, it blocks TML processing of the content of a block. Macros are fully expanded before verbatim is hit, so if comments are going to block macro expansion, there is no way they can be handled before verbatim. It's possibly to "build" a verbatim block using macros, but no way for the macro processor to predict that's going to happen. So right now I don't see how it would be possible to support <verbatim>...#{...}#...</verbatim>

So, the alternatives are:

1. Think of #{...}# as a macro construct and process it alongside macros (the original idea)
2. Think of #{...}# as a rendering construct, and process it after verbatim. The bad thing about this is that any macros inside a #{...}# would be fully expanded (at possibly high cost)
3. We go with some variant of Jozef's comment disabler (but what scope does it have?)

Given the rarity of balanced #{...}# in verbatim or literal constructs, my preference is strongly for 1. Yes, we might break the occasional patch, but there's a ton of other ways to do that already.

Further thought; if we supported both comment syntaxes #{...}# and %{...}% they could be used to comment each-other out, which might solve the patch problem.

-- Main.CrawfordCurrie - 11 Jan 2016 - 08:39

Just one plea: don't break WikiWorkbenchContrib and apps built with it. View and edit templates are using this construct a lot:

%{<verbatim class="tml">}%
%TMPL:DEF{"foobar"}%
...
%TMPL:END%
%{</verbatim>}%

This makes sure that TMPL:DEFs aren't executed when viewing them online while still being fully functional when used as view template.

Btw. I wouldn't waste much effort in implementing nested comment blocks as that's not existing in other programming languages either. It adds confusion to an otherwise clear spec. Back to earth, Major Tom.

-- MichaelDaum - 11 Jan 2016

If someone want test it - I extended my hack with the paired #{ syntax }#. Tested it on the topic: Sandbox.NewCommentsTest and it Does What I Mean - e.g. the commented out macros aren't executed. (nor the INCLUDE nor the SEARCH).

diff --git a/core/lib/Foswiki.pm b/core/lib/Foswiki.pm
index 2da5a0d..f59d674 100644
--- a/core/lib/Foswiki.pm
+++ b/core/lib/Foswiki.pm
@@ -3112,6 +3112,9 @@ sub innerExpandMacros {
         WEB   => $webContext
     );
 
+    $$text =~ s/%# .*?\n//gs; #MYHACK# comments
+    $$text =~ s/#\{[\s\S]*?\}#//gs; #MYHACK# comments
+
     # Escape ' !%VARIABLE%'
     $$text =~ s/(?<=[\s\(\.])!%($regex{tagNameRegex})/&#37;$1/g;
 

-- JozefMojzis - 11 Jan 2016

Ah, the easiest thing in the world to say - "If someone wants to test it". Code is easy, test is hard. It's not a solution until you've proven it. So write some tests.

Anyway, I see your code is pretty close to what I already have in the branch (not pushed yet, will do so soon). I just don't agree about where the best place to do this is.

-- Main.CrawfordCurrie - 11 Jan 2016 - 13:15

Crawford, if i would like to be an "official" developer i simply will fill the IWouldLikeToCheckin ... But i'm still not filled it, because i think im not enough skilled. The both above is only as my own "view" to the solution. Try it or leave as it is - it doesn't matter. As i told - I'm using it, locally.

I would be happy to do some manual testing of any new FW feature - e.g. to the new comments too. That's what i can do (somewhat) - of course, it isn't any rocket science.

-- JozefMojzis - 11 Jan 2016

JozefMojzis, as far as becoming a developer:

• Everyone has to start somewhere, even if it's fixing an occasional typo, so developers are very welcome
• We all occasionally have to revert (or have reverted) something we checked in. So don't be afraid to contribute. Worst you'll encounter is a git revert, but usually we try to improve not discard.
• So ... go for it.

-- GeorgeClark - 11 Jan 2016

Crawford, is this ready to merge? Can you set a commitment date to start the timer for 2.2? Not that we have a target date yet, but this is a feature that looks like it's ready to go.

-- GeorgeClark - 12 Nov 2016

Earlier today I was reviewing an App I'd created where I had added within a SEARCH macro remark="Some useful comment"

That is to say that any unrecognized attribute is ignored. This effectively achieves a lot of what this proposal is about. I suppose that formally suggesting that some attribute name (e.g. remark) is reserved (by convention) in a macro to indicate comments.

A further variation to that would be create a preference variable

• Set REM =

Then you could %REM{Whatever you like}%

Implementation could be no more that setting REM in DefaultPreferences.

What's proposed here is undoubtedly a more complete idea/implementation, but I thought I'd raise the thought.

-- JulianLevens - 17 Nov 2016

Crawford, This has been stuck for a while. Can you set a DateOfCommitment so we can start the timer for 2.2. Your task / Item branch looks ready to merge.

-- GeorgeClark - 02 Feb 2017

Commenting is disabled while not logged in