Feature Proposal: SEARCH rendering is always partitioned by web - this is limiting
Motivation
Foswiki has
always separated the SEARCH results by web. This causes us to go through all sorts of horrible workarounds to get topics like
SiteChanges, and makes the
limit
and
sort
parameters very odd.
Tasks.Item2276 for eg.
this is (sadly) due to the 10 year history that all SEARCH results are to be rendered one web at a time. the SEARCH loop is (though I'm slowly working through removing this) implemented as
foreach web {
print web header
in twiki, we'd do the search here too..
sort the topics
foreach topic {
foreach multi-hit {
render topic info
}
}
print web footer
}
Description and Documentation
What I'd like to try to do, is remove this pre-de-optimization. There are a couple of things I'm going to try out in an attempt to produce a backwards compatible cleanup.
What I have implemented is a partial refactor that is triggered using the
groupby
parameter. There is only one way to trigger this code -
groupby="none"
, and it is used for making paging work on the
SiteChanges topic.
Further work on this parameter will occur based on the
SupportMultiKeySorting feature and is for post 1.1.
Impact
more unit tests
Implementation
--
Contributors: SvenDowideit - 08 Nov 2009
Discussion
I am in faviour of not ordering results per web as I have already expressed in a bug item.
But I have to raise concern to stop the 14-day clock on this proposal because...
There is no proposal yet. There is a problem but not yet a solution that people can agree or disagree on.
Once there I am sure I will agree.
--
KennethLavrsen - 13 Nov 2009
Sven will correct me if I'm wrong, but I read the proposal as removing the per-de-optmisation that forces search results to be sorted per web, and also adding multi-level sort keys.
And in that case I have to add a concern - the same concern that stopped this progressing as a bugfix, viz. what if people are currently depending on the default behaviour being sort-by-web?
--
CrawfordCurrie - 13 Nov 2009
ok, so the two of you don't think we should change the default SEARCH output from partitioned per web - i kinda agree, even though this partitioning has surprised users for many years (its just another thing users have to get used to).
in that case, I'm likely to add a
nostupidwebpartitioning='on'
param >:-}.
implementation wise - i'm working towards refactoring the code so that the
foreach web
portion is removed, but that there's a switch we can throw (one way or the other, what the default is doesn't bother me much) to render the output in the historical form.
I'd love to hear more opinions wrt dangers and side effects - and i'm sure that we'd like to know about people that
are relying on the current rendering - its not just 'sort-by-web' it is impossible to not 'sort-by-web' without resourting to
TabelPlugin post processing.
--
SvenDowideit - 13 Nov 2009
As someone who started writing wiki apps around the middle of this year, I'd like to +1 to the surprising behaviour of
SEARCH
.
One of my contrived work-arounds involved
CALC
, a table, and <!-- around the table.. -->. Perhaps in 1.1 the default behaviour could be set in configure for the 1 or 2 installations that require the old behaviour.
--
PaulHarvey - 14 Nov 2009
parked due to fear - one day we might have enough unit tests that a developer can change this - or my SEARCH refactor might find a way to abstract it out a little.
--
SvenDowideit - 06 Mar 2010
and now I have to unpark this. I pretty much don't have a choice but to try to implement this
if you try to enhance
SiteChanges topi using the 1.1 paging feature from
SearchResultsPagination, what you get can't be made to work using the
TablePlugin sorting hack that we've been using to get more than one web's results mixed and ordered only by date.
so..
on the pagination topic i suggested
so... I'm thinking that I'll investigate adding a seperatewebresults="off", where on is the default and how foswiki has rendered multi-web results for a decade, and probably off will be the default if paging is on.
seeing this proposal again, i think i might call the parameter
partition
, with a default of
partition="web"
, unless paging is turned on, in which case the default is
partition="off"
.
at some stage we can thus implement
partition="date"
or any other field that you can sort on.
Guys - I've changed the proposal impl - can you consider your objections please?
@Crawford - i was never proposing to change the default - but yes, I don't have time to implement multi-field sorting so i'll go with adding yet another expert level param.
@Kenneth - er, shit, sorry, the reason i didn't make a full proposal was that i didn't at the time have a good parameter name.
--
SvenDowideit - 09 Mar 2010
No objections.
(Arthur also suggested we could call the parameter
group
-
group="web"
being the default, unless you turn on pagination..)
--
ArthurClemens - 09 Mar 2010
I don't want to throw a ocelot among the kakapos, but it occurs to me that what you have here is not so much a "partition", as a "major sort key". Thus if you specify
order="date"
you are actually (because of the partition-by-web) specifying
order="web,date"
. Because
order
is already abused this way, you could perhaps deprecate it, keep the current definition, and add an overriding param
sort
which allows the specification of multi-key orders. Thus:
The currently definable
order
keys are:
This view is somewhat complicated by the "results for this web" headers, but may provide a route into a more logcial (less expert) definition for what you want to do. As well as providing a way of specifying multi-key sorting.
--
CrawfordCurrie - 09 Mar 2010
Grouping is presenting results with a list/table/header for each group. Multiple sort keys will give you one list/table/header. I think there is use for both.
--
ArthurClemens - 09 Mar 2010
Commitment for this was 08 Nov 2009 and not 09 Mar 2010. And this is important because otherwise we canmot take this to community vote but would have to wait 14-days. This proposal has been up for the 14-day period a long time ago.
What we need now is to re-address the concerns.
What is the spec? What would the community vote for? The minute I have the 3 text lines that says what you will do - I will remove my concern. I agree on the principle. I was hit by this stupid behaviour today. I would love to see the search result grouped by web disappear but is that all this proposal changes?
--
KennethLavrsen - 09 Mar 2010
I notice now that the spec has been updated. I am not religious about exactly how the feature gets implemented and having backwards compatibility cannot be a bad thing. I will leave it to Crawford and Sven and Arthur to agree on a spec and I remove my concern.
--
KennethLavrsen - 09 Mar 2010
Arthur, I agree. As long as we are
thinking of
how multi-key sorting would work if we had time to implement it, I'm happy. But I don't want the
partition
to implicitly constrain that, because we didn't think it through. The proposed
partition="web"
implies that you can select what to partition on. In that case, what do the following parameter snippets do?
order="modified" partition="web"
order="web" partition="modified"
i.e. does
partition
imply a top level sort key? When we come to support multi-level sort keys, what will:
sort="-topic,web" partition="modified"
mean? If it implies a major sort key, how do I reverse that sort order? Hence my concern.
To clarify and simplify this I propose that
partition
be a boolean parameter, either
on
or
off
, and the partition is always done on the
major sort key (as I observed above,
order
has an
implicit major sort key,
web
, which is why we need the new
sort
parameter)
--
CrawfordCurrie - 10 Mar 2010
ew. Its a damned shame you are correct. I am happy to implement the user facing portion of this in any way - and yes, nailing down
how multi-key sorting would work if we had time to implement it is the best way forward.
I don't think I have the time to do the coding work i need to do in the next weeks at the same time as nailing down the spec, so I appreciate this massivly.
--
SvenDowideit - 10 Mar 2010
Looking at the discussion, this proposal is still alive and the chance of consensus is still there. It would be wrong to take it for community vote now. I believe Sven and Crawford finds out between them a solution and when the concern is lifted this proposal can be declared accepted. So I leave it for now.
--
KennethLavrsen - 24 Mar 2010
I have implemented the feature (as updated in the spec) - due to its requirement to complete paging. The real user facing change that is Crawford's concern is moved to
SupportMultiKeySorting.
(I'm removing his concern - I think its an appropriate refactor of this proposal)
--
SvenDowideit - 01 Apr 2010