[PATCH] bug in 'nnimap-process-expiry-targets' (nnimap.el)?

[Alan Schmitt]

Hello,

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

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

[Alan Schmitt]

Hello,

Have you had the chance to look at this patch?

Thanks,

Alan