Item1773: SEARCH separator between last result and the footer is troublesome.

separator can't be used to separate results, as its also applied after the last?

the separator setting is used between header and the results in 1.0.4, and then in the 1.0.5 (or wherever footer was implemented, this was changed (and thus also in trunk)

imo we should probably change the code so that separator is used only between results, and that footer&header are responsible for containing whatever is needed (that way they can be used either way)

%!SEARCH{
    "(WebPreferences|WebStatistics|WebHome)$"
    type="regex"
    scope="topic"
    format="!$topic"
    separator=","
    header="---++ header !$web"
    footer="---++ footer !$web"
}%

header Tasks!DakarRevisedWebHome,!WebHome,!WebPreferences,!WebStatistics---++ footer Tasks

Number of topics: 4

-- SvenDowideit

Of course, a separator should only separate results.

-- ArthurClemens - 27 Jun 2009

Agreed. A separator between header and body, and body and footer, are both errors IMHO.

-- CrawfordCurrie - 28 Jun 2009

We are steering directly towards a MAJOR compatibility issue which will cause a lot of trouble for the users.

The footer I introduced recently is not the biggest issue. Not that many can have implented applications with these. I implemented the footer so it worked exactly the same way as the header.

The header is very old and you have been given examples in our documentation that suggestes that a newline after the header text is not needed.

Look at the documentation. Look at FormattedSearch.

Example: header="| *Topic:* | *Summary:* |"

Not a trace of a $n

And the examples - also in old versions of FormattedSearch we never indicated that you needed to put a new line in the header parameter.

So if you now remove the separator after the header, you are going to break quite a number of applications that typically are searches that result in a table with the header defining the table header.

Yes, it is easy to alter these by taking adding a $n or by not using the header feature and instead placing a table header before the search. But the issue is that administrators upgrading to 1.1 will have a major pain finding all these broken applications and fixing them.

I doubt many will have used the separator parameter in a way that they also want to use it for the header. I bet most that use the seperator either

• Set it to nothing because they need a different seperator between values than the header using a traling ", " (comma space) in the format and a $n in the header.
• Set it to for example ", " and then not use the header at all.

The issue is the default value for the separator. It is a new line character. And many many people will have defined a formatted search that returns a table. And the separator default provides the default newline between the header and the search result lines.

We cannot just change this and not care about our users. So we need to provide a reasonable new default behaviour.

The proposal I can come up with is to add a new headfootseparator or two new headerseparator and footerseparator parameters. And we then have two options

• Let the headfootseparator or headerseparator / footerseparator simply have the default "$n". Upgraders that rely on the default behavior of todays separator will not know the difference. But those that redefined the separator and changed it to an empty value to have different behaviour between search results and header/footer will be in trouble as they suddenly have newlines where they didn't before.
• Let the headfootseparator or headerseparator / footerseparator have the default "same as seperator ". This means that the plain default is still that you get new lines everywhere. And if you used the separator with another value the default is still OK for you. And for those that need a different behavior after header and before footer can redefine the headfootseparator or headerseparator / footerseparator parameters.

I know this/these new parameters are a bit unclean in nature but just ignoring the long history of especially the header is not fair either.

The spec for the headfootseparator or headerseparator / footerseparator I propose is

• Gets postfixed to header IF the header is defined.
• Gets prefixed to footer IF footer is defined.
• Is default the same value as $separator

You may have better ideas how to avoid breaking existing applications. But this suggestion will for sure work - with a small but acceptable increase in NerdoMeter score for the SEARCH feature.

The case to consider is searches like this

%SEARCH{ "VarREMOTE"
  scope="topic"
  nosearch="on"
  nototal="on"
  header="| *Topic* | *Summary* |"
  format="| [[$topic]] | $summary |"
  footer="| *Topics found* | *$ntopics* |"
}%

which gives

Topic	Summary
Topics found	3
System.VarREMOTEADDR	REMOTE_ADDR environment variable Examples * %REMOTE_ADDR% expands to == Related ENV, HTTP_HOST, REMOTE_PORT, REMOTE_USER
System.VarREMOTEPORT	REMOTE_PORT environment variable Examples * %REMOTE_PORT% expands to Related ENV, HTTP_HOST, REMOTE_ADDR, REMOTE_USER
System.VarREMOTEUSER	REMOTE_USER environment variable Examples * %REMOTE_USER% expands to Displays the user identity established by the Web Server. Not available when using ...

And the goal is to avoid that users using such a construct do not have to walk through 1000s of topic to find all SEARCH macros having to add a $n to the header. (note that on foswiki.org you do not see the footer because foswiki.org is still on 1.0.0)

-- KennethLavrsen - 28 Jun 2009

The header, footer, separator and format parameters are our fantastic four (FF).

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. There's nothing to interpret about them. Adding other kinds of separator does only make sense under very constraint conditions (e.g. when formating happens recursively and you'd like to specify header and footer before descending down to the children of a hierarchical result set).

