Re: FIXMEs in imap.el and nnimap.el

[Reiner Steib <reinersteib+gmane@imap.cc>]

On Wed, Jan 07 2009, Ted Zlatanov wrote:

> On Fri, 02 Jan 2009 23:57:08 +0100 Reiner Steib <reinersteib+gmane@imap.cc> wrote: 
>
>>> (defun imap-message-copyuid-1 (mailbox)
>>> +   (imap-message-copyuid-or-appenduid-1 mailbox t))
>>> + 
>>> + (defun imap-message-appenduid-1 (mailbox)
>>> +   (imap-message-copyuid-or-appenduid-1 mailbox nil))
>>> + 
>>> + (defun imap-message-copyuid-or-appenduid-1 (mailbox copy)
>>> (if (imap-capability 'UIDPLUS)
>>> !       (if copy
>
> RS> I'd rather use
> RS>   (defun imap-message-copyuid-1 (mailbox &optional append)
> RS> or 
> RS>   (defun imap-message-appenduid-1 (mailbox &optional copy)
> RS> and adjust the callers.
>
> Why?  The two functions are clearly mapped to separate IMAP concepts, 

Dunno, I'm not familiar with those.

> so merging them and requiring all the callers to adjust is
> unnecessary pain.

The number of callers is very limited: imap-message-appenduid and
imap-message-append.

I'm not sure if the later should in fact call imap-message-appenduid
instead of imap-message-appenduid-1 (see its use of imap-utf7-encode).

OTOH, keeping imap-message-appenduid-1 as a simple wrapper won't hurt.

How about this?

--- imap.el	08 Jan 2009 21:49:23 +0100	7.52
+++ imap.el	08 Jan 2009 21:55:02 +0100	
@@ -1821,10 +1821,12 @@
 	   (imap-fetch (cdr uids) props receive nouidfetch))
        (signal (car data) (cdr data))))))
 
-(defun imap-message-copyuid-1 (mailbox)
+(defun imap-message-copyuid-1 (mailbox &optional append)
   (if (imap-capability 'UIDPLUS)
-      (list (nth 0 (imap-mailbox-get-1 'copyuid mailbox))
-	    (string-to-number (nth 2 (imap-mailbox-get-1 'copyuid mailbox))))
+      (if append
+	  (imap-mailbox-get-1 'appenduid mailbox)
+	(list (nth 0 (imap-mailbox-get-1 'copyuid mailbox))
+	      (string-to-number (nth 2 (imap-mailbox-get-1 'copyuid mailbox)))))
     (let ((old-mailbox imap-current-mailbox)
 	  (state imap-state)
 	  (imap-message-data (make-vector 2 0)))
@@ -1866,23 +1868,8 @@
 	    (or no-copyuid
 		(imap-message-copyuid-1 mailbox)))))))
 
-;; FIXME: Amalgamate with imap-message-copyuid-1, using an extra arg, since it
-;; shares most of the code?  -- fx
 (defun imap-message-appenduid-1 (mailbox)
-  (if (imap-capability 'UIDPLUS)
-      (imap-mailbox-get-1 'appenduid mailbox)
-    (let ((old-mailbox imap-current-mailbox)
-	  (state imap-state)
-	  (imap-message-data (make-vector 2 0)))
-      (when (imap-mailbox-examine-1 mailbox)
-	(prog1
-	    (and (imap-fetch-safe '("*" . "*:*") "UID")
-		 (list (imap-mailbox-get-1 'uidvalidity mailbox)
-		       (apply 'max (imap-message-map
-				    (lambda (uid prop) uid) 'UID))))
-	  (if old-mailbox
-	      (imap-mailbox-select old-mailbox (eq state 'examine))
-	    (imap-mailbox-unselect)))))))
+  (imap-message-copyuid-1 mailbox t))
 
 (defun imap-message-appenduid (mailbox &optional buffer)
   (with-current-buffer (or buffer (current-buffer))


Bye, Reiner.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/