Subject: [sup-talk] On making inbox-mode and search-results-mode more similar

From: [email-masked] (Carl Worth)
Date: Wed, 26 Aug 2009 15:24:36 -0700

[Argh! I ended up getting really verbose again with this one. Thanks
to anyone patient enough to wade through all of this. There is a patch
here for one issue at least. See below.]

I spent about 4 days away from email which gave me my first case of a
large inbox after switching to sup. And there are definitely a few
things I'd like to convince sup to do to make it easier for me to deal
with this.

Most important, with a large inbox, my mail falls into different
"classes", some of which are more important than others, and I'd like
to deal with the most important things first. I also often want to
deal with a single "class" of mail as a whole to avoid mental context
switches.

Fortunately, I've got sup labels capturing these classes quite
well. I've got some future ideas for having a prioritized (partial
order) list of labels so that inbox-mode will present messages
according to that priority. But for now, it would be good enough to be
able to easily filter my inbox down to just a single class of messages
at a time.

To this end, I started using the patch below, which adds a
refine_search keybinding to inbox-mode much like the same function in
search-result-mode. This is useful already and might be considered for
inclusion as is, (another feature I'd like for both modes is "restrict
to label" which would act like refine-search but save me from having
to type "label:" much like the 'L' command avoids having to type
"label:" compared to the 'F' command).

Beyond this patch as is, though, I'd like to talk a bit about how to
better achieve what I want. There's obviously some code duplication in
this patch which is not ideal. It would be better if InboxMode could
be a derived class from SearchResultsMode. Not only would this reduce
code-duplication, but there are features currently in InboxMode that I
think belong in all searches.

For example, if I use the refine_search command below to get a view of
my inbox restricted to a particular label, it no longer acts like my
inbox does. One of the most noticeable cases is the archive
command[*].  In both inbox-mode and search-result-mode this command
removes the "inbox" label from a thread, but in inbox-mode the thread
is immediately removed from the results, while in search-results-mode
the thread remains in the results.

One annoying side effect of this is that when I'm reading and
archiving many threads in some class, then I happen to reply to one
thread, that thread gets bumped to the top of my search so when I
navigate to the "next" thread I start cycling through threads that I
just got finished reading and archiving. To fix this I end up having
to notice I'm seeing repeated messages, remember why, then save my
current changes and refresh the search. And I have to do this after
every reply.

[Note: Even if archiving messages made them immediately disappear from
my search results, there could still be problems with "next" causing
me to re-cycle through messages I had already read but decided to
leave in my inbox. I think this is just another general race-condition
bug. I think the fix would be for the transition from
search-results-mode to thread-view-mode to somehow snapshot the
current search results so that future modifications due to newly
arriving messages don't modify the meaning of "next thread" within
thread-view-mode.]

So what I want is for anytime a message is changed such that it no
longer meets the current search criteria, it is immediately removed
from the search results. I think that means simply implementing the
is_relevant? method for SearcResultsMode where there is currently just
the following comment:

  ## a proper is_relevant? method requires some way of asking ferret
  ## if an in-memory object satisfies a query. i'm not sure how to do
  ## that yet. in the worst case i can make an in-memory index, add
  ## the message, and search against it to see if i have > 0 results,
  ## but that seems pretty insane.

That single-message index and search sounds exactly like what I want,
and not insane at all. Has someone attempted this and found a
performance issue? (In which case, I would think we just need to fix
that performance bug.) Or is it simply a matter of nobody having tried
to code this up yet? (In which case, I'll try, and I'll be glad for
any pointers that anyone can give me on how to go about creating an
in-memory index and searching on it.)

Thanks,

-Carl

[*] I'll ignore for now the fact that 'a' is a toggle of the inbox
label in SearchResultsMode and a one-way removal of the label in
InboxMode. I haven't found any need to ever put a thread _back_ into
the inbox, (except perhaps in the case of accidental archiving, and
there an immediate undo works fine). So I would imagine the use is
rare enough to not need a single-character command. The command I
would find natural for putting a thread back into the inbox would be
to simply add the inbox label to the thread. Oddly this is currently
not allowed as "inbox" is a reserved label. I don't

>From d37eca31aaddee80f05357d6a4ed896f39bbad16 Mon Sep 17 00:00:00 2001
From: Carl Worth <cworth at cworth.org>
Date: Wed, 26 Aug 2009 10:17:20 -0700
Subject: [PATCH] Add a refine_search command to InboxMode

This is just a copy of SearchResultsMode's refine_search command.
A much cleaner, but more involved, approach would be to rework
InboxMode to derive from SearchResultsMode in the first place.
---
 lib/sup/modes/inbox-mode.rb |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/lib/sup/modes/inbox-mode.rb b/lib/sup/modes/inbox-mode.rb
index ba095da..c8072cd 100644
--- a/lib/sup/modes/inbox-mode.rb
+++ b/lib/sup/modes/inbox-mode.rb
@@ -7,6 +7,7 @@ class InboxMode < ThreadIndexMode
     ## overwrite toggle_archived with archive
     k.add :archive, "Archive thread (remove from inbox)", 'a'
     k.add :read_and_archive, "Archive thread (remove from inbox) and mark read", 'A'
