sup talk - Sep 2009 - [PATCH] Allow thread index view to sort oldest first

Keith and I both want to read our email in chronological order,
(reading oldest messages first), so we believe it makes sense to
present the inbox mode in this order, (with the oldest messages at the
top).

This is distinct from the order for global searches where having the
newest messages first does seem obviously correct.

The below patches are a first cut at implementing this. They provide a
new configuration option, (:inbox_newest_first), which can be used to
control the default sorting for the inbox. Keith''s original patch
doesn''t change sup''s behavior unless the user manually
configures this
option to false. My second patch changes the default. Obviously, it
would be easy to accept Keith''s version without changing the default
for sup.

One nice thing about the inmplentation is that any refined searches
inherit the mode of the parent search, which makes it easy to maintain
oldest-message-first for "reading" and newest-message-first for
"searching".

Also note that there''s a new command ''o'' to toggle
the search order
for a current view. This is a feature that we talked about earlier as
a way to avoid the need for the distinction between the '',''
and '']''
commands in thread-view-mode. If the user can control the sort of the
search view, then it would be more natural to have commands that
simply advance to the "next" message, (since the user can choose in
advance what "next" means).

A couple of caveats with respect to the patch as it exists so far:

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.

   In my use so far this has worked well since I generally have a
   small number of messages in my inbox (and even fewer in my refined
   views of my inbox) and those are the only places where I use
   oldest-first sorting. For global searches, I do have an effectively
   infinite number of messages, but there I do use newest-first
   searching which still has the lazy loading.

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.)

-Carl

PS. We''re still total ruby newbies, so please point out any silly
mistakes we''re missing with respect to ruby idioms.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Allow-thread-index-view-to-sort-oldest-first.patch
Type: application/octet-stream
Size: 7849 bytes
Desc: not available
URL:
<http://rubyforge.org/pipermail/sup-talk/attachments/20090925/8fd3d30c/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Change-the-default-sort-for-inbox-mode-to-be-oldest-.patch
Type: application/octet-stream
Size: 777 bytes
Desc: not available
URL:
<http://rubyforge.org/pipermail/sup-talk/attachments/20090925/8fd3d30c/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: not available
URL:
<http://rubyforge.org/pipermail/sup-talk/attachments/20090925/8fd3d30c/attachment.bin>

Hi Carl,

I am interested in trying these changes but I think these patches were
generated against a custom version of master (e.g. I see the inbox
refine-search command in the context, which hasn''t been published yet).
Can you rebase them onto a recent non-modified master so that I can
apply? Thanks!
-- 
William <wmorgan-sup at masanjin.net>

Excerpts from William Morgan''s message of Thu Oct 01 10:04:52 -0700
2009:
> I am interested in trying these changes but I think these patches were
> generated against a custom version of master (e.g. I see the inbox
> refine-search command in the context, which hasn''t been published
yet).
Guilty as charged.
> Can you rebase them onto a recent non-modified master so that I can
> apply? Thanks!
See the attached patches.

And as before, the behavior is split into two commits so that you can
decide whether to change the default or not.

And there''s still the one minor bug that the oldest-first mode
doesn''t
actually guarantee that the oldest message is displayed first when
newer messages arrive for an old thread.

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Allow-thread-index-view-to-sort-oldest-first.patch
Type: application/octet-stream
Size: 7502 bytes
Desc: not available
URL:
<http://rubyforge.org/pipermail/sup-talk/attachments/20091001/ec87d014/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Change-the-default-sort-for-inbox-mode-to-be-oldest-.patch
Type: application/octet-stream
Size: 777 bytes
Desc: not available
URL:
<http://rubyforge.org/pipermail/sup-talk/attachments/20091001/ec87d014/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: not available
URL:
<http://rubyforge.org/pipermail/sup-talk/attachments/20091001/ec87d014/attachment.bin>

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 at masanjin.net>

Excerpts from William Morgan''s message of Mon Oct 12 05:48:50 -0700
2009:
> I''ve finally gotten a chance to look at this. It looks good so
far.
Thanks for taking a look at 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 totally understand that, and that''s why I did that part as a
separate patch.
> I would also like to disable forcing the loading of all messages.
I can understand that too.
> 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.
How do we get this behavior in xapian? I''m fine if the index is not
optimized for this. The current newest-first optimization is perfect
for actual searching, (as opposed to reading of new messages), and the
patches don''t change that.
> Either way, I''m happy to merge the first patch with the "n =
-1" thing
> removed.
I wouldn''t want the patch accepted with just that change. It results
in a "broken" view, (it claims to be showing the oldest message first,
but it doesn''t, and trying to scroll "up" to see earlier
messages also
doesn''t work).

So I''ll see if I can figure out how to just load the N oldest message
instead. (My working theory was that this perform similarly to
actually loading all the messages, and that''s why the patch just did
that).
> Pretty easy to change. In thread.rb, there''s a date method which
takes a
> max; you can make it take a min instead.
Excellent. I''ll take a look at that.
> 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.
OK. I''ll see if there''s nearby code to mimic for this.

-Carl
> > 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".
Heh. Just today I noticed "unless" while poking around in some sup
code. I have to say that "unless ... else" seems like a downright
malicious construct to unleash against people reading code.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: not available
URL:
<http://rubyforge.org/pipermail/sup-talk/attachments/20091012/88815a8c/attachment.bin>

On 2009-10-12, Carl Worth <cworth at cworth.org>
wrote:
> How do we get this behavior in xapian? I''m fine if the index is
not
> optimized for this. The current newest-first optimization is perfect
> for actual searching, (as opposed to reading of new messages), and the
> patches don''t change that.
    @enquire.docid_order = Xapian::Enquire::DESCENDING

This means that the docid order (which is used as the least significant
ordering) is descending rather than ascending.  In this case (I assume)
we''re using Xapian::BoolWeight so this is the only ordering.

Cheers,
    Olly