Feature Proposal: Add takeOut / putBack calls to Func

Motivation

Many plugins break encapsulation by calling the Foswiki::takeOut & Foswiki::putBack functions. These functions are very useful and it's long overdue that these be formalized.

Also, in 2.0.3, putBackBlocks was changed to improve performance. It would be good if the extensions that didn't break the API had a way to take advantage of this.

Description and Documentation

Expose the existing APIs

• Foswiki::Func::takeOutBlocks -> Foswiki::takeOutBlocks()
• Foswiki::Func::putBackBlocks -> Foswiki::putBackBlocks()

Needs some further consideration for the corresponding Render functions Foswiki::Render::takeOutProtected and putBackProtected

Examples

Quick search of a subset of the extensions found:

• FindElsewherePlugin/lib/Foswiki/Plugins/FindElsewherePlugin/Core.pm
• WysiwygPlugin/lib/Foswiki/Plugins/WysiwygPlugin/TML2HTML.pm
• ExplicitNumberingPlugin/lib/Foswiki/Plugins/ExplicitNumberingPlugin.pm
• ControlWikiWordPlugin/lib/Foswiki/Plugins/ControlWikiWordPlugin.pm
• SolrPlugin/lib/Foswiki/Plugins/SolrPlugin/Base.pm
• SolrPlugin/lib/Foswiki/Plugins/SolrPlugin/Index.pm
• EditChapterPlugin/lib/Foswiki/Plugins/EditChapterPlugin/Core.pm
• DBCachePlugin/lib/Foswiki/Plugins/DBCachePlugin/Core.pm
• ImagePlugin/lib/Foswiki/Plugins/ImagePlugin.pm

Of them, it appears that only ! FindElsewherePlugin implements its own method. The rest use Foswiki:: functions.

And for Foswiki::Render::_takeOutProtected, ControlWikiwordPlugin uses that one. The primary differece between the two functions:

• takeOutProtected is passed a regular expression instead of a tag name. It's able to remove arbitrary blocks of matching text.
• putBackBlocks provides a replacement tag name. It's able to remove <verbatim> blocks and put them back as <pre> blocks.

Impact

WhatDoesItAffect:

%WHATDOESITAFFECT%

edit

Implementation

-- Contributors: GeorgeClark - 13 Nov 2015

Discussion

+1

-- MichaelDaum - 13 Nov 2015

Pushing this out to 2.2. The API needs some discussion, and there is no pressing reason to rush it into 2.1.

-- GeorgeClark - 06 Dec 2015

It has struck me for some time that both Foswiki::Func and Foswiki::Meta are rather unwieldy and crufty, both are growing to nearly 4000 lines long.

Often in discussions we talk about having clear separation of concerns in our code — well these don't qualify in my book.

Therefore, I've been wondering about creating an API directory under Foswiki to give us Foswiki::API::whatever to give us a chance of gaining this separation.

I say chance because although Foswiki::API::RenderBlocks is a little better then adding to Foswiki::Func; the real benefits will only accrue by taking extra care in the design of what we expose there. The actual code will, in most cases, only be a veneer of some sort.

I am aware that while ideally, for this particular case, Foswiki::Render would be completely redesigned and written from the ground up to offer this as a very clean API. However, as Render is quite a large ocean we have to take smaller steps forward.

The Foswiki sub-directory could alternatively be called Func rather than API of course.

Are there not multiple APIs in Foswiki-space? Have we defined all of them? I'm assuming here that we mean plugin API.

As for design specifics, because we might actually redesign Render, should we even add takeOut and putBack at all to the API? That is, will allowing plugins to takeOut and putBack blocks hinder a potential redesign. If not, then sure add this capability. However, what is the underlying problem being solved? Aren't taking-out and putting-back the verbs describing the current implementation?

-- JulianLevens - 10 Dec 2015

I'm not sure if my idea really fits into this discussion but I'd give it a try before filling in a new proposal.

What I clearly see here for the moment is a desperate need to shift Foswiki internal towards pure and structured OO model. Say, if a plugin is somehow turned into an object (the exact way is irrelevant) and a document it's working with is a full-grown object too then we may have it look like:

$core = $plugin->core;
$core->wikiToUserName();
$req = $plugin->session->request();
foreach my $attachment ( $doc->attachments ) { $attachment->name };
$doc->meta->getKey( $type, $key );

