bug#67526: 30.0.50; Regression: gnus-group-prev-group fails to move to first group

bug#67526: 30.0.50; Regression: gnus-group-prev-group fails to move to first group

[Simon Leinen]

In the current development version of Emacs (Git master branch),
gnus-group-prev-group (bound to the "P" key) in Gnus's group buffer
works as before, except when it is on the second newgroup line.  In this
case, the cursor stays put on the second line and refuses to move up to
the first.  The same is true for gnus-group-prev-unread-group (bound to
"p"), even when the first group has unread messages.

I suspect the issue lies in gnus-group-search-forward, which was
recently simplified by Eric Abrahamsen in commit
7304cc8a9ca8a7d19baaa24f0a72c7ad9a6a9716

Cheers,
-- 
Simon.

In GNU Emacs 30.0.50 (build 1, aarch64-apple-darwin22.6.0, NS
 appkit-2299.70 Version 13.6.1 (Build 22G313)) of 2023-11-27 built on
 macsl
Repository revision: 7a5c91a2831602c3cd961158cf0b6a876852d7ac
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2299
System Description:  macOS 13.6.1

Configured using:
 'configure --verbose --with-imagemagick --with-cairo
 LIBXML2_CFLAGS=-I/opt/homebrew/opt/libxml2/include/libxml2
 'LIBXML2_LIBS=-L/opt/homebrew/opt/libxml2/lib -lxml2'
 --with-native-compilation 'LDFLAGS=-L/opt/homebrew/lib/gcc/current
 -L/opt/homebrew/lib -lgmp' CFLAGS=-I/opt/homebrew/include
 PKG_CONFIG_PATH=/opt/homebrew/lib/pkgconfig:/opt/homebrew/share/pkgconfig'

Configured features:
ACL GIF GLIB GNUTLS IMAGEMAGICK JPEG JSON LCMS2 LIBXML2 MODULES
NATIVE_COMP NOTIFY KQUEUE NS PDUMPER PNG RSVG SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP XIM ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Help

Minor modes in effect:
  global-git-commit-mode: t
  magit-auto-revert-mode: t
  csv-field-index-mode: t
  auto-insert-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  isearch-fold-quotes-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  minibuffer-regexp-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  temp-buffer-resize-mode: t

Load-path shadows:
/Users/leinen/.emacs.d/elpa/transient-20230602.2121/transient hides /Applications/Emacs.app/Contents/Resources/lisp/transient
/Users/leinen/.emacs.d/elpa/auth-source-pass-20210210.1908/auth-source-pass hides /Applications/Emacs.app/Contents/Resources/lisp/auth-source-pass
/Users/leinen/.emacs.d/elpa/eglot-20220926.1235/eglot hides /Applications/Emacs.app/Contents/Resources/lisp/progmodes/eglot