I don't think that headerseparator or footerseparator cut it. It deforms things even more by adding more stuff on top of nonoise="on" and family.

There are a lot of macros in the core that do not have the FF although they should as they iterate over a set of items. Examples are %GROUP, %INSTALLEDPLUGINS, %WEBLIST, %TOPICLIST, etc. You may argue that not all of them need a dedicated header and footer parameter. Might be.

Fact is that this consistent inconsistency limits the usefulness of Foswiki enormously.

Most probably formatting should not be siloed into each macro again and again. That's what Sven is driving at.

On the other hand, Foswiki and TMwiki have never ever been a source of consistency. Why would you bother now?

Either you care or you don't. There's not much of a choice in the middle as that won't lead to consistency obviously.

Weather you care but you don't get consistency or you don't care and you keep on beating the same old dead horse is equally bad of a choice.

Sure, users of TMwiki and Foswiki would like to keep on riding their old (dead) horse for another decade and be happy with it. They won't. They just suddenly get aware that they sit on a rotten bit of meat and leave in panic.

Let me tell you: they are not amused. They are shocked and irritated by the consistent lack of consistency.

That said, it would be better to shock them for a last time and get them anywhere near consistency.

Now, let's get this sorted out with consistency & love in mind as Sven has proposed. Users will appreciate it. They won't appreciate yet another geeky parameter to SEARCH.

-- MichaelDaum - 28 Jun 2009

There is no disagreement that the current behaviour limits the use.

The question is how do we implement it so users will not hate us when we break compatibility again.

We have done it before and had some pretty rough and justified negative feedback about it. It is difficult on an old Foswiki upgraded from TWiki and used for years to find all the SEARCHes and verify they did not use the header and relied on the default trailing $n.

I am looking for better proposals on what we can do than the one I raised (which I do not find too elegant).

Just changing the behaviour and breaking formatted searches and not caring about our users is not an option.

-- KennethLavrsen - 29 Jun 2009

the $n case, ie when separator is not set, can probably stay the same as it is now - as that is most likely what is expected

return '' unless  @result;
if ($separatorNotSet) {
   return join($n, [$header, @result, $footer]);   #should allow that header and footer could be undefined (er '') tho
} else {
   return $header.join($sep, @result).footer;
}

The discussion above, and on irc reads more like we're all arguing past each other - micha and I talking about the separator="something" case, and Kenneth about the $separatorNotSet case.

-- SvenDowideit - 29 Jun 2009

I don't understand; I can't find any occurrence of $separatorNotSet in code to explain what that means, Sven. So doing my usual "let's analyse what we've got" act, I reviewed where separator is used in the core, and discovered that when separator is not given in the macro, then:

• %SEARCH: separator is \n, and additionally $n in the separator expands to \n (but not other standard escapes),
  • $header.join($sep,@results).$sep.$footer
• %META: 'parent' uses > as the default separator, does not expand $n (or any other standard escapes)
  • no support for header or footer.
  • join($sep, @results)
• %SPACEOUT: uses ' ', does not expand $n (or any other standard escapes)
  • no support for header or footer
  • join($sep, @results)
• %WEBLIST and %TOPICLIST: use \n and expand $n (but not any other standard escapes)
  • no support for header or footer
  • join($sep, @results)
• %QUERYPARAMS: uses \n and expands all standard escapes in the final rendered string
  • no support for header or footer
  • join($sep, @results)
• %URLPARAM: uses \n, does not expand
  • no support for header or footer
  • join($sep, @results)
• %LANGUAGES: uses \n
  • no support for header or footer
  • join($sep, @results).$sep

In addition various plugins use their own mixtures of header, footer and separator. So, using any measure of consistency, the only consistent thing is .... inconsistency. Pulling this lot into a consistent framework without breaking compatibility is not practical. It seems to me we have three choices:

