Re: [sup-talk] [PATCH] Allow thread index view to sort oldest first

From: William Morgan <wmorgan-sup@masanjin.net>
To: sup-talk <sup-talk@rubyforge.org>
Subject: Re: [sup-talk] [PATCH] Allow thread index view to sort oldest first
Date: Mon, 12 Oct 2009 05:48:50 -0700	[thread overview]
Message-ID: <1255350472-sup-5679@masanjin.net> (raw)
In-Reply-To: <1253911610-sup-2052@yoom.home.cworth.org>

Hi Carl & Keith,

Reformatted excerpts from Carl Worth's message of 2009-09-25:
> The below patches are a first cut at implementing this.

I've finally gotten a chance to look at this. It looks good so far. I
would like to merge this in in such a way that it doesn't change
behavior for anyone who doesn't want it.

So I definitely don't want the second patch which changes the default
order. The configuration boolean is fine. (And if you want to add a
question to sup-config, that's icing on the cake.)

I would also like to disable forcing the loading of all messages.
Personalyl, I follow the inbox 30,000 philosophy. The 'o' keybinding is
cool, but if I scroll down then suddenly the thread loading goes crazy.
For those who have inboxes that are small enough to load but bigger than
one screen (the "reverse inbox 50" crowd), I don't think that pressing
!! is too onerous.

> 1. When doing oldest-first searching, it wasn't obvious if it's even
>    possible to query for only the N oldest messages (to lazily load
>    new threads while navigating as sup currently does). So the patch
>    currently loads all threads when in oldest-first mode.

It is possible in Ferret: remove the DESC in ferret_index.rb line 160.
It is also possible in Xapian, but we're building the Xapian index to
optimize newest-first access. (Of course that would also be possible to
change, but then we're talking about a total index rebuild.)

If you wanted to tweak that, the load-all-threads wouldn't be necessary.

Either way, I'm happy to merge the first patch with the "n = -1" thing
removed.

> 2. Currently sup uses the date of the newest message in a thread as
>    the key for sorting that message. This is correct for newest-first
>    sorting. But when doing the new oldest-first sorting, the patch
>    really should be augmented to instead use the date of the oldest
>    message in a thread that matches the current search criteria.
> 
>    We haven't looked yet into how hard this would be to fix. (And we'd
>    of course be glad for any help or pointers.)

Pretty easy to change. In thread.rb, there's a date method which takes a
max; you can make it take a min instead.

The hard work for both of these things is wiring this option through.
Although $config is a global variable, I don't really want to use it
directly in e.g. thread.rb.

> PS. We're still total ruby newbies, so please point out any silly
> mistakes we're missing with respect to ruby idioms.

Everything looks good. The only slgihtly non-idiomatic thing is using
"if !x" instead of "unless x".
-- 
William <wmorgan-sup@masanjin.net>
_______________________________________________
sup-talk mailing list
sup-talk@rubyforge.org
http://rubyforge.org/mailman/listinfo/sup-talk