Gnus nnir patch for Emacs 26?

Gnus nnir patch for Emacs 26?

[Eric Abrahamsen]

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

Someone recently reported a bug in Gnus for searching IMAP groups when
the group has a non-ascii name. The bug has been there a while, at least
as far back as Emacs 26. I've attached the diff, which looks large only
because of indentation changes, it boils down to adding the line:

(setq group (nnimap-decode-gnus-group group))

Is it too late/too intrusive for Emacs 26? This is the first time
anyone's reported the bug, so it's obviously not super pressing. Another
odd thing is that this fix will be unnecessary once the
scratch/gnus-decoded branch lands, so this change shouldn't go into
master, but I gather that's possible with a dontmerge cookie or
something.

Anyway, just asking!

Eric


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: nnir-imap-fix.diff --]
[-- Type: text/x-patch, Size: 2964 bytes --]

diff --git a/lisp/gnus/nnir.el b/lisp/gnus/nnir.el
index 9d59a4db0d..73d851c395 100644
--- a/lisp/gnus/nnir.el
+++ b/lisp/gnus/nnir.el
@@ -988,34 +988,36 @@ nnir-run-imap
        'vconcat
        (catch 'found
          (mapcar
-          #'(lambda (group)
+          (lambda (group)
             (let (artlist)
-              (condition-case ()
-                  (when (nnimap-change-group
-                         (gnus-group-short-name group) server)
-                    (with-current-buffer (nnimap-buffer)
-                      (message "Searching %s..." group)
-                      (let ((arts 0)
-                            (result (nnimap-command "UID SEARCH %s"
-                                                    (if (string= criteria "")
-                                                        qstring
-                                                      (nnir-imap-make-query
-                                                       criteria qstring)))))
-                        (mapc
-                         (lambda (artnum)
-                           (let ((artn (string-to-number artnum)))
-                             (when (> artn 0)
-                               (push (vector group artn 100)
-                                     artlist)
-                               (when (assq 'shortcut query)
-                                 (throw 'found (list artlist)))
-                               (setq arts (1+ arts)))))
-                         (and (car result)
-			      (cdr (assoc "SEARCH" (cdr result)))))
-                        (message "Searching %s... %d matches" group arts)))
-                    (message "Searching %s...done" group))
+	      (condition-case ()
+		  (progn
+		    (setq group (nnimap-decode-gnus-group group))
+                    (when (nnimap-change-group
+                           (gnus-group-short-name group) server)
+		      (with-current-buffer (nnimap-buffer)
+			(message "Searching %s..." group)
+			(let ((arts 0)
+			      (result (nnimap-command "UID SEARCH %s"
+						      (if (string= criteria "")
+                                                          qstring
+							(nnir-imap-make-query
+							 criteria qstring)))))
+                          (mapc
+                           (lambda (artnum)
+                             (let ((artn (string-to-number artnum)))
+			       (when (> artn 0)
+				 (push (vector group artn 100)
+				       artlist)
+				 (when (assq 'shortcut query)
+                                   (throw 'found (list artlist)))
+				 (setq arts (1+ arts)))))
+                           (and (car result)
+				(cdr (assoc "SEARCH" (cdr result)))))
+                          (message "Searching %s... %d matches" group arts)))
+		      (message "Searching %s...done" group)))
                 (quit nil))
-              (nreverse artlist)))
+	      (nreverse artlist)))
           groups))))))
 
 (defun nnir-imap-make-query (criteria qstring)

Re: Gnus nnir patch for Emacs 26?

[Robert Pluim]

>>>>> On Wed, 12 Jun 2019 14:46:14 -0700, Eric Abrahamsen <eric@ericabrahamsen.net> said:

    Eric> Someone recently reported a bug in Gnus for searching IMAP groups when
    Eric> the group has a non-ascii name. The bug has been there a while, at least
    Eric> as far back as Emacs 26. I've attached the diff, which looks large only
    Eric> because of indentation changes, it boils down to adding the line:

    Eric> (setq group (nnimap-decode-gnus-group group))

    Eric> Is it too late/too intrusive for Emacs 26? This is the first
    Eric> time

I think itʼs fine, but Iʼm not Eli :-)

    Eric> anyone's reported the bug, so it's obviously not super pressing. Another
    Eric> odd thing is that this fix will be unnecessary once the
    Eric> scratch/gnus-decoded branch lands, so this change shouldn't go into
    Eric> master, but I gather that's possible with a dontmerge cookie or
    Eric> something.

From CONTRIBUTE:

    Some changes should not be merged to master at all, for whatever
    reasons.  These should be marked by including something like "Do not
    merge to master" or anything that matches gitmerge-skip-regexp (see
    admin/gitmerge.el) in the commit message.

Robert

Re: Gnus nnir patch for Emacs 26?

[Eli Zaretskii]

> From: Eric Abrahamsen <eric@ericabrahamsen.net>
> Date: Wed, 12 Jun 2019 14:46:14 -0700
> 
> Someone recently reported a bug in Gnus for searching IMAP groups when
> the group has a non-ascii name. The bug has been there a while, at least
> as far back as Emacs 26. I've attached the diff, which looks large only
> because of indentation changes, it boils down to adding the line:
> 
> (setq group (nnimap-decode-gnus-group group))

I didn't understand that this change fixes the problem, the last
message still indicated the patch didn't resolve it.  Did you continue
discussing off-list or something?  If not, how do you know the fix is
complete?

