Item2382: Multiple FOREACH Macros

I'm not sure how else to rasie this issue except as a bug.

I've seen multiple references to refactoring SEARCH to use a %FOREACH macro for results. I understand %FOREACH was suggested instead of %FORMATLIST to prevent an impact with the FilterPlugin.

However, I have seen no concern raised about the ForEachPlugin. We use this macro and I can see trouble ahead.

Thanks

-- JulianLevens - 19 Nov 2009

---

VOTE FOR NEW NAME

Macro	Votes (Enter your names comma separated)	Comment
none	MichaelDaum	remove from current feature set for foswiki 1.1

---

yup.

-- SvenDowideit - 20 Nov 2009

I'm still unsure how best to deal with this - I fear without suggestions it'll just be an annoying fact

-- SvenDowideit - 30 May 2010

According to KennethLavrsen in ExtractAndCentralizeFormattingRefactor

• The ForEachPlugin was part of the discussion that led to FOREACH as the macro as the feature very much overlap.
• At least this is how I recall it. The discussion was on IRC and in some other topics here in Development web.

However, my limited knowledge still leaves me uncertain about this. After all the FOREACH plugin cannot register the FOREACH macro if it's already part of core — or can it? Even if it could, would that mean that the core would only call the plugin when it doesn't cannot handle the other parameters.

Alternatively, merge the FOREACH plugin into core as well — but I cannot speak for the impact/implications of that idea.

-- JulianLevens - 30 May 2010

ForEachPlugin uses the commonTagsHandler, whereas (I assume) Foswiki 1.1's core %FOREACH% does not.

The %FOREACH% aims to avoid having every plugin re-implement header, format, footer over and over again with ever-so-slightly different $tokens and so-on; whereas ForEachPlugin takes a different route entirely (hence the need to be a commonTagsHandler macro).

I am wondering if we can't update ForEachPlugin to alias itself if used in a post-1.0 Foswiki. This would still require effort to users upgrading to Foswiki1.1 who have invested in ForEachPlugin's %FOREACH% macro, but at least should be as simple as %s/FOREACH/FOREACH_.

Apparently, it has already been decided that this new core feature will be known as FOREACH.

-- PaulHarvey - 31 May 2010

This should probably be a release blocker, for the sake of those sites that are upgrading and depend on ForEachPlugin. See Item9449.

As discussed with Sven on IRC we just need to make the native FOREACH check for the presence of parameters unique to the ForEachPlugin's, and then do nothing (fall back to the plugin to handle the macro).

Doesn't need to block a beta release

-- PaulHarvey - 12 Aug 2010

I was thinking that detection really could be as simple as looking to see if ForEachPlugin's params are in use, and if so, to leave it alone, or to chain to the plugin's..

alternativly, this is a place where we could consider adding chains of marco handlers - with the marco handler registering its params for detection

i'm no suggesting doing any of that now tho.

-- SvenDowideit - 16 Aug 2010

I've never been totally happy with the naming of the core FOREACH macro, and on re-reading the documentation I'm even unhappier. FOREACH is such a loaded term aong programmers that they will expect the macro to be much more capable than it is. A better name would have been RENDERLIST or FORMATLIST (which both describe what it does). Yes, I know the longer term goal is to have it work on result sets, but the role of the macro is to produce formatted text output when supplied with a list. FOREACH is just the wrong name, and I don't think we should release with it. Note that RENDERLIST is already camped on by the RenderListPlugin (a bad plugin), which rules it out.

Whatever we call it, we want to get across the concept that this macro "takes a list, renders/formats it in a new way, and spits it out again". We use the parameter "format" in many places to reflect this kind of processing, and we are processing a list.

Brain dump of candidate names:

Candidate	Pros	Cons
FOREACH	Already coded	Conflicts with ForEachPlugin, too generic, not intention-revealing
RENDERLIST	Intention-revealing	Conflicts with RenderListPlugin
FORMATLIST	Intention-revealing, probably the best choice	Conflicts with FilterPlugin
EACH	Simple	Still wrong, doesn't describe what it does
LIST		Far too vague
FORMAT	Simple	Far too generic, not intention revealing
RENDER	Simple	Far too generic, not intention revealing
MAP	Simple	Too generic, too geeky
MAKELIST		Not descriptive enough
FILTERLIST		It's not a filter, it's an 'each'
FOR		As bad as FOREACH, will probably conflict with something
LOOP
LOOPOVER
EACHITEM