1. Accept the pain of change, and make the usage of header, footer and separator consistent and documented.
2. Do nothing.
3. Make as much as possible consistent, and deprecate the crappy bits. I think this means deprecating the current separator parameter and devising a new definition that offers consistent behaviour (for example:)
   • format - template of a result. Standard escapes are expanded. Default is a string appropriate to the macro
   • join - text used to join individual results. Standard escapes are expanded. Defaults to a string appropriate to the macro (note: if join is given, the old separator parameter will be ignored)
   • header - text before the results. Standard escapes ($n etc) are expanded. There is no join between the header and the results. Default is no header.
   • footer - text after the results. Standard escapes are expanded. There is no join between the last result and the footer. Default is no footer. Examples:

        %SEARCH{... header="Results:$n" footer="$nThat's all, folks" join="$n   " format="* $topic"}%
        %URLPARAM{... header="Params: " join="," format="$lt$value$gt"}%
        %SPACEOUT{... format="'$word'" join=" "}%
        

As you might have expected, I favour 3. We either allow the TWiki millstone to drag us down, or we rise above it and look to the future. I know which approach I prefer.

-- CrawfordCurrie - 29 Jun 2009

I actually aired the same proposal as (3) on IRC - introducing a new separator and deprecating the old AND letting the new override the old if it is defined, but I chose not to write it here until I had slept on it, and other ideas had been aired.

As long as we take care of writing the documentation clearly and a good release note I think this will be a very good solution.

I think the name "join" for the new parameter is a fine name.

I could also live with Sven's proposal (default a $n after the header when separator is not defined). It would cover probably 99% of the upgrade cases. But it would be very difficult to document in a clear way.

I prefer Crawford's proposal because it is clean, can be documented clearly, and is a complete solution for both compatibility and for a clean generic solution that reaches further than just SEARCH.

-- KennethLavrsen - 29 Jun 2009

Sigh, separator and join ... tell that Aunt Tiffy when she just learned about separators in the other macros. This is awful.

-- MichaelDaum - 29 Jun 2009

Michael, if you have a constructive solution to this problem, then please share it. But demotivating, throwaway criticism is not helpful.

-- CrawfordCurrie - 24 Jul 2009

Sorry to jump in, but I think Crawford analysis is great, but maybe it's time for a bit more drastic approach.

Yes, I agree compatibility should only be broken for very specific reasons, and not just because it looks better, or help you sleep at night.

Still, here, it's clear there is absolutely no consistency, and without reading the code any user won't even understand why he can't use the same fantastic four for all parameters dealing with lists. And I (and I think that's also what motivated Sven and Micha's arguments) think that's also a huge problem to take into account.

Of course, we shouldn't just break it all and let users edit hundreds of topics to fix their SEARCHes. But can't we make some clear table about the former behavior, and the new one, and maybe even make some very simple script that looks for all topics which use %SEARCH, %META, %SPACEOUT, %WEBLIST, %QUERYPARAMS, %URLPARAM or %LANGUAGES and tell the user what needs to be done, or do it for him?

Breaking compatibility without warning is bad, but keeping a broken design just because one doesn't want to break it is just as bad imho.

We could really standardise all this by making some Foswiki::Func::formatList which knows how to handle the FF, and make all these macros use it.

-- OlivierRaginel - 24 Jul 2009

Olivier, that's the best idea I've heard so far

-- MichaelDaum - 24 Jul 2009

Well, it's an implementation detail, but what Olivier is basically saying is Option (1), and Michael is supporting it. Time for a voting table:

1. Fix separator, take the pain
2. Do nothing
3. Keep separator as broken, add join |

Who:	Prefers:
CrawfordCurrie	3
KennethLavrsen	3
OlivierRaginel	1
MichaelDaum	1

-- CrawfordCurrie - 24 Jul 2009

Democracy does not always help in making good technical decisions

-- MichaelDaum - 24 Jul 2009

Well Michael, if democracy does not work for you - you suggest yourself as the new dictator? I had enough of dictators in the old project and I prefer democracy any time. Even if this means that sometimes a majority decides things that I am against. What is important is that voters are enlighted and know what they vote about. And voting should not happen until we agree what we vote about. Debate before vote. I do understand why Crawford started the vote as this discussion had stalled. Olivier suggested some script that I would like to understand more how it could solve the compatibility problem.

