* bug#18351: [Alan Schmitt] Small patch for gnus nnimap.el
@ 2014-08-29 1:19 Stefan Monnier
2014-08-29 1:59 ` Stefan Monnier
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2014-08-29 1:19 UTC (permalink / raw)
To: 18351; +Cc: Alan Schmitt
[-- Attachment #1: Type: text/plain, Size: 162 bytes --]
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
[-- Attachment #2: Type: message/rfc822, Size: 9646 bytes --]
[-- Attachment #2.1.1.1: Type: text/plain, Size: 3832 bytes --]
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
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2.1.1.2: 0001-Only-reverse-list-of-expired-messages-when-in-wrong-.patch --]
[-- Type: text/x-patch, Size: 1151 bytes --]
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
[-- Attachment #2.1.1.3: Type: text/plain, Size: 44 bytes --]
--
OpenPGP Key ID : 040D0A3B4ED2E5C7
[-- Attachment #2.1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 494 bytes --]
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#18351: [Alan Schmitt] Small patch for gnus nnimap.el
2014-08-29 1:19 bug#18351: [Alan Schmitt] Small patch for gnus nnimap.el Stefan Monnier
@ 2014-08-29 1:59 ` Stefan Monnier
2014-09-22 12:29 ` Alan Schmitt
0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2014-08-29 1:59 UTC (permalink / raw)
To: 18351; +Cc: Alan Schmitt
> 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?
Both places are good choices.
> 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.
The analysis looks good, and so does the patch.
> If you agree with this analysis, could you please apply this patch?
I can apply the patch, but since I'm not familiar with this code,
I'd like to hear some comment from one of the Gnus guys.
> I have a question about the code above. Why do the first two cases of
> the cond return a 't' at the end?
Very good question. Probably hysterical raisins?
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#18351: [Alan Schmitt] Small patch for gnus nnimap.el
2014-08-29 1:59 ` Stefan Monnier
@ 2014-09-22 12:29 ` Alan Schmitt
2014-10-04 22:10 ` Ted Zlatanov
0 siblings, 1 reply; 6+ messages in thread
From: Alan Schmitt @ 2014-09-22 12:29 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 18351
[-- Attachment #1: Type: text/plain, Size: 862 bytes --]
Hello,
On 2014-08-28 21:59, Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>> 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.
>
> The analysis looks good, and so does the patch.
>
>> If you agree with this analysis, could you please apply this patch?
>
> I can apply the patch, but since I'm not familiar with this code,
> I'd like to hear some comment from one of the Gnus guys.
I've been using this patch for about a month now and I have not
encountered any problem. Is there anything I can do to help its
application?
Thanks,
Alan
--
OpenPGP Key ID : 040D0A3B4ED2E5C7
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 494 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#18351: [Alan Schmitt] Small patch for gnus nnimap.el
2014-09-22 12:29 ` Alan Schmitt
@ 2014-10-04 22:10 ` Ted Zlatanov
2014-10-05 1:55 ` Stefan Monnier
2014-10-05 10:21 ` Alan Schmitt
0 siblings, 2 replies; 6+ messages in thread
From: Ted Zlatanov @ 2014-10-04 22:10 UTC (permalink / raw)
To: Alan Schmitt; +Cc: 18351-done
On Mon, 22 Sep 2014 14:29:59 +0200 Alan Schmitt <alan.schmitt@polytechnique.org> wrote:
AS> Hello,
AS> On 2014-08-28 21:59, Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>>> 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.
>>
>> The analysis looks good, and so does the patch.
>>
>>> If you agree with this analysis, could you please apply this patch?
>>
>> I can apply the patch, but since I'm not familiar with this code,
>> I'd like to hear some comment from one of the Gnus guys.
AS> I've been using this patch for about a month now and I have not
AS> encountered any problem. Is there anything I can do to help its
AS> application?
I've applied it in Gnus and it should get merged with Emacs trunk
soonish. It is good as far as I can tell.
Thanks
Ted
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#18351: [Alan Schmitt] Small patch for gnus nnimap.el
2014-10-04 22:10 ` Ted Zlatanov
@ 2014-10-05 1:55 ` Stefan Monnier
2014-10-05 10:21 ` Alan Schmitt
1 sibling, 0 replies; 6+ messages in thread
From: Stefan Monnier @ 2014-10-05 1:55 UTC (permalink / raw)
To: Alan Schmitt; +Cc: 18351-done
> I've applied it in Gnus and it should get merged with Emacs trunk
> soonish.
Would it be useful/needed in emacs-24?
Stefan
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#18351: [Alan Schmitt] Small patch for gnus nnimap.el
2014-10-04 22:10 ` Ted Zlatanov
2014-10-05 1:55 ` Stefan Monnier
@ 2014-10-05 10:21 ` Alan Schmitt
1 sibling, 0 replies; 6+ messages in thread
From: Alan Schmitt @ 2014-10-05 10:21 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 18351-done
[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]
On 2014-10-04 18:10, Ted Zlatanov <tzz@lifelogs.com> writes:
> On Mon, 22 Sep 2014 14:29:59 +0200 Alan Schmitt <alan.schmitt@polytechnique.org> wrote:
>
> AS> Hello,
> AS> On 2014-08-28 21:59, Stefan Monnier <monnier@IRO.UMontreal.CA> writes:
>
>>>> 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.
>>>
>>> The analysis looks good, and so does the patch.
>>>
>>>> If you agree with this analysis, could you please apply this patch?
>>>
>>> I can apply the patch, but since I'm not familiar with this code,
>>> I'd like to hear some comment from one of the Gnus guys.
>
> AS> I've been using this patch for about a month now and I have not
> AS> encountered any problem. Is there anything I can do to help its
> AS> application?
>
> I've applied it in Gnus and it should get merged with Emacs trunk
> soonish. It is good as far as I can tell.
Thank you.
Alan
--
OpenPGP Key ID : 040D0A3B4ED2E5C7
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 494 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-05 10:21 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-08-29 1:19 bug#18351: [Alan Schmitt] Small patch for gnus nnimap.el Stefan Monnier
2014-08-29 1:59 ` Stefan Monnier
2014-09-22 12:29 ` Alan Schmitt
2014-10-04 22:10 ` Ted Zlatanov
2014-10-05 1:55 ` Stefan Monnier
2014-10-05 10:21 ` Alan Schmitt
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).