bug#19950: 25.0.50; Gnus nnimap -- don't split messages back into original group

bug#19950: 25.0.50; Gnus nnimap -- don't split messages back into original group

[Eric Abrahamsen]

[-- Attachment #1: Type: text/plain, Size: 831 bytes --]


Presently, `nnimap-split-incoming-mail' will split a message back into
the group it was already in, if that's how the split rules come out.
That results in a useless COPY/EXPUNGE, effectively incrementing the
message's UID to no purpose.

It's a small inefficiency, and doesn't happen that often, but if a user
has otherwise misconfigured bits of his/her imap setup (ahem), it can
lead to some strange behavior. It shouldn't have any real downside,
either.

I've attached a patch that does this, and can push it if it's okay. The
only potential problem I can imagine would be if an utf7-encoded group
name were somehow compared against a non-encoded group name, but I don't
see how that could happen.

If this is acceptable, does it need a CHANGELOG mention? I've gotten
pretty confused about what's LOG-worthy, and what isn't.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Don-t-split-imap-messages-back-into-original-group.patch --]
[-- Type: text/x-diff, Size: 1338 bytes --]

From d45e03d434942a883ee67ee5a120cdd04c62c59a Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Wed, 18 Feb 2015 17:51:37 +0800
Subject: [PATCH] Don't split imap messages back into original group

* lisp/nnimap.el (nnimap-split-incoming-mail): If a message is already
  in the group it should be split to, don't re-copy it into the group.
---
 lisp/nnimap.el | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/lisp/nnimap.el b/lisp/nnimap.el
index 6904e0a..9f89f39 100644
--- a/lisp/nnimap.el
+++ b/lisp/nnimap.el
@@ -2086,12 +2086,15 @@ Return the server's response to the SELECT or EXAMINE command."
 		    (ranges (cdr spec)))
 		(if (eq group 'junk)
 		    (setq junk-articles ranges)
-		  (push (list (nnimap-send-command
-			       "UID COPY %s %S"
-			       (nnimap-article-ranges ranges)
-			       (utf7-encode group t))
-			      ranges)
-			sequences))))
+		  ;; Don't copy if the message is already in its
+		  ;; target group.
+		  (unless (string= group nnimap-inbox)
+		   (push (list (nnimap-send-command
+				"UID COPY %s %S"
+				(nnimap-article-ranges ranges)
+				(utf7-encode group t))
+			       ranges)
+			 sequences)))))
 	    ;; Wait for the last COPY response...
 	    (when sequences
 	      (nnimap-wait-for-response (caar sequences))
-- 
2.3.0

bug#19950: 25.0.50; nnimap: don't split messages back into original group

[Ivan Shmakov]

>>>>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:

[…]

 > If this is acceptable, does it need a CHANGELOG mention?  I've gotten
 > pretty confused about what's LOG-worthy, and what isn't.

	Apart of fixes applied to ChangeLog itself, pretty much every
	change has to be listed there.  (This is contrary to, say, NEWS,
	which only documents “user-visible” changes.)

 > * lisp/nnimap.el (nnimap-split-incoming-mail): If a message is
 > already in the group it should be split to, don't re-copy it into the
 > group.

[…]

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

Re: bug#19950: 25.0.50; nnimap: don't split messages back into original group

[Ivan Shmakov]

>>>>> Eric Abrahamsen <eric@ericabrahamsen.net> writes:

[…]

 > If this is acceptable, does it need a CHANGELOG mention?  I've gotten
 > pretty confused about what's LOG-worthy, and what isn't.

	Apart of fixes applied to ChangeLog itself, pretty much every
	change has to be listed there.  (This is contrary to, say, NEWS,
	which only documents “user-visible” changes.)

 > * lisp/nnimap.el (nnimap-split-incoming-mail): If a message is
 > already in the group it should be split to, don't re-copy it into the
 > group.

[…]

-- 
FSF associate member #7257  http://boycottsystemd.org/  … 3013 B6A0 230E 334A

bug#19950: 25.0.50; Gnus nnimap -- don't split messages back into original group

[Stefan Monnier]

> If this is acceptable, does it need a CHANGELOG mention? I've gotten
> pretty confused about what's LOG-worthy, and what isn't.

The ChangeLog duplicates the commit messages, and since every commit should
have a commit message, then every commit should add that same message
to the relevant ChangeLog.

At least for now, until we finally change our procedure to auto-generate
the ChangeLog files (should be "real soon now", tho there hasn't been
much activity in this area of late).


        Stefan

bug#19950: 25.0.50; Gnus nnimap -- don't split messages back into original group

[Eric Abrahamsen]

[-- Attachment #1: Type: text/plain, Size: 663 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> If this is acceptable, does it need a CHANGELOG mention? I've gotten
>> pretty confused about what's LOG-worthy, and what isn't.
>
> The ChangeLog duplicates the commit messages, and since every commit should
> have a commit message, then every commit should add that same message
> to the relevant ChangeLog.
>
> At least for now, until we finally change our procedure to auto-generate
> the ChangeLog files (should be "real soon now", tho there hasn't been
> much activity in this area of late).

Thanks to you both! Here's a new patch with ChangeLog added. I'll just
leave this here until Lars sees it...


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Don-t-split-imap-messages-back-into-original-group.patch --]
[-- Type: text/x-diff, Size: 1814 bytes --]

From ffbbdec9909bc08d853d239b798bd33726e6fa82 Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Wed, 18 Feb 2015 17:51:37 +0800
Subject: [PATCH] Don't split imap messages back into original group

* lisp/nnimap.el (nnimap-split-incoming-mail): If a message is already
  in the group it should be split to, don't re-copy it into the group.
---
 lisp/ChangeLog |  4 ++++
 lisp/nnimap.el | 15 +++++++++------
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/lisp/ChangeLog b/lisp/ChangeLog
index 2047481..011e3cd 100644
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,7 @@
+2015-02-27  Eric Abrahamsen  <eric@ericabrahamsen.net>
+	* lisp/nnimap.el (nnimap-split-incoming-mail): If a message is already
+	in the group it should be split to, don't re-copy it into the group.
+
 2015-02-26  Katsumi Yamaoka  <yamaoka@jpl.org>
 
 	* gnus-art.el (gnus-mime-inline-part, gnus-mm-display-part):
diff --git a/lisp/nnimap.el b/lisp/nnimap.el
index 369d9d3..c476be6 100644
--- a/lisp/nnimap.el
+++ b/lisp/nnimap.el
@@ -2086,12 +2086,15 @@ Return the server's response to the SELECT or EXAMINE command."
 		    (ranges (cdr spec)))
 		(if (eq group 'junk)
 		    (setq junk-articles ranges)
-		  (push (list (nnimap-send-command
-			       "UID COPY %s %S"
-			       (nnimap-article-ranges ranges)
-			       (utf7-encode group t))
-			      ranges)
-			sequences))))
+		  ;; Don't copy if the message is already in its
+		  ;; target group.
+		  (unless (string= group nnimap-inbox)
+		   (push (list (nnimap-send-command
+				"UID COPY %s %S"
+				(nnimap-article-ranges ranges)
+				(utf7-encode group t))
+			       ranges)
+			 sequences)))))
 	    ;; Wait for the last COPY response...
 	    (when sequences
 	      (nnimap-wait-for-response (caar sequences))
-- 
2.3.1

bug#19950: 25.0.50; Gnus nnimap -- don't split messages back into original group

[Eric Abrahamsen]

Eric Abrahamsen <eric@ericabrahamsen.net> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> If this is acceptable, does it need a CHANGELOG mention? I've gotten
>>> pretty confused about what's LOG-worthy, and what isn't.
>>
>> The ChangeLog duplicates the commit messages, and since every commit should
>> have a commit message, then every commit should add that same message
>> to the relevant ChangeLog.
>>
>> At least for now, until we finally change our procedure to auto-generate
>> the ChangeLog files (should be "real soon now", tho there hasn't been
>> much activity in this area of late).
>
> Thanks to you both! Here's a new patch with ChangeLog added. I'll just
> leave this here until Lars sees it...

Bah, I've applied this anyway. It might not be the complete solution,
but it's desirable in and of itself. 

bug#19950: closing

[Eric Abrahamsen]

I think this is how you do it...