bug#20670: [PATCH] nnimap: enable additional expunge options (v3)

[Nikolaus Rath]

On Jan 24 2017, Lars Ingebrigtsen <larsi@gnus.org> wrote:
> Nikolaus Rath <Nikolaus@rath.org> writes:
>
>> If the IMAP server supports the UID EXPUNGE command, the nnimap backend
>> currently always expunges deleted articles.
>>
>> If the IMAP server does not support this command, articles are either
>> not expunged at all (nnimap-expunge is nil), or ALL articles that are
>> currently flagged as deleted will be expunged when any one article is
>> deleted in Gnus.
>>
>> This patch introduces three new settings for nnimap-expunge:
>
> Looks good.  Two minor comments:
>
> +               (nnheader-message 3 (concat "nnimap-expunge set to 
> 'immediately, but "
> +                                           "server doesn't support 
> UIDPLUS"))))
> +
>
> You've probably done this to make the lines short enough, but the first
> one is still more than 80 chars wide.
>
> +             (nnheader-message 7 "Article marked for deletion, but not 
> expunged.")))))
>
> And so is this.  It's probably better to just reflow by breaking the
> line after `nnheader-message'.
>
> This change should also be documented in the "Customizing the IMAP
> Connection" node in the Gnus manual, and get an entry in the NEWS
> file.

Thanks! Attached is an updated patch.

Btw, is there any way to generate the list of affected functions in the
commit message automatically? Doing this by hand is rather cumbersome...


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.«

>From 3421d2b8c8d3eb20052432d1ba331be886d812a6 Mon Sep 17 00:00:00 2001
From: Nikolaus Rath <Nikolaus@rath.org>
Date: Tue, 24 Jan 2017 15:31:46 -0800
Subject: [PATCH] nnimap.el: support additional expunge options

* lisp/gnus/nnimap.el (nnimap-close-group,
nnimap-request-expire-articles, nnimap-delete-article,
nnimap-request-scan): add new 'never, 'immediate, and 'on-exit
settings for nnimap-expunge.
---
 doc/misc/gnus.texi  | 16 ++++++++--
 etc/NEWS            | 10 +++++++
 lisp/gnus/nnimap.el | 85 ++++++++++++++++++++++++++++++-----------------------
 3 files changed, 72 insertions(+), 39 deletions(-)

diff --git a/doc/misc/gnus.texi b/doc/misc/gnus.texi
index 05159..e8524 100644
--- a/doc/misc/gnus.texi
+++ b/doc/misc/gnus.texi
@@ -14273,9 +14273,19 @@ Customizing the IMAP Connection
 @code{plain} or @code{cram-md5}.
 
 @item nnimap-expunge
-If non-@code{nil}, expunge articles after deleting them.  This is always done
-if the server supports UID EXPUNGE, but it's not done by default on
-servers that doesn't support that command.
+When to expunge deleted messages.  If @code{never}, deleted articles
+are marked with the IMAP @code{\\Delete} flag but not automatically
+expunged. If @code{immediately}, deleted articles are immediately expunged
+(this requires the server to support the UID EXPUNGE command). If
+@code{on-exit}, deleted articles are flagged, and all flagged articles are
+expunged when the group is closed.
+
+For backwards compatibility, this variable may also be set to t
+or nil. If the server supports UID EXPUNGE, both t and nil are
+equivalent to @code{immediately}. If the server does not support UID
+EXPUNGE nil is equivalent to @code{never}, while t will immediately
+expunge ALL articles that are currently flagged as deleted
+(i.e., potentially not only the article that was just deleted).
 
 @item nnimap-streaming
 Virtually all @acronym{IMAP} server support fast streaming of data.
diff --git a/etc/NEWS b/etc/NEWS
index ca66d..e5ee4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -81,6 +81,16 @@ for '--daemon'.
 * Changes in Emacs 26.1
 
 +++
+** The Gnus user variable 'nnimap-expunge' supports three new values:
+'never' for never expunging messages, 'immediately' for immediately
+expunging deleted messages, and 'on-exit' to expunge deleted articles
+when exiting the group's summary buffer. Setting 'nnimap-expunge' to
+'nil' or 't' is still supported but not recommended, since it may
+result in Gnus expunging all messages that have been flagged as
+deleted by any IMAP client (rather than just those that have been
+deleted by Gnus).
+
++++
 ** The new function 'mapbacktrace' applies a function to all frames of
 the current stack trace.
 
diff --git a/lisp/gnus/nnimap.el b/lisp/gnus/nnimap.el
index 700e86..e7e64 100644
--- a/lisp/gnus/nnimap.el
+++ b/lisp/gnus/nnimap.el
@@ -92,10 +92,21 @@ nnimap-authenticator
 Possible choices are nil (use default methods), `anonymous',
 `login', `plain' and `cram-md5'.")
 
-(defvoo nnimap-expunge t
-  "If non-nil, expunge articles after deleting them.
-This is always done if the server supports UID EXPUNGE, but it's
-not done by default on servers that doesn't support that command.")
+(defvoo nnimap-expunge 'on-exit
+  "When to expunge deleted messages.
+If 'never, deleted articles are marked with the IMAP \\Delete
+flag but not automatically expunged. If 'immediately, deleted
+articles are immediately expunged (this requires the server to
+support the UID EXPUNGE command). If 'on-exit, deleted articles
+are flagged, and all flagged articles are expunged when the
+group is closed.
+
+For backwards compatibility, this variable may also be set to t
+or nil. If the server supports UID EXPUNGE, both t and nil are
+equivalent to 'immediately. If the server does not support UID
+EXPUNGE nil is equivalent to 'never, while t will immediately
+expunge ALL articles that are currently flagged as deleted
+(i.e., potentially not only the article that was just deleted).")
 
 (defvoo nnimap-streaming t
   "If non-nil, try to use streaming commands with IMAP servers.
@@ -911,7 +922,11 @@ nnimap-get-flags
     (nreverse articles)))
 
 (deffoo nnimap-close-group (_group &optional _server)
-  t)
+  (when (eq nnimap-expunge 'on-exit)
+    (nnoo-change-server 'nnimap server nil)
+    (with-current-buffer (nnimap-buffer)
+      (nnimap-wait-for-response
+       (nnimap-send-command "EXPUNGE")))))
 
 (deffoo nnimap-request-move-article (article group server accept-form
                                             &optional _last
@@ -960,8 +975,7 @@ nnimap-request-expire-articles
     articles)
    ((and force
         (eq nnmail-expiry-target 'delete))
-    (unless (nnimap-delete-article (gnus-compress-sequence articles))
-      (nnheader-message 7 "Article marked for deletion, but not expunged."))
+    (nnimap-delete-article (gnus-compress-sequence articles))
     nil)
    (t
     (let ((deletable-articles
@@ -1084,21 +1098,38 @@ nnimap-find-article-by-message-id
             (when (and limit number-of-article)
               (nnimap-find-article-by-message-id group server 
message-id))))))))
 
-(defun nnimap-delete-article (articles)
+(defun nnimap-delete-article (articles &optional sync)
+  "Delete ARTICLES.
+
+If sync is non-nil, wait for server response."
   (with-current-buffer (nnimap-buffer)
     (nnimap-command "UID STORE %s +FLAGS.SILENT (\\Deleted)"
                    (nnimap-article-ranges articles))
+    (let ((sequence
     (cond
-     ((nnimap-capability "UIDPLUS")
+            ((eq nnimap-expunge 'immediately)
+             (if (nnimap-capability "UIDPLUS")
       (nnimap-command "UID EXPUNGE %s"
                      (nnimap-article-ranges articles))
-      t)
+      (nnheader-message
+       3 (concat "nnimap-expunge set to 'immediately, but "
+                 "server doesn't support UIDPLUS"))))
+
+            ((memq nnimap-expunge '(on-exit never)) nil)
+
+            ((nnimap-capability "UIDPLUS")
+             (nnimap-command "UID EXPUNGE %s"
+                             (nnimap-article-ranges articles)))
+
      (nnimap-expunge
-      (nnimap-command "EXPUNGE")
-      t)
-     (t (gnus-message 7 (concat "nnimap: nnimap-expunge is not set and the "
-                                "server doesn't support UIDPLUS, so we won't "
-                                "delete this article now"))))))
+             (nnimap-command "EXPUNGE"))
+
+            (t
+             (nnheader-message
+              7 "Article marked for deletion, but not expunged.")))))
+
+      (if (and sync sequence)
+          (nnimap-wait-for-response sequence)))))
 
 (deffoo nnimap-request-scan (&optional group server)
   (when group
@@ -2118,27 +2149,9 @@ nnimap-split-incoming-mail
              (nnimap-wait-for-response (caar sequences))
              ;; And then mark the successful copy actions as deleted,
              ;; and possibly expunge them.
-             (nnimap-mark-and-expunge-incoming
-              (nnimap-parse-copied-articles sequences)))
-            (nnimap-mark-and-expunge-incoming junk-articles)))))))
-
-(defun nnimap-mark-and-expunge-incoming (range)
-  (when range
-    (setq range (nnimap-article-ranges range))
-    (erase-buffer)
-    (let ((sequence
-          (nnimap-send-command
-           "UID STORE %s +FLAGS.SILENT (\\Deleted)" range)))
-      (cond
-       ;; If the server supports it, we now delete the message we have
-       ;; just copied over.
-       ((nnimap-capability "UIDPLUS")
-       (setq sequence (nnimap-send-command "UID EXPUNGE %s" range)))
-       ;; If it doesn't support UID EXPUNGE, then we only expunge if the
-       ;; user has configured it.
-       (nnimap-expunge
-       (setq sequence (nnimap-send-command "EXPUNGE"))))
-      (nnimap-wait-for-response sequence))))
+              (nnimap-delete-article
+               (nnimap-parse-copied-articles sequences) t))
+            (nnimap-delete-article junk-articles t)))))))
 
 (defun nnimap-parse-copied-articles (sequences)
   (let (sequence copied range)
-- 
2.1.4