Feature Proposal: FilterPlugin is such a useful thing, it should be part of the default package
Motivation
Simplify and standardise list formatting. Let people assume the support is present. Escape from Spread<nop>ShitPlugin a bit more.
Description and Documentation
FilterPlugin pops up time and again for formatting. It's so much nicer to use than CALC. It's a simple plugin that deserves to be in the main package.
Examples
Impact
Implementation
I will happily impose the standards on the plugin and do the work to add it to the standard set.
--
Contributors: CrawfordCurrie - 20 Jul 2009
Discussion
I guess the only additional thing is to consider the overlap with the trunk FOREACH which is coming out of the SEARCH refactor - mostly to avoid confusion if there are subtle differences.
--
SvenDowideit - 20 Jul 2009
I have several reservations against this plugin that will have to be settled.
- As Sven already pointed out the plugin has functional overlap with core features in 1.1 (FOREACH) in a way that is directly confusing.
- The Plugin has a ContactAuthorFirst policy. We cannot have in the core code / default plugin set - extensions that the core developers is not allowed to touch. It could block releasing Foswiki in periods where the 1 (or 2) core developers do not have the time to maintain the extension. We cannot allow PleaseFeelFreeToModify either. All the default plugins have the FollowsReleaseProcess policy which is both more restrictive (you cannot modify features without agreed feature proposal per release process), and more free (any core developer have the right to fix bugs and implement agreed features). The FollowsReleaseProcess extensions still have authors that holds the copyright and per tradition maintains the extension but the maintenance right and responsibility lies in the core development team. This is a MUST. Otherwise we will for sure run into trouble.
To overcome these two problems.
- The plugin can be altered so that the overlap is avoided.
- The plugin can have its policy changed to FollowsReleaseProcess. The original author has every right to refuse this and I will personally not argue against this should he refuse because I can perfectly well understand if he would not want the community to refuse that he extend the extension - or that the community alters the existing features.
- ALTERNATIVELY - we could simply merge the desired features into core. The features are so neat that I see no reason why you would have to be able to turn them off.
I think it would be best for the community AND for the author of
FilterPlugin if we took one of the alternative routes.
If I was Michael I would not want an extension that I use and develop and maintain for my clients to suddenly be cripled and feature controlled. It is an extension that invites to be extended with more features and if I was Michael I would not want to wait and ask and beg each time I wanted to extend it.
Crawford you did not put a
DateOfCommitment so the 14-day rule does not apply yet (because the application does not pick it up unless there is both a committed dev and date to base the 14 day rule on)
I didn't because I knew this discussion needed to take place. At the end of the day I'm not likely to be the dev, except if there is a need to create a plugin to retro-port the features to earlier foswikis.
--
KennethLavrsen - 22 Jul 2009
I'd prefer
not to ship FilterPlugin with the core. Instead it makes more sense to copy over the interesting parts of FilterPlugin over
into the core and leave out the more exotic ones. Not sure what the others think:
Experience has shown that SUBST and EXTRACT are used
very rarely. And if they are used, coming up with a correct regular expression is extremely hard and tough to debug.
I think the above selection is in line with the intended features to be available by default. If we add FORMATLIST and MAKEINDEX to the core I will remove them from the plugin leaving behind the SUBST and EXTRACT macros.
However, the FOREACH vs. FORMATLIST macros need a close look and comparison. Sven, can we create a side-by-side comparison of FOREACH and FORMATLIST? I have no idea what FOREACH is.
Maybe FOREACH is something completely different as it is part of the result set initiative.
Features of FOREACH
- input: a list of items (and in 2.0 a ResultSet)
- output: via format, header, footer, separator
- list manipulation using sort, limit, skip, revers, paging (once Will gets the unit tests written :/ )
- filtering via exclude, include
- will be provided as a Func:: API method in 2.0 for use in all plugins (much more work will be done by then)
- input: a list of items
- output: via format, header, footer, separator
- list decomposition: via split parameter
- list item decomposition: via pattern parameter, grouping of sub matches using brackets and referenced buy $1, $2, ...
- list manipulation using sort, limit, skip, reverse
- filtering via exclude, include and unique parameters
- selection & marker a la WEBLIST
Examples for FORMATLIST:
--
MichaelDaum - 22 Jul 2009
excellent idea - though
FOREACH
is a work in progress, as its an extraction of the existing
SEARCH
outputting functionality.
the missing bits should be pretty easy to add, and are similar to what I had in mind - as you say, the filter goodness should go into the core.
I have a set of unit tests that i need to continue to work on - I found some incompatibilities between the trunk code and tmwiki/foswiki 1.0 SEARCH output that have been holding me up.
--
SvenDowideit - 22 Jul 2009
OK, so it sounds like there is emerging convergence on the feature set required. Hopefully this process has also helped bring FOREACH into the open a bit more - I was also unaware of how potentially powerful it was planned to be.
--
CrawfordCurrie - 22 Jul 2009
From what I read we are about to merge FOREACH and FORMATLIST while this gets added to the core? So FOREACH for 1.1 is equal to FORMATLIST? Which problems might arise when merging the codebase?
We now know that they overlap. But where do these two macros differ mostly ... leaving ResultSet to the side for a moment?
There are a lot of apps n people already using FORMATLIST. Can we keep the name instead of renaming it to FOREACH which is not out in the wild yet?
--
MichaelDaum - 22 Jul 2009
we already renamed the macro from
FORMAT
to
FOREACH
, I don't see any problems renaming it again :).
the worry I have, is that its likely to not be 100% compatible, as the FOREACH rendering code
is the
SEARCH
rendering code, with some legacy poo included.
I propose that we don't rename now, but provide a configure switch to register the marco to FORMATLIST too - so that
if you need the
FilterPlugin version, the core doesn't make that impossible.
the main differences are that FOREACH isn't finished, and has more unit tests to work on - we need a set of unit tests for
FilterPlugin too.
--
SvenDowideit
I can see from the discussion above that the original proposal of including
FilterPlugin is not accepted.
Michael did not jump out shouting "yes change the plugin policy". And there are also reservations from several on spec as well and scope with respect to what we want to include in the core.
But we seem to have two conclusions to build on
- There is a desire to add functionality from FilterPlugin to core.
- Michael is OK to remove those merged features from FilterPlugin to avoid overlap and conflict. This is great Michael.
- We have a decision to make SOON on FOREACH and FORMATLIST.
I have a small concern on the impact of 1.1 release schedule. This could cost 2 months delay. Once a feature is in 1.1 we cannot remove it again. So from a release schedule point of view - we should consider if FOREACH and FORMATLIST should be merged now.
If we change FOREACH to FORMATLIST now without merging in all Michaels features, users of FilterPlugin cannot easily upgrade to 1.1. I would warn against that.
Then is may be better to accept that in 2.0 we both have FORMATLIST and FOREACH with some overlap in features.
--
KennethLavrsen - 23 Jul 2009
Keeping legacy from good ol' SEARCH in FOREACH is a no-go to
merge FORMATLIST and FOREACH. However, it actually might be quite okay to have
both just for that reason where one is operating in quirk mode so to say.
I am pretty sure that a configurable option that routes FOREACH to FORMATLIST or the other way around does not work out at all.
So as I see it we can "park" the legacy in FOREACH and use the more sensible formating algorithm in FORMATLIST. Upgrading FORMATLIST to take ResultSet IDs as FOREACH will do might not be a problem.
Bottom line: we can just import the FORMATLIST code as is and create the needed unit tests. I will keep the same features in
FilterPlugin for a while for older Foswikis but switch them off on new ones. There will be a sensitive deprecation timeline for them when they are finally removed from
FilterPlugin.
--
MichaelDaum - 23 Jul 2009
I recon that if you write the unit tests for
FilterPlugin you'll probably find that merging with the legacy SEARCH is
not impossible, and i really would like to
not keep both, as we then fail to move SEARCH into a maintainable tool (ignoring the reality that i've already spent weeks of work on it :/)
--
SvenDowideit - 23 Jul 2009
Hm, how could that be? Semantics of the Fantastic Four (header-footer-format-separator) are so different. Even limit, sort and reverse are fucked up in SEARCH for performance reasons.
--
MichaelDaum - 23 Jul 2009
I suggest that adding yet another fundamentally different 'Fantastic Four' into the core is a mistake that will lead to further confusion for users, and as the existing semantics of
SEARCH
, however flawed you claim them to be, are none the less vital and significant to most Foswiki users.
Making
SEARCH
a legacy code area is an un-necessary mistake that
I was working towards resolving by doing the
ExtractFormatting and
ResultSet refactor.
Rather than claiming without proof that its impossible, make a set of unit tests for
FilterPlugin's version, and we'll be able to see how impossible it is to bring that semantics into the existing core.
--
SvenDowideit - 24 Jul 2009
Well you've got your head deep down in SEARCH and I do know you suffer from this horrid code :/
The mistakes made there have not been repeated in
FilterPlugin nor in a couple of other plugins that use the FFs.
No need to repeat the discussion from
Tasks.Item1773.
--
MichaelDaum - 24 Jul 2009
Michael, I can't find where you have indicated how your solution is any different. All I can see from the documentation of FORMATLIST is yet another semantic for
separator
, that Aunt Tiffy has to learn. I can't find any mention of the specific problem case that
Item1773 is discussing viz. what happens between the
header
and the list items. Can you please break the 140 char sound-bite limit, and be a bit clearer, because at the moment all you are doing is winding people up.
--
CrawfordCurrie - 24 Jul 2009
Most macros that add header/footer/separator capabilities use them sort of like this:
return '' unless @result;
return $header.join($sep, @result).footer;
That's what they are meant to be. SEARCH does
not work like this and thats what
Tasks.Item1773 is about.
--
MichaelDaum - 24 Jul 2009
if the join issue is the
only incompatibility, then thats extremely minor, and thus not worth adding more weight to the core.
--
SvenDowideit - 07 Aug 2009
There's more. As far as I remember SEARCH has got a special way to sort+limit (a) within one web and (b) across webs. It first limits the result and then sorts it .... which is flawed. That sort of thing. Lots of constraints in the code tailored towards a file grep. I did not analyze your new code yet Sven, but I think you know what I mean.
--
MichaelDaum - 07 Aug 2009
which is exactly why I'm asking
you the
FilterPlugin expert, to write unit tests for the code you know best -
FilterPlugin. From those we can compare and analyse what can be done, and what is actually impossible.
--
SvenDowideit - 08 Aug 2009
Did you consider helping out on unit tests for
FilterPlugin? That would be great.
--
MichaelDaum - 10 Aug 2009
Yes, I considered spending my time writing tests for code I don't think is necessary, rather than one the rather large list of core changes I am working on for the 1.1 release.
but then I decided to work on the core instead.
I'm sorry I misunderstood Michael, I though you might like to help make the core
FOREACH
compaitble with your
FilterPlugin extension, to reduce the breath of code you maintain. I'm not stressed if it doesn't happen though, as I have always though improving the core in a compatible fashion is the most important task.
--
SvenDowideit - 10 Aug 2009
Understood. I know you are working hard on result sets and
FOREACH
guarding all those changes with unit tests. The issue at hand here is "just" adding
FORMATLIST
and
MAKEINDEX
to the core. So I would propose to
- (a) add this code to the core and deprecate their use via FilterPlugin
- (b) cover the newly added code with a sufficient amount of unit tests for it
and then see where we are. Judging from the discussion above there is not enough experience with this code as it is YAFP (yet another f*** plugin). All of your
own core changes are brand-new and I'd better give it a round in the open before putting yet another burden on
FOREACH
to be FilterPlugin-ish or the other way around.
--
MichaelDaum - 11 Aug 2009
FORMATLIST
and
MAKEINDEX
cannot be imported into the core without a high risk of introducing incompatibilities. Unit tests are required to (1) verify the spec in
FilterPlugin and (2) verify that the core code is compatible with that spec. Of course Sven (or anyone else) can create a new set of unit tests for those functions as they exist in the core - and hence unit test a new spec - but that is high risk for
FilterPlugin users, as there is no verification of compatibility.
--
CrawfordCurrie - 11 Aug 2009
That's what I said and what is my own main concern on this process.
--
MichaelDaum - 11 Aug 2009
Proposal withdrawn. Marking as rejected.
--
CrawfordCurrie - 25 Aug 2010