Omega SORTREVERSE semantics confusing

[Olly Betts]

This is analogous to the changes made to the xapian-core API in 1.0.11 as a result of #311.

Currently Omega default to reverse=true (in current xapian-core terms), and SORTREVERSE=1 sets reverse=false. Argh.

The only sane way out I can see is to deprecate SORTREVERSE and add a new CGI parameter to specify forwards or reverse searching, which is documented as something that should always be specified if SORT is specified. Then in the release series after 1.4.x, we can remove SORTREVERSE, change the default if neither SORTREVERSE nor the new parameter is specified to reverse=false, and all is well.

Not sure what a good name for the new parameter is though.

Marking for 1.3.x, as this doesn't need much work at all (once we have a parameter name!) and would make sense to do before the 1.4 release series.

(Issue actually noted by Andy Chilton).

[Olly Betts]

OK, my opening bid is SORTREV.

Marking for 1.3.4 - that has a new filter encoding (but supports the old one too), and it would be nice to change the 'F', 'R', etc in the new encoding to match the saner direction semantics.

[James Aylett]

How about we roll it into SORT itself? Prefix with - to make it a reverse sort. If not that, I'd favour something like SORTORDER with values representing "ascending" and "descending", which I suspect is more the sort of thing people are used to.

[Olly Betts]

We went over "ascending"/"descending" vs "reversed" several times in the discussion in #311, and ended up with "reverse" as what we use in the C++ API. Partly that was because we'd already muddied the waters over what "ascending" and "descending" meant, and probably enough time has now passed that people won't be confused by that historical meaning, but aligning omega and xapian-core's terminology seems helpful (particular for people hacking on Xapian, but also for users who interact with both omega and core).

Putting this into SORT seems a plausible option. Using - doesn't seem ideal (as it makes the argument look like a signed number, and then -0 being different to 0 is a bit surprising), but we could use a different character.

I wonder which is easier to use in HTML forms. If you're building the UI with JS it makes little difference - I'm thinking about a plain HTML UI. I guess it depends if you want a selector for what to sort by with a checkbox for "reverse", or a selector which includes forward and reverse versions of each sort key. Both are probably plausible UIs, though it's perhaps unhelpful to change which Omega can do in plain HTML.

Perhaps this actually points to decoupling the CGI parameters from what they control, so you should be able to say something like:

$setsort{$cgi{SORT},$cgi{SORTREV}}

or if you want the leading - approach:

$if{$eq{$substr{$cgi{SORT},0,1},-},$setsort{$substr{$cgi{SORT}},1},true},$setsort{$cgi{SORT},}}

Or to just hardcode a reversed sort on value 0:

$setsort{0,1}

But that's probably out of scope for crowbarring in before 1.4.x - we'd want to do this consistently for all the CGI parameters.

[James Aylett]

#311 was seven years ago, so I don't think it's impossible to rethink things if we decide it's sensible. However the argument for consistency with C++ may be the right call. My thought was for people coming from outside the ecosystem, say web developers who have experience with either SQL (ORDER BY … DESC|ASC) or ORMs (some of which do things like .order_by('-field_name') to do reverse sorting). I don't actually know what the audience for Omega is these days, however (since a lot of web frameworks have an indirect way of integrating Xapian, web developers from those backgrounds are probably less likely to use Omega at all).

I like the decoupling idea, but agree that it isn't right for before 1.4. Whether it's worth doing depends I think a little on who is using Omega and what their background is.

[Olly Betts]

(As I noted in comment:11:ticket:311) reverse will also be familiar to people already:

For example, the UNIX sort program defaults to ascending with "-r" for reverse (the GNU version even has "--reverse" as the long form of "-r"); [...] Python's sorted function and sort method [have] an optional "reverse" parameter); PHP's sort() is ascending by default (it offers an "rsort" function to sort in reverse order).

It's a good thought as to what the audience for Omega is, though it's not easy to really know who is using it and for what. If you have a server-side framework, Xapian integration for that is likely a better approach.

Perhaps supporting both a parameter to reverse the order, and a way to specify reversing via a prefix (or suffix) on SORT would be useful. A - prefix seems a bit confusing on a number, but if it's common in ORMs then perhaps using a - prefix is better than a different prefix or suffix.

[Olly Betts]

Grouping the omega 1.3.* tickets on 1.3.5.

[Olly Betts]

Reviewing...

A - prefix makes total sense for a named key with a numeric value (in algebraic terms, sorting on -price is indeed the reverse of sorting on price). For a named key string value you can think of - being overloaded for strings I guess. The analogy rather breaks down once you have numbered keys, as we do here.

I think we ought to do something, and it'll be simpler for people to upgrade if we're essentially just renaming the parameter (and its value), which argues for a separate parameter. So I'm leaning towards SORTREV for 1.4.x and we can think about decoupling for the future.

[Olly Betts]

Hmm, though the "easy to upgrade" argument is rather reduced if you have to specify SORTREV if you specify SORT (which wasn't the case for SORTREVERSE of course).

So a single parameter specifying both the sort key and direction would be a simpler upgrade if you never reverse the order (since you still just specify one parameter, it's just a different one with a different value for each sort).

I don't think we can just add the option to prefix SORT with - though - currently SORT=1 means "reverse sort on value slot 1", so SORT=-1 would then be "forward sort on value slot 1"! We'd need a prefix or suffix for both directions, at least for the transition, so something like SORT=+1 vs SORT=-1 (the latter being the same as SORT=1 currently is).

With that, we could actually keep SORTREVERSE to mean "reverse the sort order specified by SORT", so SORT=+1 SORTREVERSE=1 would be the same as SORT=-1 or SORT=-1 SORTREVERSE=0.

With that, the simplest upgrade path is actually just to prefix your SORT parameter values with -, making explicit the currently implicit default of "reversed".

Overall, I'm liking this. Even the - prefixed slot numbers seems less odd when there's also +.

[Olly Betts]

Implemented in [27592d05bc6de402ada8cacbd42661e1bb3c4359].

I've changed around R and F in the $filters encoding - this changed to a new encoding in 1.3.4, and I think it's preferable to make this change between consecutive release snapshots rather than having the encoding be "wrong" for indefinitely.

The consequence would be that we would/wouldn't force the first page if sorting on a value slot was in effect, but only once for each search session spanning an upgrade, or if such a search URL had been bookmarked, the meaning of it would change.