Features:
(shadow emacsbug magit-submodule magit-blame magit-stash magit-reflog
magit-bisect magit-push magit-pull magit-fetch magit-clone magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-tag
magit-merge magit-branch magit-reset magit-files magit-refs magit-status
magit magit-repos magit-apply magit-wip magit-log which-func imenu
magit-diff git-commit log-edit add-log magit-core magit-autorevert
autorevert filenotify magit-margin magit-transient magit-process
with-editor server magit-mode transient edmacro kmacro magit-git
magit-base magit-section cursor-sensor dash compat bug-reference debug
backtrace tabify cus-edit cus-start cl-print gnus-cus gnus-demon
gnus-diary nndiary gnus-dup gnus-fun gnus-html url-queue mm-url
gnus-logic gnus-mh mh-comp mh-scan mh-gnus mh-e mh-buffers mh-loaddefs
gnus-registry registry eieio-base rmailsum rmail gnus-salt gnus-topic
gnus-uu yenc nnselect shortdoc help-fns radix-tree conf-mode rfc2104
vc-hg vc-bzr tramp-cmds tramp-cache time-stamp tramp-sh gnus-draft
csv-mode mailalias bbdb-message smerge-mode diff vc perl-mode url-cache
display-line-numbers vc-git diff-mode novice dired-aux vc-dispatcher
m4-mode i-d rect vc-filewise vc-rcs log-view pcvs-util html-mode
html-helper-mode tempo sgml-mode facemenu log-mode url-http url-auth
url-gw jka-compr gnus-icalendar org-capture org-refile org ob ob-tangle
ob-ref ob-lob ob-table ob-exp org-macro org-src ob-comint org-pcomplete
org-list org-footnote org-faces org-entities ob-emacs-lisp ob-core
ob-eval org-cycle org-table ol org-fold org-fold-core org-keys oc
org-loaddefs org-version org-compat org-macs icalendar diary-lib
diary-loaddefs cal-menu calendar cal-loaddefs shr-color color misearch
multi-isearch flow-fill qp vm-init gnus-vm mm-archive sort smiley
gnus-cite textsec uni-scripts idna-mapping uni-confusable textsec-check
gnus-async gnus-bcklg gnus-kill gnus-ml disp-table nndraft nnmh nndoc
utf-7 nnml gnus-init dot-gnus-sa bbdb-gnus epa-file network-stream nsm
gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-msg gnus-art
mm-uu mml2015 mm-view mml-smime nntp gnus-cache gnus-sum shr pixel-fill
kinsoku url-file svg dom gnus-group gnus-undo gnus-start gnus-dbus dbus
xml gnus-cloud nnimap nnmail mail-source nnoo gnus-spec gnus-int
gnus-range message yank-media rfc822 mml mml-sec epa epg rfc6068
epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 gmm-utils
mailheader gnus-win vm-vcard vm-user vm-smime bbdb-sc supercite regi
mail-extr vm-serial vm-search bbdb-vm bbdb-mua vm-rfaddons ps-print
ps-print-loaddefs lpr vm-ps-print vm-pine vm-pgg pgg pgg-parse pgg-def
vm-pcrisis comp comp-cstr cl-extra smime gnutls puny dig vm-message
vm-message-history vm-macro vm-license vm-startup vm vm-grepmail vm-mark
vm-edit dired dired-loaddefs vm-dired vm-digest vm-biff bbdb-com crm
mailabbrev cl vm-sort vm-reply vm-save vm-delete vm-undo vm-page
vm-summary-faces vm-pop utf7 vm-imap vm-thread vm-mime vm-motion
vm-mouse vm-toolbar vm-menu vm-window vm-crypto vm-summary vm-minibuf
vm-virtual vm-avirtual vm-autoload vm-autoloads vm-version vm-vars vcard
vm-folder vm-misc u-vm-color tapestry compile-vm find-func warnings
tramp trampver tramp-integration tramp-message help-mode tramp-compat
xdg shell parse-time iso8601 format-spec tramp-loaddefs avoid gnus
nnheader gnus-util time-date range wid-edit autoinsert cus-load
exec-path-from-shell eshell esh-cmd esh-ext esh-opt esh-proc esh-io
esh-arg pcomplete esh-module esh-groups esh-util files-x slime-media
slime-mrepl inferior-slime slime-fancy slime-indentation slime-cl-indent
cl-indent slime-trace-dialog slime-fontifying-fu slime-package-fu
slime-references slime-compiler-notes-tree slime-scratch
slime-presentations bridge slime-macrostep macrostep slime-mdot-fu
slime-enclosing-context slime-fuzzy slime-fancy-trace
slime-fancy-inspector slime-c-p-c slime-editing-commands slime-autodoc
slime-repl slime-parse slime easy-mmode apropos etags fileloop generator
xref project arc-mode archive-mode noutline outline icons pp hyperspec
thingatpt advice bbdb derived bbdb-site timezone smtpmail sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils timeclock
compile text-property-search comint ansi-osc ansi-color ring comp-run
comp-common 0blayout-autoloads adoc-mode-autoloads
aggressive-completion-autoloads all-the-icons-dired-autoloads
all-the-icons-ibuffer-autoloads all-the-icons-ivy-rich-autoloads
auto-complete-autoloads rx bbdb-autoloads cargo-autoloads
cargo-mode-autoloads crdt-autoloads csv-mode-autoloads
dired-git-autoloads all-the-icons-autoloads async-await-autoloads
direnv-autoloads django-commands-autoloads dockerfile-mode-autoloads
eglot-autoloads exec-path-from-shell-autoloads flycheck-autoloads
go-mode-autoloads helm-autoloads helm-core-autoloads async-autoloads
hydra-autoloads iter2-autoloads ivy-rich-autoloads ivy-autoloads
jinx-autoloads list-unicode-display-autoloads lua-mode-autoloads
magit-autoloads pcase git-commit-autoloads nix-mode-autoloads
magit-section-autoloads nix-update-autoloads php-mode-autoloads
popup-autoloads pos-tip-autoloads projectile-autoloads
reformatter-autoloads finder-inf rust-mode-autoloads s-autoloads
company-autoloads slime-autoloads macrostep-autoloads sx-autoloads
markdown-mode-autoloads terraform-mode-autoloads dash-autoloads
hcl-mode-autoloads transient-autoloads tree-sitter-langs-autoloads
tree-sitter-autoloads treepy-autoloads tsc-autoloads web-mode-autoloads
with-editor-autoloads info compat-autoloads xml-rpc-autoloads
xterm-color-autoloads yaml-autoloads yaml-mode-autoloads package
browse-url url url-proxy url-privacy url-expand url-methods url-history
url-cookie generate-lisp-file url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs password-cache
json subr-x map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs
cl-lib early-init rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/ns-win ns-win ucs-normalize mule-util term/common-win touch-screen
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode lisp-mode prog-mode register page tab-bar menu-bar
rfn-eshadow isearch easymenu timer select scroll-bar mouse jit-lock
font-lock syntax font-core term/tty-colors frame minibuffer nadvice seq
simple cl-generic indonesian philippine cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite emoji-zwj charscript charprop case-table
epa-hook jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button
loaddefs theme-loaddefs faces cus-face macroexp files window
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget keymap hashtable-print-readable backquote threads kqueue
cocoa ns lcms2 multi-tty make-network-process native-compile emacs)

