[PATCH] Use IMAP MOVE extension if available

[PATCH] Use IMAP MOVE extension if available

[Nikolaus Rath]

[-- Attachment #1: Type: text/plain, Size: 556 bytes --]

Hello,

Currently Gnus uses a combination of COPY, STORE + EXPUNGE to move
messages from one imap group into another.

The attached patch enables the use of the MOVE command instead if the
server supports it.

Thanks for considering.

(Is this the right place to post patches, or should it go to
bug-gnu-emacs@gnu.org?)

Best,
-Nikolaus

-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: use_imap_move.diff --]
[-- Type: text/x-diff, Size: 1554 bytes --]

--- nnimap.el.bak	2015-05-25 11:04:15.671769838 -0700
+++ nnimap.el	2015-05-25 12:58:27.051313588 -0700
@@ -881,17 +882,19 @@
       ;; way.
       (let ((message-id (message-field-value "message-id")))
 	(if internal-move-group
-	    (let ((result
-		   (with-current-buffer (nnimap-buffer)
-		     (nnimap-command "UID COPY %d %S"
-				     article
-				     (utf7-encode internal-move-group t)))))
-	      (when (car result)
-		(nnimap-delete-article article)
-		(cons internal-move-group
-		      (or (nnimap-find-uid-response "COPYUID" (cadr result))
-			  (nnimap-find-article-by-message-id
-			   internal-move-group server message-id
+            (with-current-buffer (nnimap-buffer)
+              (let* ((can-move (nnimap-capability "MOVE"))
+                    (command (if can-move
+                                 "UID MOVE %d %S"
+                               "UID COPY %d %S"))
+                    (result (nnimap-command command article
+                                            (utf7-encode internal-move-group t))))
+                (when (and (car result) (not can-move))
+                  (nnimap-delete-article article))
+                (cons internal-move-group
+                      (or (nnimap-find-uid-response "COPYUID" (cadr result))
+                          (nnimap-find-article-by-message-id
+                           internal-move-group server message-id
                            nnimap-request-articles-find-limit)))))
 	  ;; Move the article to a different method.
 	  (let ((result (eval accept-form)))

Re: [PATCH] Use IMAP MOVE extension if available

[Eric Abrahamsen]

Nikolaus Rath <Nikolaus@rath.org> writes:

> Hello,
>
> Currently Gnus uses a combination of COPY, STORE + EXPUNGE to move
> messages from one imap group into another.
>
> The attached patch enables the use of the MOVE command instead if the
> server supports it.
>
> Thanks for considering.
>
> (Is this the right place to post patches, or should it go to
> bug-gnu-emacs@gnu.org?)
>
> Best,
> -Nikolaus

This will be great to have -- would you consider putting the same check
in for `nnimap-split-incoming-mail'?

Thanks,
Eric

Re: [PATCH] Use IMAP MOVE extension if available

[Eric Abrahamsen]

Nikolaus Rath <Nikolaus@rath.org> writes:

> Hello,
>
> Currently Gnus uses a combination of COPY, STORE + EXPUNGE to move
> messages from one imap group into another.
>
> The attached patch enables the use of the MOVE command instead if the
> server supports it.
>
> Thanks for considering.
>
> (Is this the right place to post patches, or should it go to
> bug-gnu-emacs@gnu.org?)
>
> Best,
> -Nikolaus

and `nnimap-process-expiry-target', sorry...

Re: [PATCH] Use IMAP MOVE extension if available

[Nikolaus Rath]

[-- Attachment #1: Type: text/plain, Size: 906 bytes --]

On Jul 06 2015, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
> Nikolaus Rath <Nikolaus@rath.org> writes:
>
>> Hello,
>>
>> Currently Gnus uses a combination of COPY, STORE + EXPUNGE to move
>> messages from one imap group into another.
>>
>> The attached patch enables the use of the MOVE command instead if the
>> server supports it.
>>
>> Thanks for considering.
>>
>> (Is this the right place to post patches, or should it go to
>> bug-gnu-emacs@gnu.org?)
>>
>> Best,
>> -Nikolaus
>
> and `nnimap-process-expiry-target', sorry...

Updated patch is attached. Note that the changes to
nnimap-split-incoming-mail are untested (I don't use that function).

Best,
-Nikolaus

-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-IMAP-MOVE-extension-if-available.patch --]
[-- Type: text/x-diff, Size: 5723 bytes --]

From 2cd9616461e3551e3e2e556879fb3e83846754b7 Mon Sep 17 00:00:00 2001
From: Nikolaus Rath <Nikolaus@rath.org>
Date: Wed, 8 Jul 2015 19:10:46 -0700
Subject: [PATCH] Use IMAP MOVE extension if available

Currently Gnus uses a combination of COPY, STORE + EXPUNGE to move
messages from one IMAP group into another.

This patch enables the use of the MOVE command instead if the
server supports it.
---
 lisp/nnimap.el | 59 ++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 35 insertions(+), 24 deletions(-)

diff --git a/lisp/nnimap.el b/lisp/nnimap.el
index cc1691b..db1a91a 100644
--- a/lisp/nnimap.el
+++ b/lisp/nnimap.el
@@ -960,17 +960,19 @@ If non-nil, articles flagged as deleted (using the IMAP
       ;; way.
       (let ((message-id (message-field-value "message-id")))
 	(if internal-move-group
-	    (let ((result
-		   (with-current-buffer (nnimap-buffer)
-		     (nnimap-command "UID COPY %d %S"
-				     article
-				     (utf7-encode internal-move-group t)))))
-	      (when (car result)
-		(nnimap-delete-article article)
-		(cons internal-move-group
-		      (or (nnimap-find-uid-response "COPYUID" (cadr result))
-			  (nnimap-find-article-by-message-id
-			   internal-move-group server message-id
+            (with-current-buffer (nnimap-buffer)
+              (let* ((can-move (nnimap-capability "MOVE"))
+                    (command (if can-move
+                                 "UID MOVE %d %S"
+                               "UID COPY %d %S"))
+                    (result (nnimap-command command article
+                                            (utf7-encode internal-move-group t))))
+                (when (and (car result) (not can-move))
+                  (nnimap-delete-article article))
+                (cons internal-move-group
+                      (or (nnimap-find-uid-response "COPYUID" (cadr result))
+                          (nnimap-find-article-by-message-id
+                           internal-move-group server message-id
                            nnimap-request-articles-find-limit)))))
 	  ;; Move the article to a different method.
 	  (let ((result (eval accept-form)))
@@ -1009,11 +1011,12 @@ If non-nil, articles flagged as deleted (using the IMAP
 	(gnus-sorted-complement articles deletable-articles))))))
 
 (defun nnimap-process-expiry-targets (articles group server)
-  (let ((deleted-articles nil))
+  (let ((deleted-articles nil)
+        (articles-to-delete nil))
     (cond
      ;; shortcut further processing if we're going to delete the articles
      ((eq nnmail-expiry-target 'delete)
-      (setq deleted-articles articles)
+      (setq articles-to-delete articles)
       t)
      ;; or just move them to another folder on the same IMAP server
      ((and (not (functionp nnmail-expiry-target))
@@ -1023,11 +1026,14 @@ If non-nil, articles flagged as deleted (using the IMAP
       (and (nnimap-change-group group server)
 	   (with-current-buffer (nnimap-buffer)
 	     (nnheader-message 7 "Expiring articles from %s: %s" group articles)
-	     (nnimap-command
-	      "UID COPY %s %S"
-	      (nnimap-article-ranges (gnus-compress-sequence articles))
-	      (utf7-encode (gnus-group-real-name nnmail-expiry-target) t))
-	     (setq deleted-articles articles)))
+             (let ((can-move (nnimap-capability "MOVE")))
+               (nnimap-command
+                (if can-move
+                    "UID MOVE %s %S"
+                  "UID COPY %s %S")
+                (nnimap-article-ranges (gnus-compress-sequence articles))
+                (utf7-encode (gnus-group-real-name nnmail-expiry-target) t))
+               (set (if can-move 'deleted-articles 'articles-to-delete) articles))))
       t)
      (t
       (dolist (article articles)
@@ -1048,11 +1054,13 @@ If non-nil, articles flagged as deleted (using the IMAP
 		    (setq target nil))
 		(nnheader-message 7 "Expiring article %s:%d" group article))
 	      (when target
-		(push article deleted-articles))))))
-      (setq deleted-articles (nreverse deleted-articles))))
+		(push article articles-to-delete))))))
+      (setq articles-to-delete (nreverse articles-to-delete))))
     ;; Change back to the current group again.
     (nnimap-change-group group server)
-    (nnimap-delete-article (gnus-compress-sequence deleted-articles))
+    (when articles-to-delete
+      (nnimap-delete-article (gnus-compress-sequence articles-to-delete))
+      (setq deleted-articles articles-to-delete))
     deleted-articles))
 
 (defun nnimap-find-expired-articles (group)
@@ -2103,6 +2111,7 @@ Return the server's response to the SELECT or EXAMINE command."
 				  nnmail-split-fancy))
 	  (nnmail-inhibit-default-split-group t)
 	  (groups (nnimap-get-groups))
+          (can-move (nnimap-capability "MOVE"))
 	  new-articles)
       (erase-buffer)
       (nnimap-command "SELECT %S" nnimap-inbox)
@@ -2137,14 +2146,16 @@ Return the server's response to the SELECT or EXAMINE command."
 		  ;; Don't copy if the message is already in its
 		  ;; target group.
 		  (unless (string= group nnimap-inbox)
-		   (push (list (nnimap-send-command
-				"UID COPY %s %S"
+                    (push (list (nnimap-send-command
+                                 (if can-move
+                                     "UID MOVE %d %S"
+                                     "UID COPY %s %S")
 				(nnimap-article-ranges ranges)
 				(utf7-encode group t))
 			       ranges)
 			 sequences)))))
 	    ;; Wait for the last COPY response...
-	    (when sequences
+	    (when (and (not can-move) sequences)
 	      (nnimap-wait-for-response (caar sequences))
 	      ;; And then mark the successful copy actions as deleted,
 	      ;; and possibly expunge them.
-- 
2.1.4

Re: [PATCH] Use IMAP MOVE extension if available

[Eric Abrahamsen]

Nikolaus Rath <Nikolaus@rath.org> writes:

> On Jul 06 2015, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>> Nikolaus Rath <Nikolaus@rath.org> writes:
>>
>>> Hello,
>>>
>>> Currently Gnus uses a combination of COPY, STORE + EXPUNGE to move
>>> messages from one imap group into another.
>>>
>>> The attached patch enables the use of the MOVE command instead if the
>>> server supports it.
>>>
>>> Thanks for considering.
>>>
>>> (Is this the right place to post patches, or should it go to
>>> bug-gnu-emacs@gnu.org?)
>>>
>>> Best,
>>> -Nikolaus
>>
>> and `nnimap-process-expiry-target', sorry...
>
> Updated patch is attached. Note that the changes to
> nnimap-split-incoming-mail are untested (I don't use that function).

Thanks, I'll run this for a while to test it.

Eric

Re: [PATCH] Use IMAP MOVE extension if available

[Eric Abrahamsen]

Nikolaus Rath <Nikolaus@rath.org> writes:

> On Aug 04 2015, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>> Nikolaus Rath <Nikolaus@rath.org> writes:
>>
>>> On Aug 03 2015, Steinar Bang <sb@dod.no> wrote:
>>>>>>>>> Dan Christensen <jdc@uwo.ca>:
>>>>
>>>>> I believe that Gnus has to re-login now and then, e.g. when connections
>>>>> are idle for too long.
>>>>
>>>> Ok, that may be a case for optimization: the CAPABILITIES may change
>>>> before and after login, but it is not very likely that they will change
>>>> for the rest of the session.
>>>>
>>>> Ie. issue CAPABILITIES only after the initial login.
>>>
>>> Preserving capabilities from one connection to the next would require
>>> some major coding effort. In that case we're still better of trying very
>>> hard to extract the capabilities from the LOGIN response.
>>
>> I'd say we ought to leave it the way it is now, until we've got a better
>> parser for command responses.
>
> I hope with "the way it is now", you mean with either your or my patch
> applied?
>
> Without them, capabilities are not detected at all in many cases. I
> think fixing that should not wait until the response parser is fixed.

Your patch is already applied! That's "the way it is now". :)

Re: [PATCH] Use IMAP MOVE extension if available

[Nikolaus Rath]

On Aug 04 2015, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
> Nikolaus Rath <Nikolaus@rath.org> writes:
>
>> On Aug 03 2015, Steinar Bang <sb@dod.no> wrote:
>>>>>>>> Dan Christensen <jdc@uwo.ca>:
>>>
>>>> I believe that Gnus has to re-login now and then, e.g. when connections
>>>> are idle for too long.
>>>
>>> Ok, that may be a case for optimization: the CAPABILITIES may change
>>> before and after login, but it is not very likely that they will change
>>> for the rest of the session.
>>>
>>> Ie. issue CAPABILITIES only after the initial login.
>>
>> Preserving capabilities from one connection to the next would require
>> some major coding effort. In that case we're still better of trying very
>> hard to extract the capabilities from the LOGIN response.
>
> I'd say we ought to leave it the way it is now, until we've got a better
> parser for command responses.

I hope with "the way it is now", you mean with either your or my patch
applied?

Without them, capabilities are not detected at all in many cases. I
think fixing that should not wait until the response parser is fixed.


Best,
-Nikolaus

-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

Re: [PATCH] Use IMAP MOVE extension if available

[Eric Abrahamsen]

Nikolaus Rath <Nikolaus@rath.org> writes:

> On Aug 03 2015, Steinar Bang <sb@dod.no> wrote:
>>>>>>> Dan Christensen <jdc@uwo.ca>:
>>
>>> I believe that Gnus has to re-login now and then, e.g. when connections
>>> are idle for too long.
>>
>> Ok, that may be a case for optimization: the CAPABILITIES may change
>> before and after login, but it is not very likely that they will change
>> for the rest of the session.
>>
>> Ie. issue CAPABILITIES only after the initial login.
>
> Preserving capabilities from one connection to the next would require
> some major coding effort. In that case we're still better of trying very
> hard to extract the capabilities from the LOGIN response.

I'd say we ought to leave it the way it is now, until we've got a better
parser for command responses.

E

Re: [PATCH] Use IMAP MOVE extension if available

[Nikolaus Rath]

On Aug 03 2015, Steinar Bang <sb@dod.no> wrote:
>>>>>> Dan Christensen <jdc@uwo.ca>:
>
>> I believe that Gnus has to re-login now and then, e.g. when connections
>> are idle for too long.
>
> Ok, that may be a case for optimization: the CAPABILITIES may change
> before and after login, but it is not very likely that they will change
> for the rest of the session.
>
> Ie. issue CAPABILITIES only after the initial login.

Preserving capabilities from one connection to the next would require
some major coding effort. In that case we're still better of trying very
hard to extract the capabilities from the LOGIN response.

Best,
-Nikolaus

-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

Re: [PATCH] Use IMAP MOVE extension if available

[Steinar Bang]

>>>>> Dan Christensen <jdc@uwo.ca>:

> I believe that Gnus has to re-login now and then, e.g. when connections
> are idle for too long.

Ok, that may be a case for optimization: the CAPABILITIES may change
before and after login, but it is not very likely that they will change
for the rest of the session.

Ie. issue CAPABILITIES only after the initial login.

Re: [PATCH] Use IMAP MOVE extension if available

[Dan Christensen]

Nikolaus Rath <Nikolaus@rath.org> writes:

> I don't understand this worry about one additional back and forth during
> *login*. For me, starting Gnus takes about 2 seconds, and issuing the
> CAPABILITY command takes about 160 ms. Is it really worth optimizing
> this?

I believe that Gnus has to re-login now and then, e.g. when connections
are idle for too long.  Logging in to my remote IMAP server currently
takes a bit under 1s, so the above could presumably add 10 to 20% to the
time taken to open a group.  If it's possible to avoid the round-trip
when it isn't needed, it seems like it would be a good idea.  (I haven't
followed the details of this discussion, so I don't know how hard it
would be to implement this.)

Dan

Re: [PATCH] Use IMAP MOVE extension if available

[Eric Abrahamsen]

Nikolaus Rath <Nikolaus@rath.org> writes:

> On Aug 02 2015, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>> Nikolaus Rath <Nikolaus@rath.org> writes:
>>
>>> On Jul 27 2015, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>>>> diff --git a/lisp/nnimap.el b/lisp/nnimap.el
>>>> index 161a6b4..f8e6e3e 100644
>>>> --- a/lisp/nnimap.el
>>>> +++ b/lisp/nnimap.el
>>>> @@ -489,11 +489,18 @@ textual parts.")
>>>>  		      (when (functionp (nth 2 credentials))
>>>>  			(funcall (nth 2 credentials)))
>>>>  		      ;; See if CAPABILITY is set as part of login
>>>> -		      ;; response.
>>>> -		      (dolist (response (cddr login-result))
>>>> -			(when (string= "CAPABILITY" (upcase (car response)))
>>>> -			  (setf (nnimap-capabilities nnimap-object)
>>>> -				(mapcar #'upcase (cdr response))))))
>>>> +		      ;; response.  If it wasn't, specifically request
>>>> +		      ;; it.
>>>> +		      (when
>>>> +			  (catch 'caps
>>>> +			    (dolist (response (cddr login-result))
>>>> +			      (when (string= "CAPABILITY" (upcase (car response)))
>>>> +				(throw 'caps (setq capabilities (cdr response)))))
>>>> +			    (let ((req (nnimap-command "CAPABILITY")))
>>>> +			      (when (car req)
>>>> +				(throw 'caps (setq capabilities (cdaddr req))))))
>>>> +			(setf (nnimap-capabilities nnimap-object)
>>>> +			      (mapcar #'upcase capabilities))))
>>>>  		  ;; If the login failed, then forget the credentials
>>>>  		  ;; that are now possibly cached.
>>>>  		  (dolist (host (list (nnoo-current-server 'nnimap)
>>>
>>>
>>> This looks a lot more complex, but as far as I can tell it still
>>> needlessy asks for capabilities if they have been supplied in the
>>> response code of OK response instead of a in separate, untagged
>>> response. So is it really worth the extra complexity, if in many cases
>>> we still issue an explicit CAPABILITY command?
>>>
>>> (For everyone unfamiliar with RFC3501, what I mean is that this:
>>>
>>> * CAPABILITY IMAP4rev1 AND SOME MORE \r\n
>>> 1 OK Logged in\r\n
>>>
>>> will be parsed correcly but this:
>>>
>>> 1 OK [CAPABILITY IMAP4rev1 AND SOME MORE] Logged in\r\n
>>>
>>> will still cause Gnus to issue a separate CAPABILITY command).
>>
>> Okay, I pushed the CAPABILITY patch, and the MOVE patch right after
>> that. Sorry this is all going so slowly!
>
> Don't worry, I'm not in a rush. Luckily I can run the patches locally
> until they applied upstream :-).
>
>> I still think that we'll be able to come up with something that doesn't
>> unconditionally issue an extra CAPABILITY command. My patch was more
>> complex, but at least it avoided redundancy in *some* cases -- now we're
>> always doing the extra command.
>
> I don't understand this worry about one additional back and forth during
> *login*. For me, starting Gnus takes about 2 seconds, and issuing the
> CAPABILITY command takes about 160 ms. Is it really worth optimizing
> this?

No, I guess you're right. It just irks, a little.

>> Are you sure that `nnimap-parse-response' and `nnimap-parse-line' can't
>> handle the bracket case above?
>
> Depends what you mean. The data does end up in the list that's returned
> by nnimap-parse-response, because (as the comment says), it just
> "lightly" converts the response to a list of strings and lists of
> string. The problem is the "lightly" - apparently this function
> "succeeds" no matter what data you give it, even if this data completely
> fails the assumptions that went into writing that function. Looking at
> the code, I am not able to tell (in general) at which position the
> response code is going to be. So the best I can do is just use the
> location at which it ends up with my server - but I don't think that's a
> good idea.
>
> That's also the reason why I did not use nnimap-parse-response at all
> for parsing the NAMESPACE command (in my other patch). In that case,
> "parsing" the return value of nnimap-parse-response is actually a lot
> harder than parsing the IMAP response directly. For example, the
> nnimap-parse-response return value contains strings with opening parens,
> but no strings with the corresponding closing parens.
>
>> I haven't had time to test yet, but looking at the code it seems like
>> it ought to be able to handle it. If there are multiple commonly-used
>> formats for IMAP server responses, it seems like we really ought to be
>> handling them at the base level, when parsing responses.
>
> Making nnimap-parse-response into a proper parser would certainly be a
> good idea, but that's probably better left as a separate patch.

Oh certainly -- I'm not in any hurry to get stuck into that! But at some
point, yes, it would probably be nice.

>> I'll admit I only started learning about IMAP about six months ago, so
>> I'm happy to take other people's word on how all this works. Out of
>> curiosity, is there a particular server that returns the kind of
>> response you note above? I suppose we should be making a list of
>> known-good IMAP servers that Gnus can interact happily with.
>
> This is mail.messagingengine.com, the IMAP server from
> www.fastmail.com. IIRC they are using Cyrus.

That's pretty mainstream stuff, and I've heard Cyrus referred to as one
of the "sane" IMAP servers, so we should definitely be trying to play
nice with it...

Thanks,
Eric

Re: [PATCH] Use IMAP MOVE extension if available

[Nikolaus Rath]

On Aug 02 2015, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
> Nikolaus Rath <Nikolaus@rath.org> writes:
>
>> On Jul 27 2015, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>>> diff --git a/lisp/nnimap.el b/lisp/nnimap.el
>>> index 161a6b4..f8e6e3e 100644
>>> --- a/lisp/nnimap.el
>>> +++ b/lisp/nnimap.el
>>> @@ -489,11 +489,18 @@ textual parts.")
>>>  		      (when (functionp (nth 2 credentials))
>>>  			(funcall (nth 2 credentials)))
>>>  		      ;; See if CAPABILITY is set as part of login
>>> -		      ;; response.
>>> -		      (dolist (response (cddr login-result))
>>> -			(when (string= "CAPABILITY" (upcase (car response)))
>>> -			  (setf (nnimap-capabilities nnimap-object)
>>> -				(mapcar #'upcase (cdr response))))))
>>> +		      ;; response.  If it wasn't, specifically request
>>> +		      ;; it.
>>> +		      (when
>>> +			  (catch 'caps
>>> +			    (dolist (response (cddr login-result))
>>> +			      (when (string= "CAPABILITY" (upcase (car response)))
>>> +				(throw 'caps (setq capabilities (cdr response)))))
>>> +			    (let ((req (nnimap-command "CAPABILITY")))
>>> +			      (when (car req)
>>> +				(throw 'caps (setq capabilities (cdaddr req))))))
>>> +			(setf (nnimap-capabilities nnimap-object)
>>> +			      (mapcar #'upcase capabilities))))
>>>  		  ;; If the login failed, then forget the credentials
>>>  		  ;; that are now possibly cached.
>>>  		  (dolist (host (list (nnoo-current-server 'nnimap)
>>
>>
>> This looks a lot more complex, but as far as I can tell it still
>> needlessy asks for capabilities if they have been supplied in the
>> response code of OK response instead of a in separate, untagged
>> response. So is it really worth the extra complexity, if in many cases
>> we still issue an explicit CAPABILITY command?
>>
>> (For everyone unfamiliar with RFC3501, what I mean is that this:
>>
>> * CAPABILITY IMAP4rev1 AND SOME MORE \r\n
>> 1 OK Logged in\r\n
>>
>> will be parsed correcly but this:
>>
>> 1 OK [CAPABILITY IMAP4rev1 AND SOME MORE] Logged in\r\n
>>
>> will still cause Gnus to issue a separate CAPABILITY command).
>
> Okay, I pushed the CAPABILITY patch, and the MOVE patch right after
> that. Sorry this is all going so slowly!

Don't worry, I'm not in a rush. Luckily I can run the patches locally
until they applied upstream :-).

> I still think that we'll be able to come up with something that doesn't
> unconditionally issue an extra CAPABILITY command. My patch was more
> complex, but at least it avoided redundancy in *some* cases -- now we're
> always doing the extra command.

I don't understand this worry about one additional back and forth during
*login*. For me, starting Gnus takes about 2 seconds, and issuing the
CAPABILITY command takes about 160 ms. Is it really worth optimizing
this?

> Are you sure that `nnimap-parse-response' and `nnimap-parse-line' can't
> handle the bracket case above?

Depends what you mean. The data does end up in the list that's returned
by nnimap-parse-response, because (as the comment says), it just
"lightly" converts the response to a list of strings and lists of
string. The problem is the "lightly" - apparently this function
"succeeds" no matter what data you give it, even if this data completely
fails the assumptions that went into writing that function. Looking at
the code, I am not able to tell (in general) at which position the
response code is going to be. So the best I can do is just use the
location at which it ends up with my server - but I don't think that's a
good idea.

That's also the reason why I did not use nnimap-parse-response at all
for parsing the NAMESPACE command (in my other patch). In that case,
"parsing" the return value of nnimap-parse-response is actually a lot
harder than parsing the IMAP response directly. For example, the
nnimap-parse-response return value contains strings with opening parens,
but no strings with the corresponding closing parens.

> I haven't had time to test yet, but looking at the code it seems like
> it ought to be able to handle it. If there are multiple commonly-used
> formats for IMAP server responses, it seems like we really ought to be
> handling them at the base level, when parsing responses.

Making nnimap-parse-response into a proper parser would certainly be a
good idea, but that's probably better left as a separate patch.

> I'll admit I only started learning about IMAP about six months ago, so
> I'm happy to take other people's word on how all this works. Out of
> curiosity, is there a particular server that returns the kind of
> response you note above? I suppose we should be making a list of
> known-good IMAP servers that Gnus can interact happily with.

This is mail.messagingengine.com, the IMAP server from
www.fastmail.com. IIRC they are using Cyrus.

Best,
-Nikolaus

-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

Re: [PATCH] Use IMAP MOVE extension if available

[Eric Abrahamsen]

Nikolaus Rath <Nikolaus@rath.org> writes:

> On Jul 27 2015, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>> diff --git a/lisp/nnimap.el b/lisp/nnimap.el
>> index 161a6b4..f8e6e3e 100644
>> --- a/lisp/nnimap.el
>> +++ b/lisp/nnimap.el
>> @@ -489,11 +489,18 @@ textual parts.")
>>  		      (when (functionp (nth 2 credentials))
>>  			(funcall (nth 2 credentials)))
>>  		      ;; See if CAPABILITY is set as part of login
>> -		      ;; response.
>> -		      (dolist (response (cddr login-result))
>> -			(when (string= "CAPABILITY" (upcase (car response)))
>> -			  (setf (nnimap-capabilities nnimap-object)
>> -				(mapcar #'upcase (cdr response))))))
>> +		      ;; response.  If it wasn't, specifically request
>> +		      ;; it.
>> +		      (when
>> +			  (catch 'caps
>> +			    (dolist (response (cddr login-result))
>> +			      (when (string= "CAPABILITY" (upcase (car response)))
>> +				(throw 'caps (setq capabilities (cdr response)))))
>> +			    (let ((req (nnimap-command "CAPABILITY")))
>> +			      (when (car req)
>> +				(throw 'caps (setq capabilities (cdaddr req))))))
>> +			(setf (nnimap-capabilities nnimap-object)
>> +			      (mapcar #'upcase capabilities))))
>>  		  ;; If the login failed, then forget the credentials
>>  		  ;; that are now possibly cached.
>>  		  (dolist (host (list (nnoo-current-server 'nnimap)
>
>
> This looks a lot more complex, but as far as I can tell it still
> needlessy asks for capabilities if they have been supplied in the
> response code of OK response instead of a in separate, untagged
> response. So is it really worth the extra complexity, if in many cases
> we still issue an explicit CAPABILITY command?
>
> (For everyone unfamiliar with RFC3501, what I mean is that this:
>
> * CAPABILITY IMAP4rev1 AND SOME MORE \r\n
> 1 OK Logged in\r\n
>
> will be parsed correcly but this:
>
> 1 OK [CAPABILITY IMAP4rev1 AND SOME MORE] Logged in\r\n
>
> will still cause Gnus to issue a separate CAPABILITY command).

Okay, I pushed the CAPABILITY patch, and the MOVE patch right after
that. Sorry this is all going so slowly!

I still think that we'll be able to come up with something that doesn't
unconditionally issue an extra CAPABILITY command. My patch was more
complex, but at least it avoided redundancy in *some* cases -- now we're
always doing the extra command.

Are you sure that `nnimap-parse-response' and `nnimap-parse-line' can't
handle the bracket case above? I haven't had time to test yet, but
looking at the code it seems like it ought to be able to handle it. If
there are multiple commonly-used formats for IMAP server responses, it
seems like we really ought to be handling them at the base level, when
parsing responses.

I'll admit I only started learning about IMAP about six months ago, so
I'm happy to take other people's word on how all this works. Out of
curiosity, is there a particular server that returns the kind of
response you note above? I suppose we should be making a list of
known-good IMAP servers that Gnus can interact happily with.

Yours,
Eric

Re: [PATCH] Use IMAP MOVE extension if available

[Nikolaus Rath]

On Jul 27 2015, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
> diff --git a/lisp/nnimap.el b/lisp/nnimap.el
> index 161a6b4..f8e6e3e 100644
> --- a/lisp/nnimap.el
> +++ b/lisp/nnimap.el
> @@ -489,11 +489,18 @@ textual parts.")
>  		      (when (functionp (nth 2 credentials))
>  			(funcall (nth 2 credentials)))
>  		      ;; See if CAPABILITY is set as part of login
> -		      ;; response.
> -		      (dolist (response (cddr login-result))
> -			(when (string= "CAPABILITY" (upcase (car response)))
> -			  (setf (nnimap-capabilities nnimap-object)
> -				(mapcar #'upcase (cdr response))))))
> +		      ;; response.  If it wasn't, specifically request
> +		      ;; it.
> +		      (when
> +			  (catch 'caps
> +			    (dolist (response (cddr login-result))
> +			      (when (string= "CAPABILITY" (upcase (car response)))
> +				(throw 'caps (setq capabilities (cdr response)))))
> +			    (let ((req (nnimap-command "CAPABILITY")))
> +			      (when (car req)
> +				(throw 'caps (setq capabilities (cdaddr req))))))
> +			(setf (nnimap-capabilities nnimap-object)
> +			      (mapcar #'upcase capabilities))))
>  		  ;; If the login failed, then forget the credentials
>  		  ;; that are now possibly cached.
>  		  (dolist (host (list (nnoo-current-server 'nnimap)


This looks a lot more complex, but as far as I can tell it still
needlessy asks for capabilities if they have been supplied in the
response code of OK response instead of a in separate, untagged
response. So is it really worth the extra complexity, if in many cases
we still issue an explicit CAPABILITY command?

(For everyone unfamiliar with RFC3501, what I mean is that this:

* CAPABILITY IMAP4rev1 AND SOME MORE \r\n
1 OK Logged in\r\n

will be parsed correcly but this:

1 OK [CAPABILITY IMAP4rev1 AND SOME MORE] Logged in\r\n

will still cause Gnus to issue a separate CAPABILITY command).


Best,
-Nikolaus
-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

Re: [PATCH] Use IMAP MOVE extension if available

[Eric Abrahamsen]

[-- Attachment #1: Type: text/plain, Size: 2563 bytes --]

Nikolaus Rath <Nikolaus@rath.org> writes:

> On Jul 20 2015, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
>> Nikolaus Rath <Nikolaus@rath.org> writes:
>>
>>> Hello,
>>>
>>> The attached revision fixes a small error when parsing the response to
>>> the UID MOVE command. Previously, it would fail to find the COPYUID
>>> response and fall back to using the message id to determine the UID of
>>> the copied message.
>>
>> I'm running these patches locally, and (I think) seeing more hangs than
>> I used to. This only happens on first startup of Gnus. I set
>> toggle-debug-on-quit to t, and quit on the next hang -- see the
>> traceback below.
>>
>> This is connecting to a dovecot server running on localhost. The first
>> thing this tells me is that the MOVE capability (which dovecot provides)
>> isn't getting noticed by Gnus -- as you noted in your previous exchange
>> with Lars (was there no resolution to that?). That does need to get
>> fixed, otherwise Gnus is missing a lot of added functionality.
>
> I think I've addressed Lars' comments, so for me the proper resolution
> would be for someone to apply the patch :-).

How does the attached patch look, instead? It's more code, but the
behavior should be cleaner.

>
>> The other thing is this hang. I'm not immediately inclined to blame Gnus
>> for this, but the fact is that this a local connection and there
>> shouldn't be anything stalling it. If anyone has any bright ideas...
>>
>>   nnimap-wait-for-response(85)
>>   nnimap-get-response(85)
>>   nnimap-command("UID STORE %s +FLAGS.SILENT (\\Deleted)" "")
>>   nnimap-delete-article(nil t)
>
> What's the content of the " *nnimap..." buffer at this point? (note the
> leading space).

It appears I wasn't seeing the problem I thought I was seeing. I traced
the login procedure (many times), and it turns out this is
`gnus-request-scan' for a server that doesn't support MOVE
(outlook.com), so that much is correct. The problem is that the server
connection often dies instantly, before the LOGIN is issued, which
throws the whole connection process off. At the very least, the wait for
response is being called on a dead connection. What's worse is that Gnus
seems to be getting confused about which server it's acting on: the
traceback above claims to be for the server "PR", but it's actually
trying to talk to the server "Outlook". I don't really understand what's
happening.

Anyway, this isn't a reason to hold up the MOVE patch, so maybe I'll
chuck in this CAPABILITY one and then do the MOVE. Sorry this is taking
so long.

Eric



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-nnimap.el-Ask-for-CAPABILITY-if-we-didn-t-get-it.patch --]
[-- Type: text/x-diff, Size: 2074 bytes --]

From 5db7b1c66265423f1dbc83301be63771ff4b779b Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Mon, 27 Jul 2015 12:20:08 +0800
Subject: [PATCH] nnimap.el: Ask for CAPABILITY if we didn't get it

* lisp/nnimap.el (nnimap-open-connection-1): If imap servers don't return
	CAPABILITY as part of login, specifically request it.
---
 lisp/ChangeLog |  5 +++++
 lisp/nnimap.el | 17 ++++++++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 46b5310..58f5a08 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,8 @@
+2015-07-27  Eric Abrahamsen  <eric@ericabrahamsen.net>
+
+	* nnimap.el (nnimap-open-connection-1): If imap servers don't return
+	CAPABILITY as part of login, specifically request it.
+
 2015-07-17  Julien Danjou  <jd@abydos>
 
 	* sieve-mode.el (sieve-font-lock-keywords): Add missing "body" test
diff --git a/lisp/nnimap.el b/lisp/nnimap.el
index 161a6b4..f8e6e3e 100644
--- a/lisp/nnimap.el
+++ b/lisp/nnimap.el
@@ -489,11 +489,18 @@ textual parts.")
 		      (when (functionp (nth 2 credentials))
 			(funcall (nth 2 credentials)))
 		      ;; See if CAPABILITY is set as part of login
-		      ;; response.
-		      (dolist (response (cddr login-result))
-			(when (string= "CAPABILITY" (upcase (car response)))
-			  (setf (nnimap-capabilities nnimap-object)
-				(mapcar #'upcase (cdr response))))))
+		      ;; response.  If it wasn't, specifically request
+		      ;; it.
+		      (when
+			  (catch 'caps
+			    (dolist (response (cddr login-result))
+			      (when (string= "CAPABILITY" (upcase (car response)))
+				(throw 'caps (setq capabilities (cdr response)))))
+			    (let ((req (nnimap-command "CAPABILITY")))
+			      (when (car req)
+				(throw 'caps (setq capabilities (cdaddr req))))))
+			(setf (nnimap-capabilities nnimap-object)
+			      (mapcar #'upcase capabilities))))
 		  ;; If the login failed, then forget the credentials
 		  ;; that are now possibly cached.
 		  (dolist (host (list (nnoo-current-server 'nnimap)
-- 
2.4.6

Re: [PATCH] Use IMAP MOVE extension if available

[Nikolaus Rath]

On Jul 20 2015, Eric Abrahamsen <eric@ericabrahamsen.net> wrote:
> Nikolaus Rath <Nikolaus@rath.org> writes:
>
>> Hello,
>>
>> The attached revision fixes a small error when parsing the response to
>> the UID MOVE command. Previously, it would fail to find the COPYUID
>> response and fall back to using the message id to determine the UID of
>> the copied message.
>
> I'm running these patches locally, and (I think) seeing more hangs than
> I used to. This only happens on first startup of Gnus. I set
> toggle-debug-on-quit to t, and quit on the next hang -- see the
> traceback below.
>
> This is connecting to a dovecot server running on localhost. The first
> thing this tells me is that the MOVE capability (which dovecot provides)
> isn't getting noticed by Gnus -- as you noted in your previous exchange
> with Lars (was there no resolution to that?). That does need to get
> fixed, otherwise Gnus is missing a lot of added functionality.

I think I've addressed Lars' comments, so for me the proper resolution
would be for someone to apply the patch :-).


> The other thing is this hang. I'm not immediately inclined to blame Gnus
> for this, but the fact is that this a local connection and there
> shouldn't be anything stalling it. If anyone has any bright ideas...
>
>   nnimap-wait-for-response(85)
>   nnimap-get-response(85)
>   nnimap-command("UID STORE %s +FLAGS.SILENT (\\Deleted)" "")
>   nnimap-delete-article(nil t)

What's the content of the " *nnimap..." buffer at this point? (note the
leading space).


Best,
-Nikolaus

-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

Re: [PATCH] Use IMAP MOVE extension if available

[Eric Abrahamsen]

Nikolaus Rath <Nikolaus@rath.org> writes:

> Hello,
>
> The attached revision fixes a small error when parsing the response to
> the UID MOVE command. Previously, it would fail to find the COPYUID
> response and fall back to using the message id to determine the UID of
> the copied message.

I'm running these patches locally, and (I think) seeing more hangs than
I used to. This only happens on first startup of Gnus. I set
toggle-debug-on-quit to t, and quit on the next hang -- see the
traceback below.

This is connecting to a dovecot server running on localhost. The first
thing this tells me is that the MOVE capability (which dovecot provides)
isn't getting noticed by Gnus -- as you noted in your previous exchange
with Lars (was there no resolution to that?). That does need to get
fixed, otherwise Gnus is missing a lot of added functionality.

The other thing is this hang. I'm not immediately inclined to blame Gnus
for this, but the fact is that this a local connection and there
shouldn't be anything stalling it. If anyone has any bright ideas...

  nnimap-wait-for-response(85)
  nnimap-get-response(85)
  nnimap-command("UID STORE %s +FLAGS.SILENT (\\Deleted)" "")
  nnimap-delete-article(nil t)
  nnimap-split-incoming-mail()
  nnimap-request-scan(nil "PR")
  gnus-request-scan(nil (nnimap "PR" (nnimap-address "localhost")
  (nnimap-user "user") (nnimap-authenticator login) (stuff omitted) (nnimap-stream network) (nnimap-inbox "INBOX")))
  gnus-get-unread-articles(4 nil)
  gnus-setup-news(nil 4 nil)
  ...bytecode omitted...
  gnus-1(4 nil nil)
  gnus(4)

Re: [PATCH] Use IMAP MOVE extension if available

[Eric Abrahamsen]

Nikolaus Rath <Nikolaus@rath.org> writes:

> Hello,
>
> The attached revision fixes a small error when parsing the response to
> the UID MOVE command. Previously, it would fail to find the COPYUID
> response and fall back to using the message id to determine the UID of
> the copied message.

I've incorporated this into the version I'm running locally, and will
push soon!

Re: [PATCH] Use IMAP MOVE extension if available

[Nikolaus Rath]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

Hello,

The attached revision fixes a small error when parsing the response to
the UID MOVE command. Previously, it would fail to find the COPYUID
response and fall back to using the message id to determine the UID of
the copied message.

Best,
-Nikolaus

-- 
GPG encrypted emails preferred. Key id: 0xD113FCAC3C4E599F
Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F

             »Time flies like an arrow, fruit flies like a Banana.«

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-nnimap.el-use-IMAP-MOVE-extension-if-available.patch --]
[-- Type: text/x-diff, Size: 6279 bytes --]

From f7cff6a6068079ecfb31d31986902b5b17cee0b5 Mon Sep 17 00:00:00 2001
From: Nikolaus Rath <Nikolaus@rath.org>
Date: Thu, 9 Jul 2015 20:01:12 -0700
Subject: [PATCH] nnimap.el: use IMAP MOVE extension if available

* lisp/nnimap.el (nnimap-request-move-article)
(nnimap-process-expiry-targets, nnimap-split-incoming-mail): use MOVE
extension if available.
---
 lisp/ChangeLog |  6 ++++++
 lisp/nnimap.el | 59 ++++++++++++++++++++++++++++++++++------------------------
 2 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index d4af26a..58d00ed 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,5 +1,11 @@
 2015-07-11 Nikolaus Rath <Nikolaus@rath.org>
 
+	* nnimap.el (nnimap-request-move-article)
+	(nnimap-process-expiry-targets, nnimap-split-incoming-mail): use MOVE
+	extension if available.
+	
+2015-07-09 Nikolaus Rath <Nikolaus@rath.org>
+
 	* nnimap.el (nnimap-hide-deleted): introduced as new server variable.
 	(nnimap-header-parameters): also request flags.
 	(nnimap-transform-headers): parse flags and conditionally skip over
diff --git a/lisp/nnimap.el b/lisp/nnimap.el
index 2f4a818..f56fb83 100644
--- a/lisp/nnimap.el
+++ b/lisp/nnimap.el
@@ -960,17 +960,19 @@ If non-nil, articles flagged as deleted (using the IMAP
       ;; way.
       (let ((message-id (message-field-value "message-id")))
 	(if internal-move-group
-	    (let ((result
-		   (with-current-buffer (nnimap-buffer)
-		     (nnimap-command "UID COPY %d %S"
-				     article
-				     (utf7-encode internal-move-group t)))))
-	      (when (car result)
-		(nnimap-delete-article article)
-		(cons internal-move-group
-		      (or (nnimap-find-uid-response "COPYUID" (cadr result))
-			  (nnimap-find-article-by-message-id
-			   internal-move-group server message-id
+            (with-current-buffer (nnimap-buffer)
+              (let* ((can-move (nnimap-capability "MOVE"))
+                    (command (if can-move
+                                 "UID MOVE %d %S"
+                               "UID COPY %d %S"))
+                    (result (nnimap-command command article
+                                            (utf7-encode internal-move-group t))))
+                (when (and (car result) (not can-move))
+                  (nnimap-delete-article article))
+                (cons internal-move-group
+                      (or (nnimap-find-uid-response "COPYUID" (caddr result))
+                          (nnimap-find-article-by-message-id
+                           internal-move-group server message-id
                            nnimap-request-articles-find-limit)))))
 	  ;; Move the article to a different method.
 	  (let ((result (eval accept-form)))
@@ -1009,11 +1011,12 @@ If non-nil, articles flagged as deleted (using the IMAP
 	(gnus-sorted-complement articles deletable-articles))))))
 
 (defun nnimap-process-expiry-targets (articles group server)
-  (let ((deleted-articles nil))
+  (let ((deleted-articles nil)
+        (articles-to-delete nil))
     (cond
      ;; shortcut further processing if we're going to delete the articles
      ((eq nnmail-expiry-target 'delete)
-      (setq deleted-articles articles)
+      (setq articles-to-delete articles)
       t)
      ;; or just move them to another folder on the same IMAP server
      ((and (not (functionp nnmail-expiry-target))
@@ -1023,11 +1026,14 @@ If non-nil, articles flagged as deleted (using the IMAP
       (and (nnimap-change-group group server)
 	   (with-current-buffer (nnimap-buffer)
 	     (nnheader-message 7 "Expiring articles from %s: %s" group articles)
-	     (nnimap-command
-	      "UID COPY %s %S"
-	      (nnimap-article-ranges (gnus-compress-sequence articles))
-	      (utf7-encode (gnus-group-real-name nnmail-expiry-target) t))
-	     (setq deleted-articles articles)))
+             (let ((can-move (nnimap-capability "MOVE")))
+               (nnimap-command
+                (if can-move
+                    "UID MOVE %s %S"
+                  "UID COPY %s %S")
+                (nnimap-article-ranges (gnus-compress-sequence articles))
+                (utf7-encode (gnus-group-real-name nnmail-expiry-target) t))
+               (set (if can-move 'deleted-articles 'articles-to-delete) articles))))
       t)
      (t
       (dolist (article articles)
@@ -1048,11 +1054,13 @@ If non-nil, articles flagged as deleted (using the IMAP
 		    (setq target nil))
 		(nnheader-message 7 "Expiring article %s:%d" group article))
 	      (when target
-		(push article deleted-articles))))))
-      (setq deleted-articles (nreverse deleted-articles))))
+		(push article articles-to-delete))))))
+      (setq articles-to-delete (nreverse articles-to-delete))))
     ;; Change back to the current group again.
     (nnimap-change-group group server)
-    (nnimap-delete-article (gnus-compress-sequence deleted-articles))
+    (when articles-to-delete
+      (nnimap-delete-article (gnus-compress-sequence articles-to-delete))
+      (setq deleted-articles articles-to-delete))
     deleted-articles))
 
 (defun nnimap-find-expired-articles (group)
@@ -2103,6 +2111,7 @@ Return the server's response to the SELECT or EXAMINE command."
 				  nnmail-split-fancy))
 	  (nnmail-inhibit-default-split-group t)
 	  (groups (nnimap-get-groups))
+          (can-move (nnimap-capability "MOVE"))
 	  new-articles)
       (erase-buffer)
       (nnimap-command "SELECT %S" nnimap-inbox)
@@ -2137,14 +2146,16 @@ Return the server's response to the SELECT or EXAMINE command."
 		  ;; Don't copy if the message is already in its
 		  ;; target group.
 		  (unless (string= group nnimap-inbox)
-		   (push (list (nnimap-send-command
-				"UID COPY %s %S"
+                    (push (list (nnimap-send-command
+                                 (if can-move
+                                     "UID MOVE %d %S"
+                                     "UID COPY %s %S")
 				(nnimap-article-ranges ranges)
 				(utf7-encode group t))
 			       ranges)
 			 sequences)))))
 	    ;; Wait for the last COPY response...
-	    (when sequences
+	    (when (and (not can-move) sequences)
 	      (nnimap-wait-for-response (caar sequences))
 	      ;; And then mark the successful copy actions as deleted,
 	      ;; and possibly expunge them.
-- 
2.1.4