bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#18351: [Alan Schmitt] Small patch for gnus nnimap.el


From: Stefan Monnier
Subject: bug#18351: [Alan Schmitt] Small patch for gnus nnimap.el
Date: Thu, 28 Aug 2014 21:19:13 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.4.50 (gnu/linux)

Package:emacs,gnus

Thanks Alan for your detailed bug-report with patch.
Turning this to the bug-tracker, so we have a tracking number for it.


        Stefan


--- Begin Message --- Subject: Small patch for gnus nnimap.el Date: Wed, 27 Aug 2014 08:45:15 +0200 User-agent: Gnus/5.130012 (Ma Gnus v0.12) Emacs/24.3 (darwin)
Hello,

I posted this on gnus.general a couple weeks ago, but nobody
replied. I'm thus wondering if emacs.devel might be a better place to
send gnus patches. If not, could you please direct me to the correct
place?


As I'm trying to make the registry working with expiration, I think
I found a bug in nnimap.el. Here is the code in question.

#+begin_src emacs-lisp
(defun nnimap-process-expiry-targets (articles group server)
  (let ((deleted-articles nil))
    (cond
     ;; shortcut further processing if we're going to delete the articles
     ((eq nnmail-expiry-target 'delete)
      (setq deleted-articles articles)
      t)
     ;; or just move them to another folder on the same IMAP server
     ((and (not (functionp nnmail-expiry-target))
           (gnus-server-equal (gnus-group-method nnmail-expiry-target)
                              (gnus-server-to-method
                               (format "nnimap:%s" server))))
      (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)))
      t)
     (t
      (dolist (article articles)
        (let ((target nnmail-expiry-target))
          (with-temp-buffer
            (mm-disable-multibyte)
            (when (nnimap-request-article article group server (current-buffer))
              (when (functionp target)
                (setq target (funcall target group)))
              (if (and target
                       (not (eq target 'delete)))
                  (if (or (gnus-request-group target t)
                          (gnus-request-create-group target))
                      (progn
                        (nnmail-expiry-target-group target group)
                        (nnheader-message 7 "Expiring article %s:%d to %s"
                                          group article target))
                    (setq target nil))
                (nnheader-message 7 "Expiring article %s:%d" group article))
              (when target
                (push article deleted-articles))))))))
    ;; Change back to the current group again.
    (nnimap-change-group group server)
    (setq deleted-articles (nreverse deleted-articles))
    (nnimap-delete-article (gnus-compress-sequence deleted-articles))
    deleted-articles))
#+end_src

The result of this function should be a list or messages that have been
deleted, ordered by '<' (since 'gnus-sorted-complement' is then called
on that list). If articles are deleted or sent to a static group on the
same server, then 'deleted-articles' is just set to the list of all
articles to expire. Otherwise each article is considered in turn, and
articles that were successfully moved are put in the deleted-articles
list.

The problem with this function is that the list of deleted articles is
reversed in every case, but it should only be reversed in the last case
(when it was created considering each article in turn and pushing it
onto the list, thus creating it in reverse). Attached is a patch fixing
this.

As an example why the current behavior is wrong, here is part of
a debugging session:

> Result: (4402 4406 4409 4414 4415)
> 
> Result: "work"
> 
> Expiring articles from work: (4402 4406 4415)
> Result: (4402 4406 4409 4414 4406 4402)

The first list is the list of all articles to consider. Then the
articles "(4402 4406 4415)" are expired, and the result should be the
remaining articles. Here the list of expired articles was reversed, and
when taking the complement only 4415 was removed (since it's assumed
it's ordered by '<') and the other remaining articles were put at the
end.

If you agree with this analysis, could you please apply this patch?

I have a question about the code above. Why do the first two cases of
the cond return a 't' at the end?

Thanks,

Alan

From 073f9742158e42460fa24c414d6589e552873af5 Mon Sep 17 00:00:00 2001
From: Alan Schmitt <alan.schmitt@polytechnique.org>
Date: Mon, 11 Aug 2014 16:32:08 +0200
Subject: [PATCH] Only reverse list of expired messages when in wrong order

* lisp/nnimap.el (nnimap-process-expiry-targets): the list of expired
  messages should only be reversed when it was built in reverse order.
---
 lisp/nnimap.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/nnimap.el b/lisp/nnimap.el
index 1730bd4..ad48d47 100644
--- a/lisp/nnimap.el
+++ b/lisp/nnimap.el
@@ -986,10 +986,10 @@ textual parts.")
                    (setq target nil))
                (nnheader-message 7 "Expiring article %s:%d" group article))
              (when target
-               (push article deleted-articles))))))))
+               (push article deleted-articles))))))
+      (setq deleted-articles (nreverse deleted-articles))))
     ;; Change back to the current group again.
     (nnimap-change-group group server)
-    (setq deleted-articles (nreverse deleted-articles))
     (nnimap-delete-article (gnus-compress-sequence deleted-articles))
     deleted-articles))
 
-- 
2.0.3


-- 
OpenPGP Key ID : 040D0A3B4ED2E5C7

Attachment: signature.asc
Description: PGP signature


--- End Message ---

reply via email to

[Prev in Thread] Current Thread [Next in Thread]