Re: [PATCH] Use IMAP MOVE extension if available

[Eric Abrahamsen <eric@ericabrahamsen.net>]

[-- 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