Memory information:
((conses 16 3134762 535482) (symbols 48 56061 1)
 (strings 32 542776 21641) (string-bytes 1 17547753)
 (vectors 16 207492) (vector-slots 8 3810397 458734)
 (floats 8 1694 18270) (intervals 56 229832 2696) (buffers 992 177))

bug#67526: 30.0.50; Regression: gnus-group-prev-group fails to move to first group

[Eli Zaretskii]

> From: Simon Leinen <simon.leinen@switch.ch>
> Date: Wed, 29 Nov 2023 11:16:20 +0100
> 
> In the current development version of Emacs (Git master branch),
> gnus-group-prev-group (bound to the "P" key) in Gnus's group buffer
> works as before, except when it is on the second newgroup line.  In this
> case, the cursor stays put on the second line and refuses to move up to
> the first.  The same is true for gnus-group-prev-unread-group (bound to
> "p"), even when the first group has unread messages.
> 
> I suspect the issue lies in gnus-group-search-forward, which was
> recently simplified by Eric Abrahamsen in commit
> 7304cc8a9ca8a7d19baaa24f0a72c7ad9a6a9716

Adding Eric to the discussion.

bug#67526: 30.0.50; Regression: gnus-group-prev-group fails to move to first group

[Eric Abrahamsen]

On 11/29/23 15:16 PM, Eli Zaretskii wrote:
>> From: Simon Leinen <simon.leinen@switch.ch>
>> Date: Wed, 29 Nov 2023 11:16:20 +0100
>> 
>> In the current development version of Emacs (Git master branch),
>> gnus-group-prev-group (bound to the "P" key) in Gnus's group buffer
>> works as before, except when it is on the second newgroup line.  In this
>> case, the cursor stays put on the second line and refuses to move up to
>> the first.  The same is true for gnus-group-prev-unread-group (bound to
>> "p"), even when the first group has unread messages.
>> 
>> I suspect the issue lies in gnus-group-search-forward, which was
>> recently simplified by Eric Abrahamsen in commit
>> 7304cc8a9ca8a7d19baaa24f0a72c7ad9a6a9716
>
> Adding Eric to the discussion.

I'm not able to reproduce this, with or without topic-mode. Is there
anything special about the level of the first or the second group, and
`gnus-level-subscribed'? I can't see anything else that might influence
behavior here...

bug#67526: 30.0.50; Regression: gnus-group-prev-group fails to move to first group

[Simon Leinen]

Dear Eric,

thanks for looking at this!

> I'm not able to reproduce this, with or without topic-mode. Is there
> anything special about the level of the first or the second group, and
> `gnus-level-subscribed'? I can't see anything else that might influence
> behavior here...

How do I find out whether anything is special about the level of those
groups? For me,

gnus-topic-mode is nil
gnus-level-subscribed is 5

The top three lines of my newsgroup list look like this:

     193: nnimap+mail.switch.ch:INBOX
      19: nnimap+mail.switch.ch:switch/network
      27: nnimap+mail.switch.ch:net/freertr

Reproducing the issue is as easy as typing "n" and then "p".  The cursor
will be stuck on the second line.  I can try narrowing this case, but
it's hard for me to get gnus working again from emacs -q :-)

I do note that the issue goes away when I revert a bit of your change,
as follows.  I haven't found out *why* that is necessary to get the old
behavior back...