+    k.add :refine_search, "Refine search", '|'
   end
 
   def initialize
@@ -17,6 +18,12 @@ class InboxMode < ThreadIndexMode
 
   def is_relevant? m; (m.labels & [:spam, :deleted, :killed, :inbox]) == Set.new([:inbox]) end
 
+  def refine_search
+    text = BufferManager.ask :search, "refine query: ", "label:inbox -label:spam -label:deleted -label:killed "
+    return unless text && text !~ /^\s*$/
+    SearchResultsMode.spawn_from_query text
+  end
+
   ## label-list-mode wants to be able to raise us if the user selects
   ## the "inbox" label, so we need to keep our singletonness around
   def self.instance; @@instance; end
-- 
1.6.3.3

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://rubyforge.org/pipermail/sup-talk/attachments/20090826/0ec68a3f/attachment.bin>

---

From: [email-masked] (Carl Worth)
Date: Wed, 26 Aug 2009 15:52:38 -0700

Excerpts from Carl Worth's message of Wed Aug 26 15:24:36 -0700 2009:
> +    text = BufferManager.ask :search, "refine query: ", "label:inbox
> -label:spam -label:deleted -label:killed "

Is that even the right syntax for searching for threads with the inbox
label but excluding threades with the spam, deleted, or killed labels?

I took a guess at the syntax and I thought I saw it working, but it
doesn't appear to be now. Can someone tell me the actual syntax? (Or
better, is there a writeup somewhere of the general search syntax
accepted by sup?)

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://rubyforge.org/pipermail/sup-talk/attachments/20090826/c0c5ef5d/attachment.bin>

---

From: [email-masked] (Rich Lane)
Date: Thu, 27 Aug 2009 02:50:29 -0400

> in-memory index and searching on it.)

We had a thread a while back about applying label changes immediately
and I think the consensus was that that's a good idea. Doing this could
simplify a great deal of thread-index-mode code because a fast
is_relevant? could be implemented using existing index operations
without the special cases for archiving/etc and without creating an
inmemory database. I have a patch I plan to mail out soon that makes
changing labels with Xapian much quicker, so that should help. It'd be
great if you decide to tackle this.

---

From: [email-masked] (William Morgan)
Date: Thu, 03 Sep 2009 11:32:54 -0700

Reformatted excerpts from Carl Worth's message of 2009-08-26:
> is_relevant? method for SearcResultsMode

is_relevant? is currently only used for determining when to add messages
to a thread-index-mode (e.g. when a new message is picked up during a
poll). Determining whether to drop a message is currently done
"manually" by inbox-mode, and not at all by search-results-mode.

> and not insane at all.

Hm. Well, you could certainly try it. I've assumed it would be too slow,
but it's possible it'd be fast enough. I think it's the only option
though, if you want this to work with arbitrary queries.

> (In which case, I'll try, and I'll be glad for any pointers that
> anyone can give me on how to go about creating an in-memory index and
> searching on it.)

Excellent, ask Rich. :)

> The command I would find natural for putting a thread back into the
> inbox would be to simply add the inbox label to the thread. Oddly this
> is currently not allowed as "inbox" is a reserved label.

Yeah, there's no good reason for this. I was just being overly
protective. :)
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (William Morgan)
Date: Thu, 03 Sep 2009 11:36:42 -0700

Reformatted excerpts from Rich Lane's message of 2009-08-26:
> and I think the consensus was that that's a good idea. Doing this
> could simplify a great deal of thread-index-mode code because a fast
> inmemory database.

But it's not just a label issue. Determining whether a given message
belongs in a general search buffer requires the full engine. And even if
the query is just on labels, it can contain arbitrary boolean
expressions, and I don't want to be the one having to parse that, and
parse it in such a way that it matches what the search engine is doing.

> I have a patch I plan to mail out soon that makes changing labels with
> Xapian much quicker, so that should help.

This is definitely good, either way.
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (William Morgan)
Date: Thu, 03 Sep 2009 11:39:58 -0700

> label but excluding threades with the spam, deleted, or killed labels?

Instead of -label:killed, you want to specify :skip_killed to
#load_thread. Killed messages require special semantics.

> better, is there a writeup somewhere of the general search syntax

There is not a consistent one, though there might be something useful on
the wiki. The general answer is, whatever Ferret/Xapian supports, plus
the tweaks in #parse_query.
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Rich Lane)
Date: Thu, 03 Sep 2009 14:44:06 -0400

> parse it in such a way that it matches what the search engine is doing.

You're right that it require the full engine. If we're immediately
saving the label changes to the (on-disk) index, we can simply query it
and get the correct answer.

---

From: [email-masked] (William Morgan)
Date: Thu, 03 Sep 2009 11:50:25 -0700

Reformatted excerpts from Rich Lane's message of 2009-09-03:
> saving the label changes to the (on-disk) index, we can simply query
> it and get the correct answer.

Oh, as opposed to a new in-memory index. Yeah.
-- 
William <wmorgan-sup at masanjin.net>