Another approach would be to bite the bullet and import the FORMATLIST code from the FilterPlugin. However I feel this could only be done without $map and without importing any of the other macros from this plugin, which would introduce an incompatibility with existing wikiapps that use FilterPlugin. The doc would also need an extensive rewrite, and unit tests would have to be developed. This workload probably rules this approach out at this late stage.

-- CrawfordCurrie - 20 Aug 2010

Some other names for people to comment on.

Candidate	Pros	Cons
FORMATEACHRESULT	Explicit intention?	Too long a name?
FORMATEACH	Explicit intention?	?
FORMATRESULT	Explicit intention?	?

-- JulianLevens - 20 Aug 2010

How about APPLYFORMAT ?

-- MichaelTempest - 20 Aug 2010

which is why I called the macro FORMAT - inline with the format which it is essentially an extraction of. Similarly, my intention is to provide a format class that can be reused by all macro's...

so I still suggest FORMAT is the most consistent.

-- SvenDowideit - 20 Aug 2010

See also http://foswiki.org/Development/AddFilterPlugin.

Sven, it was you who suggested the name change, AFAICT: http://irclogs.foswiki.org/bin/irclogger_log/foswiki?date=2009-06-10,Wed&sel=130#l126

-- CrawfordCurrie - 20 Aug 2010

FORMATEACH is my favourite so far. I like it..... but... can't somebody go back in a time machine to 6 months ago and do it then instead of now ? ...

-- PaulHarvey - 20 Aug 2010

/me reading FORMA TEACH

Other candidate TRAVERSE: result sets might not remain a linear list in the future. For now it traverses a linear list. Later it might traverse a tree as well, e.g. webs or a soap response.

-- MichaelDaum - 20 Aug 2010

I support that we change name to avoid clash with plugin. Original decision was assuming that old plugin would be obsolete. We are more clever now, so lets us rename the macro.

I will support any macro. The suggested names are all OK for me.

-- KennethLavrsen - 21 Aug 2010

TRAVERSE is good too. In fact, if it can be taught how to follow a tree represented as a newick string I could do lots more interesting with Foswiki Or trees/graphs of topic nodes are good too...

-- PaulHarvey - 21 Aug 2010

TRAVERSE is too good a name to waste on a macro that just iterates over a list in a string, and not even as intelligently as FORMATLIST.

Unless I'm shouted down, I'm going to change it to RELIST (inspiring, huh? "re-build this list")

%RELIST{"a,b,c" format="   *$item$n"}%

-- CrawfordCurrie - 21 Aug 2010

Nobody shouted, so RELIST it is.

-- CrawfordCurrie - 23 Aug 2010

I SHOUT. RELIST is aweful.

Admitted the current implementation of what is/was FOREACH is rather limitted and inherits all the quirks from %SEARCH that we'd actually like to avoid at some point. I agree that calling it TRAVERSE sounds like painting a Trabbie a Porsche.

But is this code worth any name, given you want to spare TRAVERSE for something else/unknown, "the real thing"?

Don't just invent YASSM - yet another silly sounding macro - and think we do fine.

-- MichaelDaum - 23 Aug 2010

I agree that RELIST is not so nice, sounds like an obscure acronym. I would like to throw in a couple of other suggestions:

• ITEMISE
• LISTITEMS
• ENUMERATE

-- ArthurClemens - 23 Aug 2010

I think I actually suggested changing in the irc link that Crawford links because he and I had been discussing it elsewhere

I also don't like RELIST - ENUMERATE is better. Please, don't focus on its current feature set, and consider that the MACRO name should lead into its future. TRAVERSE isn't bad either - basically, its destined to become a way to iterate over stuff - right now a list, or a topic=Task*, and later, i would expect, TML elemets - QUERY style.

BUT when it comes down to it this macro's purpose is to allow the control of the rendering output of an (ordered?) set - the _DEFAULT and topic/web/topicexclude etc are there as shorcuts to what will become resultsets - which is why it started as FORMAT - we're only limited to 'lists' atm - this will change.

-- SvenDowideit - 24 Aug 2010

I see quite some support for TRAVERSE. However, I am sure we can't make it what it promisses in an evolutionary code morphing way. It rather needed a clean design and nail it down.

The other option is to remove this code from 1.1 and implement a real TRAVERSE on real result sets targeting the next release. We simply don't have this feature in our current trunk for 1.1

I'd prefer not to ship a FOREACH/RELIST/TRAVERSE/FORMAT thingy right now. Instead we should design a proper TRAVERSE - TheRealThing - as that's what we all would prefer instead. We simply aren't there yet. This name-finding discussion makes that all too clear.

-- MichaelDaum - 24 Aug 2010

I think we're stuck in analysis paralysis here. I'm really glad I renamed the macro for what it is, because it has exposed that it isn't what any of us thought it was - including the original author. So, my gut is telling me that:

