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: 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. 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. `---- 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. 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. 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! Best, -- David -- OpenPGP... 0x99ADB83B5A4478E6 Jabber.... dmjena@jabber.org Email..... dmaus@ictsoc.de
From c9b1e3f65e8d647e47399b5d2f23e0916b904c24 Mon Sep 17 00:00:00 2001
From: David Maus <dmaus@ictsoc.de>
Date: Sat, 11 Sep 2010 08:11:55 +0200
Subject: [PATCH] Don't perform search if search vector is empty
* elmo-imap4.el (elmo-imap4-search-internal): Don't perform search if
search vector is empty.
---
elmo/elmo-imap4.el | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/elmo/elmo-imap4.el b/elmo/elmo-imap4.el
index 13264f9..c56aa8d 100644
--- a/elmo/elmo-imap4.el
+++ b/elmo/elmo-imap4.el
@@ -2449,7 +2449,8 @@ command list) or a list of msg ids."
(elmo-imap4-search-generate-uid from-msgs)
(elmo-imap4-search-generate folder condition from-msgs))
(elmo-imap4-search-generate folder condition from-msgs))))
- (elmo-imap4-search-perform session imap-search)))
+ (when imap-search
+ (elmo-imap4-search-perform session imap-search))))
(luna-define-method elmo-folder-search :around ((folder elmo-imap4-folder)
condition &optional numbers)
--
1.7.1
Attachment:
pgpPoBABzL5F9.pgp
Description: PGP signature