Isn't it more accurate than a code full of Foswiki::Func::someFunc(...) or even Foswiki::API::whatever::someOtherFunc(...)?

I'm not proposing any solution here yet. But this subject looks somewhat related to what I bear in my mind. If it's of any interest than I would come up with a proposal and some roadmap.

-- VadimBelman - 10 Dec 2015

I'm not sure I really want to turn this proposal into a redesign Foswiki::Func, or redesign Foswiki::* object design.

The purpose as I understand it, of moving things into Func, is that it allows a layer of separation between Foswiki internals, and functions used commonly across plugins. By doing that it gives Foswiki some freedom to refactor / redesign internals without needing to break the extensions using those pieces.

These four calls - takeOut / putBack Blocks and Protected, are simply utility functions used to move segments of "stuff" out of the way, in a large block of arbitrary text, and put them back later. Yes it's critical currently to how we render and process macros, but fundamentally these are pretty simple, useful utility functions that would probably have some usefulness regardless of the internal design / redesign.

By not doing this, we make it harder in the future to redesign the foswiki internals. The actual list of official "APIs" is in DevelopingPlugins. When a fairly significant list of extensions are calling functions outside of this list, then what's official (and intended to remain stable / subject to our deprecation policies) needs to be evaluated and possibly extended.

In thinking about my proposal, I'm leaning toward a "named parameter" style of calling that covers both functions:

Foswiki::Render::_takeOutProtected( \$text, $re, $id, \%map ) -> $text
Foswiki::Render::_putBackProtected( \$text, $id, \%map, $callback ) -> $text
Foswiki::takeOutBlocks( \$text, $tag, \%map ) -> $text
Foswiki::putBackBlocks( \$text, \%map, $tag, $newtag, $callBack ) -> $text

Replaced by the following API

Func::takeOut( \$text, \%map
     { tag => $tag,           # The HTML tag to be removed.  
       re => $re,             #  .. or Regular Expression matching arbitrary block to be removed.
       id => $id.             # identifier of block type removed - defaults to tag,  required for re.
     })

Func::putBack( \$text. \%map ) -> $text
      tag      => $tag,       # The HTML tag to be restored
      id       => $id,        # .. or the ID of the regular expression removed.
      callback => $callback,  # Callback to process the text before it's restored
      newtag   => $newtag,    # Replacement tag when tag is used. 
    }

-- GeorgeClark - 11 Dec 2015

Whoops - in addition to DevelopingPlugins,. there is also the PublishedAPI. With inconsistencies.

-- GeorgeClark - 11 Dec 2015

The official API IMHO is the one we refer to from the Reference Manual that we distribute with Foswiki. And that is DevelopingPlugins in which we have a section called "APIs available to Extensions"

I think George's proposal got a bit hijacked. It is a good proposal.

-- KennethLavrsen - 06 Jan 2016

I agree with JulianLevens points. The takeOut and putBack methods are too low level to be part of the API. They're exactly bonded with the implementation of the rendering. One of the purposes of any API is hide the implementation details.

Adding them into the API will hinder the potential rendering redesign. E.g. if someone will write an better renderer, for example compiling the whole topic into abstract syntax tree first ( AST) then the implementation details could (and probably will) change. And the plugins which currently breaks the encapsulation should be modified.

Most of modern software designs hides the implementation details using method calls. Unfortunately, this isn;t a case of the FW - here are many decisions which slows down the changes in the FW development. For example: FW in 2008 deprecated many method calls in favour of using global variables - which me considering as a wrong approach. E.g. instead of the universal method call getMainWebname( ) (which truly hides the implementation details) now the developer must use the exposed configuration global variable $Foswiki::cfg{UsersWebName}, and even more, in 2008 the developer musty use one another global variable $Foswiki::regex. This couldn't be considered as an good approach which hides the implementation details. Of course, this last paragraph has nothing with this proposal - me using it only as example why is better hide the details.

So, IMHO, such functions should not be in the API - they're implementation dependent. We really need read JulianLevens comments many times - it contains many important thoughts.

-- JozefMojzis - 19 Jan 2016

Removing this from the 2.2 Release Plan. No big benefit for adding this right now.

-- GeorgeClark - 14 Dec 2017

Commenting is disabled while not logged in