> Is it too late/too intrusive for Emacs 26? This is the first time
> anyone's reported the bug, so it's obviously not super pressing. Another
> odd thing is that this fix will be unnecessary once the
> scratch/gnus-decoded branch lands, so this change shouldn't go into
> master, but I gather that's possible with a dontmerge cookie or
> something.

If this patch indeed fixes the problem, it's okay for emacs-26, but
please only add that one line, don't reindent anything and don't
replace lambda with its fancier equivalent.  Just the minimal required
change.

Thanks.

Re: Gnus nnir patch for Emacs 26?

[Eric Abrahamsen]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eric Abrahamsen <eric@ericabrahamsen.net>
>> Date: Wed, 12 Jun 2019 14:46:14 -0700
>> 
>> Someone recently reported a bug in Gnus for searching IMAP groups when
>> the group has a non-ascii name. The bug has been there a while, at least
>> as far back as Emacs 26. I've attached the diff, which looks large only
>> because of indentation changes, it boils down to adding the line:
>> 
>> (setq group (nnimap-decode-gnus-group group))
>
> I didn't understand that this change fixes the problem, the last
> message still indicated the patch didn't resolve it.  Did you continue
> discussing off-list or something?  If not, how do you know the fix is
> complete?

Yes, Dieter wrote off-list (I think by accident) right afterwards to say
he'd been mistaken and the fix worked. I was also able to reproduce the
bug and confirm the fix.

>> Is it too late/too intrusive for Emacs 26? This is the first time
>> anyone's reported the bug, so it's obviously not super pressing. Another
>> odd thing is that this fix will be unnecessary once the
>> scratch/gnus-decoded branch lands, so this change shouldn't go into
>> master, but I gather that's possible with a dontmerge cookie or
>> something.
>
> If this patch indeed fixes the problem, it's okay for emacs-26, but
> please only add that one line, don't reindent anything and don't
> replace lambda with its fancier equivalent.  Just the minimal required
> change.

Here's a proper commit with a simpler-looking version that doesn't
re-indent everything. I've done before-and-after tests and this works
just the same. I've also added a do not merge thing following Robert's
pointer.

How's it look?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0007-Make-sure-Gnus-imap-group-names-are-decoded-before-s.patch --]
[-- Type: text/x-patch, Size: 934 bytes --]

From f999e6e090f300769fe6087290459701dc929acc Mon Sep 17 00:00:00 2001
From: Eric Abrahamsen <eric@ericabrahamsen.net>
Date: Thu, 13 Jun 2019 15:09:24 -0700
Subject: [PATCH 7/7] Make sure Gnus imap group names are decoded before
 searching

do not merge (fix unnecessary in Emacs 27)

* lisp/gnus/nnir.el (nnir-run-imap): Ensure that non-ascii group names
  have been fully decoded before passing them to imap search.
---
 lisp/gnus/nnir.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/gnus/nnir.el b/lisp/gnus/nnir.el
index 05b2f0aa8a..0062cd8589 100644
--- a/lisp/gnus/nnir.el
+++ b/lisp/gnus/nnir.el
@@ -969,6 +969,7 @@ nnir-run-imap
          (mapcar
           #'(lambda (group)
             (let (artlist)
+	      (setq group (nnimap-decode-gnus-group group))
               (condition-case ()
                   (when (nnimap-change-group
                          (gnus-group-short-name group) server)
-- 
2.22.0

Re: Gnus nnir patch for Emacs 26?

[Eli Zaretskii]

> From: Eric Abrahamsen <eric@ericabrahamsen.net>
> Date: Thu, 13 Jun 2019 15:20:52 -0700
> 
> > If this patch indeed fixes the problem, it's okay for emacs-26, but
> > please only add that one line, don't reindent anything and don't
> > replace lambda with its fancier equivalent.  Just the minimal required
> > change.
> 
> Here's a proper commit with a simpler-looking version that doesn't
> re-indent everything. I've done before-and-after tests and this works
> just the same. I've also added a do not merge thing following Robert's
> pointer.
> 
> How's it look?

Fine with me, thanks.

P.S. I was wondering why we use all those wrappers around
decode-coding-string instead of calling the latter directly, but since
you are saying this code will go away in Emacs 27, this is a moot
point.  I do suggest to look for such trivial wrappers on master and
eliminate them if any are still there, as a cleanup.

Re: Gnus nnir patch for Emacs 26?

[Eric Abrahamsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eric Abrahamsen <eric@ericabrahamsen.net>
>> Date: Thu, 13 Jun 2019 15:20:52 -0700
>> 
>> > If this patch indeed fixes the problem, it's okay for emacs-26, but
>> > please only add that one line, don't reindent anything and don't
>> > replace lambda with its fancier equivalent.  Just the minimal required
>> > change.
>> 
>> Here's a proper commit with a simpler-looking version that doesn't
>> re-indent everything. I've done before-and-after tests and this works
>> just the same. I've also added a do not merge thing following Robert's
>> pointer.
>> 
>> How's it look?
>
> Fine with me, thanks.

Cool, will push tomorrow.

> P.S. I was wondering why we use all those wrappers around
> decode-coding-string instead of calling the latter directly, but since
> you are saying this code will go away in Emacs 27, this is a moot
> point.  I do suggest to look for such trivial wrappers on master and
> eliminate them if any are still there, as a cleanup.

The code in scratch/gnus-decoded will do some of that, since the point
of that branch is to make sure group names are decoded strings as they
enter the system, rather than decoding them as they come out of the
system (ie, as they are presented to the user).