Subject: bug#67445: Likely Gnus bracketing mistake

From: Eric Abrahamsen <[email-masked]>
Date: Sat, 25 Nov 2023 13:19:56 -0800

Eli Zaretskii <[email-masked]> writes:

>> From: Mattias Engdegård <[email-masked]>
>> Date: Sat, 25 Nov 2023 15:50:59 +0100
>> 
>> This looks like a bracketing mistake (gnus-group.el line 1758):
>> 
>> >     (if (and backward (progn (beginning-of-line)) (bobp))
>> 
>> especially considering `beginning-of-line` always returns nil.

That whole special-case test doesn't seem necessary, I think the
attached patch is sufficient. I've tested it lightly, does it look okay?

>> (Is there a Gnus maintainer these days?)
>
> Yes, Eric (CC'ed).

Hey, I've been promoted!

diff --git a/lisp/gnus/gnus-group.el b/lisp/gnus/gnus-group.el
index 01e6a8f317f..41d92e715ee 100644
--- a/lisp/gnus/gnus-group.el
+++ b/lisp/gnus/gnus-group.el
@@ -1745,45 +1745,43 @@ gnus-group-level
       gnus-level-killed))
 
 (defun gnus-group-search-forward (&optional backward all level first-too)
-  "Find the next newsgroup with unread articles.
-If BACKWARD is non-nil, find the previous newsgroup instead.
-If ALL is non-nil, just find any newsgroup.
-If LEVEL is non-nil, find group with level LEVEL, or higher if no such
-group exists.
-If FIRST-TOO, the current line is also eligible as a target."
+  "Move point to the next newsgroup with unread articles.
+If BACKWARD is non-nil, move to the previous newsgroup instead.
+If ALL is non-nil, consider any newsgroup, not only those with
+unread articles.  If LEVEL is non-nil, find group with level
+LEVEL, or higher if no such group exists.  If FIRST-TOO, the
+current line is also eligible as a target."
   (let ((way (if backward -1 1))
 	(low gnus-level-killed)
 	(beg (point))
 	pos found lev)
-    (if (and backward (progn (beginning-of-line)) (bobp))
-	nil
-      (unless first-too
-	(forward-line way))
-      (while (and
-	      (not (eobp))
-	      (not (setq
-		    found
-		    (and
-		     (get-text-property (point) 'gnus-group)
-		     (or all
-			 (and
-			  (let ((unread
-				 (get-text-property (point) 'gnus-unread)))
-			    (and (numberp unread) (> unread 0)))
-			  (setq lev (get-text-property (point)
-						       'gnus-level))
-			  (<= lev gnus-level-subscribed)))
-		     (or (not level)
-			 (and (setq lev (get-text-property (point)
-							   'gnus-level))
-			      (or (= lev level)
-				  (and (< lev low)
-				       (< level lev)
-				       (progn
-					 (setq low lev)
-					 (setq pos (point))
-					 nil))))))))
-	      (zerop (forward-line way)))))
+    (unless first-too
+      (forward-line way))
+    (while (and
+	    (not (if (> 0 way) (eobp) (bobp)))
+	    (not (setq
+		  found
+		  (and
+		   (get-text-property (point) 'gnus-group)
+		   (or all
+		       (and
+			(let ((unread
+			       (get-text-property (point) 'gnus-unread)))
+			  (and (numberp unread) (> unread 0)))
+			(setq lev (get-text-property (point)
+						     'gnus-level))
+			(<= lev gnus-level-subscribed)))
+		   (or (not level)
+		       (and (setq lev (get-text-property (point)
+							 'gnus-level))
+			    (or (= lev level)
+				(and (< lev low)
+				     (< level lev)
+				     (progn
+				       (setq low lev)
+				       (setq pos (point))
+				       nil))))))))
+	    (zerop (forward-line way))))
     (if found
 	(progn (gnus-group-position-point) t)
       (goto-char (or pos beg))