diff --git a/lisp/gnus/gnus-group.el b/lisp/gnus/gnus-group.el
index 9a1a6f9b27d..2f81ea96210 100644
--- a/lisp/gnus/gnus-group.el
+++ b/lisp/gnus/gnus-group.el
@@ -1758,7 +1758,7 @@ gnus-group-search-forward
     (unless first-too
       (forward-line way))
     (while (and
-	    (not (if backward (bobp) (eobp)))
+	    (not (endp))
 	    (not (setq
 		  found
 		  (and

bug#67526: 30.0.50; Regression: gnus-group-prev-group fails to move to first group

[Simon Leinen]

> I do note that the issue goes away when I revert a bit of your
> change, as follows.  I haven't found out *why* that is necessary to
> get the old behavior back...

OK, I think I know now.  It was a mistake to add the "(if backward
(bobp) ...)" branch.  Probably the intent was to restore symmetry for
the backward-moving case.  But the pre-existing (eobp) test will
succeed only when the cursor is *beyond* the last newsgroup line -
because the end of the buffer is not at the end of the last newsgroup
line, but on an empty line *after* that last newsgroup line.  Testing
for (bobp) in the backwards case is not equivalent, because (at least
in my case) the beginning of the buffer position is very much *on* a
valid/eligible newsgroup line.  Maybe this is not the case for Eric,
because he uses topic mode? Anyway in my limited testing, the old
logic works fine for that case too.

So I suggest to revert that part of Eric's change, as per the patch
I'm resending here with a correction - I had written ENDP instead of
EOBP, sorry about the confusion.

diff --git a/lisp/gnus/gnus-group.el b/lisp/gnus/gnus-group.el
index 9a1a6f9b27d..f2258026eca 100644
--- a/lisp/gnus/gnus-group.el
+++ b/lisp/gnus/gnus-group.el
@@ -1758,7 +1758,7 @@ gnus-group-search-forward
     (unless first-too
       (forward-line way))
     (while (and
-	    (not (if backward (bobp) (eobp)))
+	    (not (eobp))
 	    (not (setq
 		  found
 		  (and

If you're worried - and I haven't thought through whether this should
be of any concern - that the (eobp) check should not be done in the
backwards case, then a suitable alternate clause could be

          (or backward (not (eobp)))
-- 
Simon.

bug#67526: 30.0.50; Regression: gnus-group-prev-group fails to move to first group

[Eric Abrahamsen]

Simon Leinen <simon.leinen@switch.ch> writes:

>> I do note that the issue goes away when I revert a bit of your
>> change, as follows.  I haven't found out *why* that is necessary to
>> get the old behavior back...
>
> OK, I think I know now.  It was a mistake to add the "(if backward
> (bobp) ...)" branch.  Probably the intent was to restore symmetry for
> the backward-moving case.  But the pre-existing (eobp) test will
> succeed only when the cursor is *beyond* the last newsgroup line -
> because the end of the buffer is not at the end of the last newsgroup
> line, but on an empty line *after* that last newsgroup line.  Testing
> for (bobp) in the backwards case is not equivalent, because (at least
> in my case) the beginning of the buffer position is very much *on* a
> valid/eligible newsgroup line.  Maybe this is not the case for Eric,
> because he uses topic mode? Anyway in my limited testing, the old
> logic works fine for that case too.
>
> So I suggest to revert that part of Eric's change, as per the patch
> I'm resending here with a correction - I had written ENDP instead of
> EOBP, sorry about the confusion.
>
> diff --git a/lisp/gnus/gnus-group.el b/lisp/gnus/gnus-group.el
> index 9a1a6f9b27d..f2258026eca 100644
> --- a/lisp/gnus/gnus-group.el
> +++ b/lisp/gnus/gnus-group.el
> @@ -1758,7 +1758,7 @@ gnus-group-search-forward
>      (unless first-too
>        (forward-line way))
>      (while (and
> -	    (not (if backward (bobp) (eobp)))
> +	    (not (eobp))
>  	    (not (setq
>  		  found
>  		  (and
>
> If you're worried - and I haven't thought through whether this should
> be of any concern - that the (eobp) check should not be done in the
> backwards case, then a suitable alternate clause could be
>
>           (or backward (not (eobp)))

Well that's weird -- I'm not sure how I boffed my testing, but I'd
boffed it, and now I'm seeing the same result as you. Thanks for doing
the extra work.

I'm just going to revert to the original logic, with the obvious fix to
the progn sexp. I should know better than to start stomping off across
unfamiliar code...