1. There is a role for a FORMATLIST macro a la FilterPlugin (indeed, this is how FOREACH/RELIST is currently used in the core code)
2. There is some higher purpose (TRAVERSE/ENUMERATE/ITEMISE) that is currently insufficiently specified.

So, given the looming release, here's what I propose:

Crawford's proposal WITHDRAWN - see below

1. Bite the bullet and import the FORMATLIST code from FilterPlugin with minimal changes (AFAICT the only shortfall is failure to support standard formatting tokens in the format). Deprecate FORMATLIST in FilterPlugin (only register the macro if the core impl isn't available)
2. Create a feature request that actually documents what we want FOREACH-on-steroids to do, with the target release 2.0.

I am happy to do the work to import the FORMATLIST from FilterPlugin. I will even write some of the many missing unit tests. However I refuse to play name tennis any more.

-- CrawfordCurrie - 25 Aug 2010

you say AFAICT the only shortfall is failure to support standard formatting tokens in the format when in fact the real shortfall is a total lack of unit tests?

added to which is that it doesn't use the long standing (and hated by Micha) formatting system established by tmwiki and foswiki - thus meaning that rather than being able to work towards unifying the formatting operations, we're moving to a new one, and having to support the old.

considering that the point of the code I did for the macro was to expose and enhance the maintainablilty of the currently existing code - basically to reduce the amount of code we need to support, and to be able to move towards things we don't yet know, I think both your proposals are doomed - but that is clearly just my opinion.

goal number 1 for the work I did is to reduce the amount of code, and to work towards refactoring the core, and eventually extensions, so taht there is one grand unified 'fantastic four' (header, format, footer, separator).

This then dovetails into the Set of TOM objects idea that resultsets leads to.

so imo, importing yet more dissimilar code into the release only makes our life harder and more depressing.

from my pov, we need to unify what we have before changing things damatically - otherwise we have more complexity to try to reunify - and given that extremely few people do this level of refactoring, and even fewer write enough tests to know what works, I clearly strongly dislike the aledged quick fix of adding a plugin - even more so at this late stage.

Given that all the other plugins we've added in 1.1 are considered partial improvements, which there seems to be concensus that they need re-writing to properly integrate into the core, I argue that adding even more technical deficit is bad.

-- SvenDowideit - 25 Aug 2010

For sure there is no way we change any more code other than fixing bugs - also for FOREACH/RELIST. No new plugins. No more functionality to the feature for 1.1.

We need to define a macro that does not clash with existing plugins. The only thing to fix is to avoid name clash.

Personally I cannot react or respond to wet dreams of what the macro can and will do in the future on a feature proposal not yet written.

Just get the list of names you care for on a list and let us vote for them and get it over with, because none of you are grouping around specific names right now. Someone is not going to be happy after the result is known but that it just too bad. You cannot get everything in this world settled by consensus and it is not exact science. There is no right or wrong. It is feelings and taste. so let us vote.

The best way would be that each of you add ONE favorite proposed macro to the vote I just added near the top. And when we have the list we vote by adding our name in the 2nd column. If not a single macro ends up with a clear majority we may vote again between the two with most votes to get a majority vote. Let is see how it goes first. But max ONE proposed macro from each of you and only criteria is that it is not used by any existing plugin.

Timeline

• Until 26 Aug 2010 - 12:00 GMT - add proposals
• From 26 Aug 2010 12:00 GMT to 28 Aug 2010 12:00 GMT - vote
• From 28 Aug 2010 12:00 GMT to 30 Aug 2010 12:00 GMT - optional 2nd vote if 1st vote does not produce a clear winner. Kenneth decides if needed based on protests against 1st vote.

I will manage the vote process but I will not vote myself so noone will think that I try to manipulate the process to promote my favorite. OK with everyone?

-- KennethLavrsen - 25 Aug 2010

There is a deeper issue here, and simply voting on the name isn't the right approach. There are many subtleties. I eventually trawled for all the topics in Development that relate to this discussion, and reviewed what I could find in IRC. Here are what I think are the key references:

• AddFilterPlugin - comparison of functionality between FORMATLIST and FOREACH
• SearchResultsPagination - issues related to formatting SEARCH output
• SettingAndGettingVariablesUsingMacros - the requirement for FORMATLIST capability in the core

I'll try to summarise:

1. Sven is trying to introduce the concept of "lists of objects". FOREACH/RELIST is intended for formatting such lists. It was not implemented as a general purpose string list formatter until I added that capability (type="string" was subsequently recoded by SD) - but this is ok-ish, because "a string" is a valid "object". However it makes a bad string processor, because it lacks many of the very desireable features that FORMATLIST offers.
2. An "object view" of the world is coming, but it's still a long way off. Until it's there, we require some way to reformat a generic string list of text items. SSP cannot do the job, due to brain-dead newline handling. FORMATLIST is a general purpose list reformatter. However it is in FilterPlugin which cannot be added to core, due to functionality bloat and a total absence of unit tests.
3. The feature set of the two macros now overlaps, with FORMATLIST being much better for manipulating strings (http://trunk.foswiki.org/Development/AddFilterPlugin) while FOREACH/RELIST is much more SEARCH-like, reflecting its role in formatting lists of topics.

Having considered the pros and cons, I'm of the opinion that there are two sensible ways ahead:

Alternative 1

1. Bite the bullet and import just the FORMATLIST code from FilterPlugin with minimal changes (AFAICT the only shortfall is failure to support standard formatting tokens in the format). Deprecate FORMATLIST in FilterPlugin (only register the macro if the core impl isn't available)
2. Rename RELIST reflecting all the considerations above (this could be the subject of a vote), and document it as being specific to formatting object lists. Remove the type="string" mode which overlaps FORMATLIST, and cross-document the macros.
   • RELIST formats objects - it is not a general string list formatter, and it was a mistake to give it that role.

Alternative 2

1. Live the current limited functionality of the FOREACH/RELIST macro, and plan to import the FORMATLIST capabilities for handling string lists in a future release.
2. Rename the macro reflecting all the considerations I listed above (this could be the subject of a vote)

Alternative 3

1. Fix SSP so it can handle lists made up of items that contain newlines.
   • I wouldn't know where to start.

So this is much more that name tennis. The problem is that handling lists of objects is the right way to go longer term, but in the short term it doesn't cut the mustard. On the flip side of the coin, importing FORMATLIST capability raises the maintenance/support burden.

Whatever we do, it is critical that a feature request is written to reflect the "lists of objects" view of the world. Failure to communicate this clearly will simply create more pain.

-- CrawfordCurrie - 25 Aug 2010

Alternative 4

1. remove/hide/disable the FOREACH/RELIST as they are now
2. design proper result sets for 2.0

As it is right now FOREACH/RELIST creates more problems than it solves. I wasn't aware of the type=string option. This all is going into the wrong direction.

FORMATLIST from FilterPlugin is bullet proven by tons of wiki-apps. I couldn't do my job without. We can't add it to 1.1 because we are on feature freeze.

Still - compared to the wet deam (I like that) of real result sets - it is plain wrong to rely on strings being parsed into lists back and forth repeatedly. There are a lot of situations where lists and tables are created on the fly just for the purpose of reformating them later on. That's a waste of resources and error prone. Instead, these structured objects should be stored and refered to internally using an ID. This is related to the proposed %SET and %GET macros for 2.0.

FOREACH/RELIST being designed as a pure topic iterator is a real shortcoming in design, besides the lack of processing capabilities and this awful type parameter. Result sets are meant to be polymorph, so no type parameter is needed.

FOREACH/RELIST's formating capabilites are a code reuse of what %SEARCH does. This too is the wrong thing to do! Instead it should implement what we informally agreed on wrt our Fantastic Four (header, format, separator, footer). They are implemented wrong in %SEARCH and we can't change that due to legacy issues. A FOREACH/RELIST/FORMAT/FORMATLIST/TRAVERSE thingy should be the oportunity to do it right once and forever right from the start. No more core morphing please. The current code in Search.pm is next to unmanageable.

So let %SEARCH itself use the current formatResult() code full of legacy wrinkles, and let's implement TheRealThing ... next development cycle.

-- MichaelDaum - 25 Aug 2010

We are not going to remove the FOREACH/RELIST feature now. We are many that look forward to it just the way it works and is implemented now.

And Michael. You are not correctky informed about FOREACH/RELIST. I took great care when I introduced the compatibility mode for separator for SEARCH.

• SEARCH will not add separator after the header if separator is defined. Just like we all wanted it
• SEARCH will add a new line after header of no separator is defined. This is the compatibility compromise that will prevent 100000 formatted searches that builds TML tables from breaking.
• FOREACH/RELIST will NEVER add a separator after header.

The two macros uses the same formatting code.

But when SEARCH calls the common code without a separator, the variable that holds the separator remains undefined and this triggers the compatibility mode and adds the default NL after header.

When RELIST calls the same common code without a separator, the same separator variabkle is assigned a defined value which is a new line. And then the common formatting code will not add the new line after header.

So the fantastic 4 works as they should for the FOREACH/RELIST feature with no compatibility mode to polute the feature.

-- KennethLavrsen - 25 Aug 2010

My priorities are:

1. Get 1.1 out the door
2. Enable this feature

I love FilterPlugin's FORMATLIST. I use it in so much of my own work. But it is way too regex/perl-tastic. We want somebody who can cook a SEARCH to be able to use FORMAT/FOREACH/THISTHING.

To be honest, I'd settle for most of the names suggested.

I like FORMAT, FORMATEACH, FOREACH... in the case of FOREACH, I still think fall-back to ForEachPlugin if we detect ForEachPlugin params is probably not as scary as we think it might be (ie. a valid way forward).

-- PaulHarvey - 25 Aug 2010

The issue I have with any "fall back" thing is maintenance. We've done this sort of thing before, and it has rotted because of inadequate unit tests, or because the target moved on (e.g. if someone recodes ForEachPlugin)

Michael is right in most senses, but he's taking an "all or nothing" approach that just doesn't move us forward. So despite the wrinkles, I think the FORMAT approach is the right one. I', not going to argue the detailed pros and cons; just state that I think what we have now is the best we can have - now. Anyone with a vested interest will hopefully be motivated a bit more to contribute to the future direction.

-- CrawfordCurrie - 25 Aug 2010

FORMAT it it.

-- CrawfordCurrie - 25 Aug 2010

While renaming to FORMAT, CDot introduced the type="string" marker for FORMAT. While this is correct, he put it into the ENCODE part, which caused the HTMLValidation tests to fail (and most likely a lot of other stuff as preferences were no longer encoded).

While fixing, CDot noticed the iterator was wrongly named $topic. As these are no topics, I've changed this back.

-- OlivierRaginel - 26 Aug 2010

As a remark on the new name FORMAT: a lot of wiki apps on my client's intranet now broke, that is those that used FORMAT in parametrized INCLUDEs. IMHO, the macro name FORMAT is too generic and not descriptive enough to distinguish it from other things. Might be I am not the only one getting bitten by this.

It would be better to have called it FORMATRESULT instead. That's less generic and expresses more precisely what this thing is doing. Thus there's less of a chance it clashes.

Is there still time to change the name once again?

-- MichaelDaum - 30 Aug 2010

mmm, I would vote against Michael's request for at least 2 reasons -

1. i believe that it makes alot of sense that the core macros use generic names, as it is after all the core product.
2. we regularly have complaints (iirc even from Micha?) that there is too much typing as it is - so going longer in the core is a bit naf.
3. oh, one more? I have used FORMATRESULT in some of my client sites - ok, so tbh, i think this argument is a bit spurious anyway :/

-- SvenDowideit - 31 Aug 2010

I had hoped we had closed this. Micha, I'm sorry to hear that your apps broke. However IMHO %FORMAT% is not too generic, it's a good name, though it took me a while to understand why. The risk of clashing with an include parameter is high with any of the suggestions, and I don't favour another change unless it's to a significantly better name that no-one has suggested yet.

-- CrawfordCurrie - 31 Aug 2010

I also believe we should stay with FORMAT. It is no more in danger of name clash than TAB and BUTTON that also is included in 1.1.

And in parametrized INCLUDEs compatibility is not really an issue.

Foswiki is so nicely made that if you define a macro in an INCLUDE this definition overrides any core or plugin definition of the same macro. I use this when I include a topic where I want EDITTABLE disabled.

So the users Michael have will not see the difference when upgrading to 1.1.

Example (see in trunk) where we include a section defined in this topic setting the value of FORMAT to HELLO. And we see a nice HELLO as a result.

• This is Hello

Here is a section to include where the HELLO is shown as no value in trunk and the raw %FORMAT in 1.0.9.

• This is

Sections ends here.

Naturally you cannot use FORMAT in the new meaning in the included topic. But if you want to change the included topic and start using FORMAT in it, you can also change the name of the parameter now that you are editing anyway

So with no real problem in practical I want to stick to FORMAT.

I do not see a community will to change this again, especially now that I have demonstrated that there is no real issue. Setting back to Closed since I cannot see this leading to anywhere. We had a lengthly discussion and Michael refused to discuss the name and argued against democracy, so I am not willing to drive this decision for the 3rd time. We have to draw a line in the sand now. I tried to organise a vote which was also refused. I hope you can all learn from this that open debate, time to hear each others arguments, and in case of no consensus let the vote speak as a final resort is the way to do things in future. Otherwise we end up reopening the same discussions again and again.

-- KennethLavrsen - 31 Aug 2010