bug#50466: 28.0.50; isearch isearch-repeat-on-direction-change vs isearch-beginning-of-buffer

bug#50466: 28.0.50; isearch isearch-repeat-on-direction-change vs isearch-beginning-of-buffer

[Ergus via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi:

I have this config in my system:

(setq isearch-repeat-on-direction-change t)
(define-key isearch-mode-map (kbd "M-<") #'isearch-beginning-of-buffer)

If I have the cursor on the beginning of:

1abc
2abc
3abc
4abc

and do

C-s abc ;; cursor at the end of 1abc
C-s     ;; cursor at the end of 2abc
C-s     ;; cursor at the end of 3abc
C-r     ;; cursor at the beginning of 2abc
M-<     ;; cursor at the end of 3abc <- this is wrong

The cursor goes to the end of 3abc (next candidate) instead of 1abc
(first) as expected from isearch-beginning-of-buffer.

If I repeat M-< then it goes to the right candidate (end of 1abc).

This seems related with the fact that the cursor needs to move to the
other-end or something like that. Because isearch-beginning-of-buffer
does not respect when the cursor was previously at the beginning or the
end of the candidates.





In GNU Emacs 28.0.50 (build 6, x86_64-pc-linux-gnu, GTK+ Version 3.24.30, cairo version 1.17.4)
 of 2021-09-06 built on Ergus
Repository revision: 61e5570863aa236628a87c722912dea86d2903b0
Repository branch: master
System Description: Arch Linux

Configured using:
 'configure --prefix=/home/ergo/.local/ --with-mailutils --with-json
 --with-x-toolkit=gtk3 --with-xft --with-wide-int --with-modules
 --with-cairo --with-harfbuzz --with-native-compilation'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER
PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM
GTK3 ZLIB

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

Major mode: Lisp Interaction

Minor modes in effect:
  show-paren-mode: t
  global-auto-revert-mode: t
  xclip-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  electric-pair-mode: t
  flyspell-mode: t
  company-mode: t
  flycheck-mode: t
  counsel-mode: t
  ivy-mode: t
  composable-mark-mode: t
  composable-mode: t
  repeat-mode: t
  xterm-mouse-mode: t
  winner-mode: t
  save-place-mode: t
  which-key-mode: t
  override-global-mode: t
  delete-selection-mode: t
  savehist-mode: t
  global-display-fill-column-indicator-mode: t
  display-fill-column-indicator-mode: t
  global-display-line-numbers-mode: t
  display-line-numbers-mode: t
  eldoc-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  context-menu-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t

Load-path shadows:
/usr/share/emacs/site-lisp/cmake-mode hides /home/ergo/.emacs.d/elpa/cmake-mode-20210104.1831/cmake-mode
/home/ergo/.emacs.d/elpa/magit-20210906.1023/magit-section-pkg hides /home/ergo/.emacs.d/elpa/magit-section-20210829.1849/magit-section-pkg
/usr/share/emacs/site-lisp/notmuch-crypto hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-crypto
/usr/share/emacs/site-lisp/notmuch-compat hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-compat
/usr/share/emacs/site-lisp/notmuch-hello hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-hello
/usr/share/emacs/site-lisp/notmuch hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch
/usr/share/emacs/site-lisp/notmuch-show hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-show
/usr/share/emacs/site-lisp/notmuch-maildir-fcc hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-maildir-fcc
/usr/share/emacs/site-lisp/coolj hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/coolj
/usr/share/emacs/site-lisp/notmuch-draft hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-draft
/usr/share/emacs/site-lisp/notmuch-tree hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-tree
/usr/share/emacs/site-lisp/notmuch-parser hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-parser
/usr/share/emacs/site-lisp/notmuch-lib hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-lib
/usr/share/emacs/site-lisp/notmuch-mua hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-mua
/usr/share/emacs/site-lisp/notmuch-message hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-message
/usr/share/emacs/site-lisp/notmuch-address hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-address
/usr/share/emacs/site-lisp/notmuch-wash hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-wash
/usr/share/emacs/site-lisp/notmuch-tag hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-tag
/usr/share/emacs/site-lisp/notmuch-print hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-print
/usr/share/emacs/site-lisp/notmuch-query hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-query
/usr/share/emacs/site-lisp/notmuch-jump hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-jump
/usr/share/emacs/site-lisp/notmuch-company hides /home/ergo/.emacs.d/elpa/notmuch-20210830.2334/notmuch-company
/home/ergo/.emacs.d/elpa/transient-20210819.2118/transient hides /home/ergo/.local/share/emacs/28.0.50/lisp/transient

Features:
(shadow sort notmuch-company notmuch-lib notmuch-version notmuch-compat
mm-view mml-smime smime dig mailcap mail-extr emacsbug message rmc puny
rfc822 mml mml-sec epa derived epg rfc6068 epg-config gnus-util rmail
rmail-loaddefs auth-source password-cache time-date mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils thingatpt amx s
paren autorevert filenotify xclip yasnippet-snippets yasnippet elec-pair
flyspell-correct-ivy flyspell-correct flyspell ispell company-keywords
company-gtags company-dabbrev-code company-dabbrev company-files
company-semantic company-template company-capf company flycheck json map
find-func dash counsel xdg xref project dired dired-loaddefs compile
text-property-search comint ansi-color swiper ivy-avy avy ivy flx
ivy-faces ivy-overlay colir pcase term/tmux term/xterm xterm jka-compr
init composable composable-mark powerline comp comp-cstr warnings subr-x
powerline-separators color powerline-themes repeat xt-mouse
simple-16-theme winner ring saveplace diminish edmacro kmacro which-key
advice configmail cl-extra help-mode use-package use-package-ensure
use-package-delight use-package-diminish use-package-bind-key bind-key
use-package-core disp-table delsel savehist easy-mmode
display-fill-column-indicator display-line-numbers info ede/auto
eieio-base cl-seq seq eieio byte-opt bytecomp byte-compile cconv
eieio-core cl-macs gv eieio-loaddefs cl-loaddefs cl-lib tex-site rx
slime-autoloads early-init iso-transl tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-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 cl-generic 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 charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice button
loaddefs faces cus-face macroexp files window text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 262277 6438)
 (symbols 48 18897 0)
 (strings 32 60245 8110)
 (string-bytes 1 2209331)
 (vectors 16 30930)
 (vector-slots 8 648313 10429)
 (floats 8 204 994)
 (intervals 56 1285 0)
 (buffers 992 11))

bug#50466: 28.0.50; isearch isearch-repeat-on-direction-change vs isearch-beginning-of-buffer

[Lars Ingebrigtsen]

Ergus <spacibba@aol.com> writes:

> (setq isearch-repeat-on-direction-change t)
> (define-key isearch-mode-map (kbd "M-<") #'isearch-beginning-of-buffer)
>
> If I have the cursor on the beginning of:
>
> 1abc
> 2abc
> 3abc
> 4abc
>
> and do
>
> C-s abc ;; cursor at the end of 1abc
> C-s     ;; cursor at the end of 2abc
> C-s     ;; cursor at the end of 3abc
> C-r     ;; cursor at the beginning of 2abc
> M-<     ;; cursor at the end of 3abc <- this is wrong
>
> The cursor goes to the end of 3abc (next candidate) instead of 1abc
> (first) as expected from isearch-beginning-of-buffer.

Simpler reproduction: Go to the end of the 3abc line and

C-r abc
M-<

That'll take you to the end of 4abc.  And it's because:

(defun isearch-beginning-of-buffer (&optional arg)
[...]
    (setq isearch-just-started t)
    (goto-char (point-min))
    (isearch-repeat 'forward arg)))

(defun isearch-repeat (direction &optional count)
[...]
    ;; C-s in reverse or C-r in forward, change direction.
    (if (and isearch-other-end isearch-repeat-on-direction-change)
        (goto-char isearch-other-end))

The following patch seems to fix the problem:

diff --git a/lisp/isearch.el b/lisp/isearch.el
index 1c776a06e1..6abd12700c 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1898,7 +1898,8 @@ isearch-repeat
 	      (funcall isearch-wrap-function)
 	    (goto-char (if isearch-forward (point-min) (point-max))))))
     ;; C-s in reverse or C-r in forward, change direction.
-    (if (and isearch-other-end isearch-repeat-on-direction-change)
+    (if (and isearch-other-end isearch-repeat-on-direction-change
+             (not isearch-just-started))
         (goto-char isearch-other-end))
     (setq isearch-forward (not isearch-forward)
 	  isearch-success t))

But I'm not really very familiar with the isearch machinery, so I've
added Juri to the CCs; perhaps he has a comment here.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#50466: 28.0.50; isearch isearch-repeat-on-direction-change vs isearch-beginning-of-buffer

[Juri Linkov]

> But I'm not really very familiar with the isearch machinery, so I've
> added Juri to the CCs; perhaps he has a comment here.

Gregory already provided the patch on emacs-devel that fixes this.
But that patch was not installed because the copyright assignment process
was still in progress.  This patch could be installed now.
The patch name was 0001-Fix-behavior-of-isearch-beginning-end-of-buffer.patch
and its date: Sat, 10 Apr 2021 23:55:50 +0000 (5 months, 1 day, 19 hours ago)

bug#50466: 28.0.50; isearch isearch-repeat-on-direction-change vs isearch-beginning-of-buffer

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

>> But I'm not really very familiar with the isearch machinery, so I've
>> added Juri to the CCs; perhaps he has a comment here.
>
> Gregory already provided the patch on emacs-devel that fixes this.
> But that patch was not installed because the copyright assignment process
> was still in progress.  This patch could be installed now.
> The patch name was 0001-Fix-behavior-of-isearch-beginning-end-of-buffer.patch
> and its date: Sat, 10 Apr 2021 23:55:50 +0000 (5 months, 1 day, 19 hours ago)

Right; now applied.  (I also applied the other patch in that post, since
it seemed like it was agreed that that was a good feature.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#50466: 28.0.50; isearch isearch-repeat-on-direction-change vs isearch-beginning-of-buffer

[Juri Linkov]

>> Gregory already provided the patch on emacs-devel that fixes this.
>> But that patch was not installed because the copyright assignment process
>> was still in progress.  This patch could be installed now.
>> The patch name was 0001-Fix-behavior-of-isearch-beginning-end-of-buffer.patch
>> and its date: Sat, 10 Apr 2021 23:55:50 +0000 (5 months, 1 day, 19 hours ago)
>
> Right; now applied.  (I also applied the other patch in that post, since
> it seemed like it was agreed that that was a good feature.)

Thanks Gregory for this great feature, and thank you for installing it.
I had to tweak it a little.  I added the same logic from the patch
to isearch-beginning-of-buffer and isearch-end-of-buffer.
But these commands are quite obsolete now because of 'isearch-allow-motion',
so I mentioned this in their docstrings.