bug#35219: 27.0.50; Problems with nnimap groups with non-ASCII characters

[Eric Abrahamsen <eric@ericabrahamsen.net>]

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

Andy Moreton <andrewjmoreton@gmail.com> writes:

> On Thu 18 Apr 2019, Eric Abrahamsen wrote:
>
>> Andy Moreton <andrewjmoreton@gmail.com> writes:
>>> I see a similar symptom, but with a different recipe:
>>>  - start Gnus
>>>  - open the server buffer, select a server, and subscribe to a new group
>>>  - quit the server buffer
>>>  - in the group buffer, kill the group line for the new group
>>> At this point, emacs is busy but unresponsive. Breaking in with ^G
>>> results in emacs becoming responsive agin, but all of the group lines
>>> disappear from the group buffer.
>>>
>>> Something is still not quite right.
>>
>> I'm not able to reproduce that, using a nntp group from gmane -- can you
>> give me more detail about how you trigger it?
>
> I used an nntp group from gmane to check this. After hitting ^g in the
> recipe above, emacs shows the last 3 lines of the original group buffer,
> and typing 'g' toudpate the group buffer restores the display to all of
> the missing groups.
>
> After more testing, it seems that this wrong display depends on using
> topics in the group buffer. If I toggle topics off ('t' in the group
> buffer) then killing the newly added group appears to work normally.
>
> With topics enabled, adding a new group and then saving .newsrc.eld ('s'
> in the group buffer) still results in the bad behaviour when killing the
> new group.

Okay, this was a fairly intense deep dive! Thanks for the report. It
looks like the flurry of fixes over the past week or so also introduced
some new bugs.

The infloop comes from `gnus-group-goto-group' returning 1 instead of
nil when it can't find a group. So `while' loops in gnus-topic.el never
exit. The attached patch should fix it.

Yamaoka-san, this would revert some of your changes. I think this is
correct because the original `gnus-group-goto-group' function did not
actually check that a group exists in the newsrc or active hashtables:
it used `gnus-intern-safe' which _always returned the group name_, no
matter what. In fact, I don't know why the original function even
bothered checking that, but the point is that the goto function would
always have a group name to work with.

With the attached change, Gnus' original behavior is restored for me,
including the fact that `gnus-group-jump-to-group' will jump to
non-existent groups (creating them in the process).

> Killing/yanking a group that was already in.newsrc.eld before gnus was
> started works normally.
>
> I've only checked this with my usual config though, rather than
> something stripped down to produce a minimal test case.

FWIW, I have a package in ELPA called gnus-mock that I'm using for
development: it's a trashable-and-restorable Gnus installation with
dummy data that you can use to play with Gnus changes. I'm not expecting
anyone else to go to the trouble of using this to report bugs, but it is
there. I'm working on a semi-interactive set of ERT tests that are meant
to run while Gnus is open, and ensure that point ends up in the right
place after various commands, etc.

Eric

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

diff --git a/lisp/gnus/gnus-group.el b/lisp/gnus/gnus-group.el
index b1e4091c97..c757c82fbc 100644
--- a/lisp/gnus/gnus-group.el
+++ b/lisp/gnus/gnus-group.el
@@ -2549,37 +2549,33 @@ gnus-group-jump-to-group
     (gnus-group-position-point)))
 
 (defun gnus-group-goto-group (group &optional far test-marked)
-  "Goto to newsgroup GROUP.
+  "Go to newsgroup GROUP.
 If FAR, it is likely that the group is not on the current line.
-If TEST-MARKED, the line must be marked."
+If TEST-MARKED, the line must be marked.
+
+Return nil if GROUP is not found."
   (when group
-    (let ((start (point))
-	  (active (and (or
-                        ;; Some kind of group may be only there.
-                        (gnus-active group)
-                        ;; All groups (but with exception) are there.
-                        (gnus-group-entry group))
-		       group)))
+    (let ((start (point)))
       (beginning-of-line)
       (cond
        ;; It's quite likely that we are on the right line, so
        ;; we check the current line first.
        ((and (not far)
-	     (equal (get-text-property (point) 'gnus-group) active)
+	     (equal (get-text-property (point) 'gnus-group) group)
 	     (or (not test-marked) (gnus-group-mark-line-p)))
 	(point))
        ;; Previous and next line are also likely, so we check them as well.
        ((and (not far)
 	     (save-excursion
 	       (forward-line -1)
-	       (and (equal (get-text-property (point) 'gnus-group) active)
+	       (and (equal (get-text-property (point) 'gnus-group) group)
 		    (or (not test-marked) (gnus-group-mark-line-p)))))
 	(forward-line -1)
 	(point))
        ((and (not far)
 	     (save-excursion
 	       (forward-line 1)
-	       (and (equal (get-text-property (point) 'gnus-group) active)
+	       (and (equal (get-text-property (point) 'gnus-group) group)
 		    (or (not test-marked) (gnus-group-mark-line-p)))))
 	(forward-line 1)
 	(point))
@@ -2588,7 +2584,7 @@ gnus-group-goto-group
 	(let (found)
 	  (while (and (not found)
 		      (gnus-text-property-search
-		       'gnus-group active 'forward 'goto))
+		       'gnus-group group 'forward 'goto))
 	    (if (gnus-group-mark-line-p)
 		(setq found t)
 	      (forward-line 1)))
@@ -2596,7 +2592,7 @@ gnus-group-goto-group
        (t
 	;; Search through the entire buffer.
 	(if (gnus-text-property-search
-	     'gnus-group active nil 'goto)
+	     'gnus-group group nil 'goto)
 	    (point)
 	  (goto-char start)
 	  nil))))))