Crawford - if the end result is - 4 people wants to "1", 3 people wants "2" and 3 people wants "3" - does "1" win? Or it is 6 against 4? It is not a correct vote Crawford.

And we actually have a "4". Sven suggested a lighter solution which in 99% of the cases would be OK and which maybe even Michael and Olivier would accept. And I for sure believe our users could live with. It has a challenge with respect to documenting it but not an impossible challenge.

I am not sure Oliver's idea was to "take the pain". Olivier you said "Of course, we shouldn't just break it all and let users edit hundreds of topics to fix their SEARCHes."

Crawford suggested a way to implement the feature we want the right way using a new setting called join. And leave the old bad one as it is. This means combining

• Respect for our users having a lot of content
• Doing the what is right for the future

If you look at the %SEARCH, %META, %SPACEOUT, %WEBLIST, %QUERYPARAMS, %URLPARAM or %LANGUAGES

LANGUAGES is really only used in skins for the user interface to change language. End users are not likely to ever use this. WEBLIST is mostly used in the left bar and really needs to be improved anyway. Current spec sucks.

URLPARAM does indeed have a separator where the default today is \n. But the purpose is not exactly the same as in SEARCH.

SPACEOUT is also a completely different beast where the default is a space and not a \n. I see no issue with this being different and I doubt many has ever used this.

META has the separator as a suboption for parent with a default " > ". Again a totally different beast.

I really do not think users see a big link between these very different macros and already today they behave differently. You also want to change the default for META to "\n" instead of " > "??

Please try and reconsider and think about what is in the best interest of the old loyal end users.

We have at the moment 4 and not 3 proposals

1. Fix separator (let the users take the pain by not caring about compatibility)
2. Do nothing (keep things broken and awful)
3. Keep separator as broken, add join (one solution that fixes compatibility but adds complexity and is different from other macros that have a separator today)
4. Fix separator, but leave the default behavior as it is today assuming that 99% of the users depending on the old behaviour use the default value and have not specified any separator. (Sven's proposal - which requires that we take care to document the behavior well)

And I am not convinced that we cannot collaborate with an open mind and find a "5" that is even better.

My current preference is Crawford's proposal with Join. My 2nd choice is Sven's "Fix and keep old default" and if this can gain concensus I am OK with that also and would change my vote to Sven's idea.

-- KennethLavrsen - 24 Jul 2009

I think "careful documentation" is a waste of time, and we should just put the old behaviour behind us. The realistic alternatives are to deprecate it (replace separator with join but keep separator as it is) or fix separator, according to some consistent rules that will piss off the users.

IMHO there is no contest. I will implement join unless (1) there's a tiebreaker vote or (2) someone beats me to a better solution.

-- CrawfordCurrie - 06 Aug 2009

Crawford, instead of rushing to add join, can you complete the full set of unit tests first (maybe we have most of them already..), and then we can find out if my proposed solution is as complex to document as Kenneth states? imo it would not be difficult to document.. (and tbh, it feels like the difference between join and separator will be close to zero)

-- SvenDowideit - 07 Aug 2009

I added unit tests for SEARCH, QUERYPARAMS, WEBLIST, TOPICLIST and reviewed those for URLPARAM. I did not look at LANGUAGES or DISPLAYDEPENDENCIES, neother of which have even basic unit test suites.

Note that the tests I added test behaviour as it is. As such they illustrate the inconsistencies quite well.

I did not add any tests for token expansion inside format, header, footer or separator.

-- CrawfordCurrie - 07 Aug 2009

cool thanks! in the changes I just did I can see we have rather alot more to add (oneweb, 2 webs, 3) nonoise, nosummary etc, what fun.

-- SvenDowideit - 11 Aug 2009

just pinging this task to say, its going o take a while longer. the SEARChsep code is a convoluted mess of add, remove, add again, remove sometimes blah.

i'm ploughing through it, but its fascinating.

-- SvenDowideit - 12 Aug 2009

I have been considering this for a long time.

And I have come to the conclusion that Sven's proposal is the best.

When you just use the plain simple example most people will have used for years ands years where you let the SEARCH build a table, thing will still work as you get the newline between the header and the result.

And the minute you start using the separator option things will work like Michael has suggested. I think this is the solution that is going to cause the least trouble for everyone.

And I will take the task to document the special behaviour of no separator. I think it is possible to document so all understand why a new line is added after header then no separator is given.

-- KennethLavrsen - 17 Oct 2009