[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Pushing AND, OR search to IMAP



At Sat, 11 Sep 2010 09:01:18 +0200,
David Maus wrote:
> 
> Erik Hetzner wrote:
> 
> >I am not too surprised that you don’t see much speedup. Probably this
> >only would have a noticeable effect on AND searches which have a very
> >large number of results for one condition and a small number for
> >another. Still I think it is the right thing to do.
> 
> Agreed.  It saves uneccessary roundtrips.
> 
> I investigated the changes and they look good.  Just a few suggetions
> and comments:

Thanks for the detailed response. More below.
 
> A. `elmo-imap4-search-generate' can return an empty search vector
> ==
> 
> If you use the =flag= condition and there are no messages with this
> particular flag, the vector returned by this function is empty.
> 
> Example: /flag:unread/%Inbox on a mailbox that has no unread messages.
> 
> Attached patch catches this situation and does not create an invalid
> search command.

Thanks, I pushed this change up to:

  http://github.com/egh/wanderlust/tree/new-imap4-search

I am new to git so I am not sure if I got this right.
 
> B. Using CHARSET
> ==
> 
> We need to handle a possible NO response if the server does not
> support the requested charset.  I.e., RFC3501, 6.4.4:
> 
> ,----
> |       If the server does not support the specified [CHARSET], it MUST
> |       return a tagged NO response (not a BAD).  This response SHOULD
> |       contain the BADCHARSET response code, which MAY list the
> |       [CHARSET]s supported by the server.
> `----

If I am not mistaken this is not currently implemented, either in my
branch or the original?
 
> C. Terminology
> ==
> 
> To avoid ambiguities I suggest not to talk about message ids in the
> doc strings, but use the term "UID".  The IMAP spec distinguishes
> between UIDs (persistent between sessions) and message sequence
> numbers (not persistent at all) and by default ELMO uses UIDs.

Good point; done.

> D. Using UID commands
> ==
> 
> This is a more general question: Whether or not ELMO should use UIDs
> as messages numbers and subsequently uses the UID command for searches
> and fetches depends on `elmo-imap4-use-uid'.  The improved filter
> operations should take this into consideration -- although if someone
> would disable this option, WL would go crazy: Message sequence numbers
> are not persistent (not even in a session) and for example saving
> draft messages to an IMAP folder would fail because WL identifies a
> draft message by its UID (obtained via UIDNEXT) and not its sequence
> number.
> 
> Therefor it might be a good idea to make `elmo-imap4-use-uid' an
> obsolete variable.  After all the UID command is specified in the
> specs.

As you saw, I didn’t bother to support sequence numbers. It makes
sense to me to remove support for them; but perhaps I am missing some
use they have.

> E. Doc strings
> ==
> 
> I'd personally like the doc strings of the functions to be more
> imperative: The first sentence says what the function evaluates to,
> the arguments explained, then longer descriptions.
> 
> E.g., instead
> 
> ,----
> | Generate an IMAP search command if possible or a list of msgids for a
> | given search condition. Returns either (charset imap command list) or
> | a list of msg ids.
> `----
> 
> Something like this
> 
> ,----
> | Return search vector in FOLDER for CONDITON and FROM-MSGS.
> | 
> | FOLDER is a elmo folder structure.
> | CONDITION is a search condition.
> | FROM-MSGS is a set of messages.  When nil, generate vector for all
> | messages in FOLDER.
> | 
> | A search vector is either a list of UIDs ...
> `----
> 
> This format helps a lot if you use something like `eldoc-mode' when
> hacking Lisp because eldoc displays the first sentence of the
> doc string in the mini-buffer.
> 
> Otherwise: Nice work!

Thanks for the help with the docstrings. They have always been a bit
mysterious to me. I am rewriting them as you describe.

best, Erik

Attachment: pgph46ByJG2rt.pgp
Description: PGP signature