bug#17453: Isearch doesn't work properly with Follow Mode.

[Alan Mackenzie]

Hi, Emacs.

Isearch doesn't work properly with Follow Mode.  There follows a patch
which is a first attempt to fix this.

There are three main problems (there may be others):

1. Isearch commands stay in the same window when they ought to move
across the entire screen.  This is only actually a problem with lazy
highlighting enabled.

2. Lazy highlighting is only applied to the current follow window, not
all of them.

3. Scroll commands cannot scroll out of the current window.  They ought
to be able to scroll through any follow window.

Most of the exercise was straightforward.  The only trouble came where
isearch-message (or isearch-message-function) was called from
isearch-search.  There are circumstances (see comments in the code)
where calling isearch-message with follow-mode enabled can cause
unwanted vertical scrolling.  This happens when point is temporarily
moved to isearch-other-end and then calling isearch-search.

To get round this I refactored isearch-message out of isearch-search and
inserted into the places just before isearch-search is called.

Here's the patch:




--- emacs/emacs.bzr/trunk/lisp/isearch.el       2014-03-03 11:50:56.000000000 
+0000
+++ /home/acm/isearch.el        2014-05-09 22:10:29.000000000 +0000
@@ -166,6 +166,21 @@
   :type 'boolean
   :group 'isearch)
 
+(defvar isearch-windows nil
+  "List of windows active in the incremental search.
+This is either the ordered list of active windows administered by
+`follow-mode' or a list of the single window involved in the
+search.")
+
+(defmacro isearch-windows-start (&optional wins)
+  "Get the start point of the windows involved in the isearch."
+  `(window-start (car ,(if wins wins 'isearch-windows))))
+
+(defmacro isearch-windows-end (&optional wins update)
+  "Get the end point of the windows involved in the isearch."
+  `(window-end (car (last ,(if wins wins 'isearch-windows)))
+              ,@(if update `(,update))))
+
 (defvar isearch-mode-hook nil
   "Function(s) to call after starting up an incremental search.")
 
@@ -184,6 +199,11 @@
   "Function to call to display the search prompt.
 If nil, use function `isearch-message'.")
 
+(defmacro isearch-call-message (&optional cqh ellip)
+  `(if isearch-message-function
+       (funcall isearch-message-function ,cqh ,ellip)
+     (isearch-message ,cqh ,ellip)))
+
 (defvar isearch-wrap-function nil
   "Function to call to wrap the search when search is failed.
 If nil, move point to the beginning of the buffer for a forward search,
@@ -203,7 +223,7 @@
 `isearch-message-prefix' advice property to specify the prefix string
 displayed in the search message.")
 
-;; Search ring.
+;;; Search ring.
 
 (defvar search-ring nil
   "List of search string sequences.")
@@ -860,6 +880,9 @@
        isearch-regexp regexp
        isearch-word word
        isearch-op-fun op-fun
+       isearch-windows (if (and (boundp 'follow-mode) follow-mode)
+                           (follow-all-followers)
+                         (list (selected-window)))
        isearch-last-case-fold-search isearch-case-fold-search
        isearch-case-fold-search case-fold-search
        isearch-invisible search-invisible
@@ -1254,13 +1277,6 @@
          (unwind-protect
              (progn ,@body)
 
-           ;; Set point at the start (end) of old match if forward (backward),
-           ;; so after exiting minibuffer isearch resumes at the start (end)
-           ;; of this match and can find it again.
-           (if (and old-other-end (eq old-point (point))
-                    (eq isearch-forward isearch-new-forward))
-               (goto-char old-other-end))
-
            ;; Always resume isearching by restarting it.
            (isearch-mode isearch-forward
                          isearch-regexp
@@ -1273,7 +1289,17 @@
                  isearch-message isearch-new-message
                  isearch-forward isearch-new-forward
                  isearch-word isearch-new-word
-                 isearch-case-fold-search isearch-new-case-fold))
+                 isearch-case-fold-search isearch-new-case-fold)
+
+           ;; Restore the minibuffer message before moving point.
+           (isearch-call-message nil t)
+
+           ;; Set point at the start (end) of old match if forward (backward),
+           ;; so after exiting minibuffer isearch resumes at the start (end)
+           ;; of this match and can find it again.
+           (if (and old-other-end (eq old-point (point))
+                    (eq isearch-forward isearch-new-forward))
+               (goto-char old-other-end)))
 
          ;; Empty isearch-string means use default.
          (when (= 0 (length isearch-string))
@@ -1417,16 +1443,20 @@
            (isearch-ring-adjust1 nil))
        ;; If already have what to search for, repeat it.
        (or isearch-success
-           (progn
-             ;; Set isearch-wrapped before calling isearch-wrap-function
-             (setq isearch-wrapped t)
-             (if isearch-wrap-function
-                 (funcall isearch-wrap-function)
-               (goto-char (if isearch-forward (point-min) (point-max)))))))
+           ;; Set isearch-wrapped before calling isearch-wrap-function
+           (setq isearch-wrapped t)))
     ;; C-s in reverse or C-r in forward, change direction.
     (setq isearch-forward (not isearch-forward)
          isearch-success t))
 
+  (isearch-call-message nil t)         ; Call before moving point.
+  (if (and (eq isearch-forward (eq direction 'forward))
+          (not (equal isearch-string ""))
+          (not isearch-success))
+      (if isearch-wrap-function
+         (funcall isearch-wrap-function)
+       (goto-char (if isearch-forward (point-min) (point-max)))))
+
   (setq isearch-barrier (point)) ; For subsequent \| if regexp.
 
   (if (equal isearch-string "")
@@ -1867,6 +1897,7 @@
                                            (length isearch-string))))
           isearch-message (mapconcat 'isearch-text-char-description
                                      isearch-string "")))
+  (isearch-call-message nil t)                 ; Do this before moving point.
   ;; Use the isearch-other-end as new starting point to be able
   ;; to find the remaining part of the search string again.
   ;; This is like what `isearch-search-and-update' does,
@@ -2041,6 +2072,7 @@
              (setq isearch-case-fold-search
                    (isearch-no-upper-case-p isearch-string isearch-regexp))))
       ;; Not regexp, not reverse, or no match at point.
+      (isearch-call-message nil t)     ; Do this before moving point.
       (if (and isearch-other-end (not isearch-adjusted))
          (goto-char (if isearch-forward isearch-other-end
                       (min isearch-opoint
@@ -2207,10 +2239,12 @@
 together with as much of the search string as will fit; the symbol
 `above' if we need to scroll the text downwards; the symbol `below',
 if upwards."
-  (let ((w-start (window-start))
-        (w-end (window-end nil t))
-        (w-L1 (save-excursion (move-to-window-line 1) (point)))
-        (w-L-1 (save-excursion (move-to-window-line -1) (point)))
+  (let ((w-start (isearch-windows-start))
+       (w-end (isearch-windows-end nil t))
+        (w-L1 (with-selected-window (car isearch-windows)
+               (save-excursion (move-to-window-line 1) (point))))
+        (w-L-1 (with-selected-window (car (last isearch-windows))
+                (save-excursion (move-to-window-line -1) (point))))
         start end)                  ; start and end of search string in buffer
     (if isearch-forward
         (setq end isearch-point  start (or isearch-other-end isearch-point))
@@ -2236,14 +2270,18 @@
       (setq start isearch-point  end (or isearch-other-end isearch-point)))
     (if above
         (progn
-          (goto-char start)
+          (select-window (car isearch-windows))
+         (goto-char start)
           (recenter 0)
-          (when (>= isearch-point (window-end nil t))
-            (goto-char isearch-point)
+          (when (>= isearch-point (isearch-windows-end nil t))
+            (select-window (car (last isearch-windows)))
+           (goto-char isearch-point)
             (recenter -1)))
+      (select-window (car (last isearch-windows)))
       (goto-char end)
       (recenter -1)
-      (when (< isearch-point (window-start))
+      (when (< isearch-point (isearch-windows-start))
+       (select-window (car isearch-windows))
         (goto-char isearch-point)
         (recenter 0))))
   (goto-char isearch-point))
@@ -2390,6 +2428,7 @@
   (isearch-ring-adjust1 advance)
   (if search-ring-update
       (progn
+       (isearch-call-message nil t)
        (isearch-search)
        (isearch-push-state)
        (isearch-update))
@@ -2462,6 +2501,13 @@
 
 (defun isearch-message (&optional c-q-hack ellipsis)
   ;; Generate and print the message string.
+
+  ;; N.B.: This function should always be called with point at the
+  ;; search point, because in certain (rare) circumstances, undesired
+  ;; scrolling can happen when point is elsewhere.  These
+  ;; circumstances are when follow-mode is active, the search string
+  ;; spans two (or several) windows, and the message about to be
+  ;; displayed will cause the echo area to expand.
   (let ((cursor-in-echo-area ellipsis)
        (m isearch-message)
        (fail-pos (isearch-fail-pos t)))
@@ -2630,9 +2676,6 @@
 
 (defun isearch-search ()
   ;; Do the search with the current search string.
-  (if isearch-message-function
-      (funcall isearch-message-function nil t)
-    (isearch-message nil t))
   (if (and (eq isearch-case-fold-search t) search-upper-case)
       (setq isearch-case-fold-search
            (isearch-no-upper-case-p isearch-string isearch-regexp)))
@@ -2936,8 +2979,9 @@
 (defvar isearch-lazy-highlight-timer nil)
 (defvar isearch-lazy-highlight-last-string nil)
 (defvar isearch-lazy-highlight-window nil)
-(defvar isearch-lazy-highlight-window-start nil)
-(defvar isearch-lazy-highlight-window-end nil)
+(defvar isearch-lazy-highlight-windows nil)
+(defvar isearch-lazy-highlight-windows-start 0)
+(defvar isearch-lazy-highlight-windows-end 0)
 (defvar isearch-lazy-highlight-case-fold-search nil)
 (defvar isearch-lazy-highlight-regexp nil)
 (defvar isearch-lazy-highlight-lax-whitespace nil)
@@ -2972,11 +3016,15 @@
 search string to change or the window to scroll).  It is also used
 by other Emacs features."
   (when (and (null executing-kbd-macro)
-             (sit-for 0)         ;make sure (window-start) is credible
+            ;; make sure (window-start) is credible
+             (if (and (boundp 'follow-mode) follow-mode)
+                (progn (follow-adjust-window (selected-window))
+                       t)
+              (sit-for 0))
              (or (not (equal isearch-string
                              isearch-lazy-highlight-last-string))
-                 (not (eq (selected-window)
-                          isearch-lazy-highlight-window))
+                 (not (memq (selected-window)
+                          isearch-lazy-highlight-windows))
                 (not (eq isearch-lazy-highlight-case-fold-search
                          isearch-case-fold-search))
                 (not (eq isearch-lazy-highlight-regexp
@@ -2987,10 +3035,10 @@
                          isearch-lax-whitespace))
                 (not (eq isearch-lazy-highlight-regexp-lax-whitespace
                          isearch-regexp-lax-whitespace))
-                 (not (= (window-start)
-                         isearch-lazy-highlight-window-start))
-                 (not (= (window-end)   ; Window may have been split/joined.
-                        isearch-lazy-highlight-window-end))
+                 (not (= (isearch-windows-start isearch-lazy-highlight-windows)
+                         isearch-lazy-highlight-windows-start))
+                 (not (= (isearch-windows-end isearch-lazy-highlight-windows)  
 ; Window may have been split/joined.
+                        isearch-lazy-highlight-windows-end))
                 (not (eq isearch-forward
                          isearch-lazy-highlight-forward))
                 ;; In case we are recovering from an error.
@@ -3005,8 +3053,14 @@
     (setq isearch-lazy-highlight-start-limit beg
          isearch-lazy-highlight-end-limit end)
     (setq isearch-lazy-highlight-window       (selected-window)
-         isearch-lazy-highlight-window-start (window-start)
-         isearch-lazy-highlight-window-end   (window-end)
+         isearch-lazy-highlight-windows
+         (if (and (boundp 'follow-mode) follow-mode)
+             (follow-all-followers)
+           (list (selected-window)))
+         isearch-lazy-highlight-windows-start
+         (isearch-windows-start isearch-lazy-highlight-windows)
+         isearch-lazy-highlight-windows-end
+         (isearch-windows-end isearch-lazy-highlight-windows)
          ;; Start lazy-highlighting at the beginning of the found
          ;; match (`isearch-other-end').  If no match, use point.
          ;; One of the next two variables (depending on search direction)
@@ -3048,11 +3102,11 @@
                       (min (or isearch-lazy-highlight-end-limit (point-max))
                            (if isearch-lazy-highlight-wrapped
                                isearch-lazy-highlight-start
-                             (window-end)))
+                             isearch-lazy-highlight-windows-end))
                     (max (or isearch-lazy-highlight-start-limit (point-min))
                          (if isearch-lazy-highlight-wrapped
                              isearch-lazy-highlight-end
-                           (window-start))))))
+                           isearch-lazy-highlight-windows-start)))))
        ;; Use a loop like in `isearch-search'.
        (while retry
          (setq success (isearch-search-string
@@ -3068,6 +3122,24 @@
        success)
     (error nil)))
 
+(defun isearch-lazy-highlight-put-overlays (mb me)
+  "Put a highlighting overlay on the buffer for each pertinent window.
+
+An overlay is put over the positions (MB ME) in each window in
+`isearch-lazy-highlight-windows' which (at least partially) contains them."
+  ;; non-zero-length match
+  (mapc (lambda (w)
+         (if (and (< mb (window-end w))
+                  (> me (window-start w)))
+             (let ((ov (make-overlay mb me)))
+               (push ov isearch-lazy-highlight-overlays)
+               ;; 1000 is higher than ediff's 100+,
+               ;; but lower than isearch main overlay's 1001
+               (overlay-put ov 'priority 1000)
+               (overlay-put ov 'face lazy-highlight-face)
+               (overlay-put ov 'window w))))
+       isearch-lazy-highlight-windows))
+
 (defun isearch-lazy-highlight-update ()
   "Update highlighting of other matches for current search."
   (let ((max lazy-highlight-max-at-a-time)
@@ -3096,23 +3168,15 @@
                          (if isearch-lazy-highlight-forward
                              (if (= mb (if isearch-lazy-highlight-wrapped
                                            isearch-lazy-highlight-start
-                                         (window-end)))
+                                         isearch-lazy-highlight-windows-end))
                                  (setq found nil)
                                (forward-char 1))
                            (if (= mb (if isearch-lazy-highlight-wrapped
                                          isearch-lazy-highlight-end
-                                       (window-start)))
+                                       isearch-lazy-highlight-windows-start))
                                (setq found nil)
                              (forward-char -1)))
-
-                       ;; non-zero-length match
-                       (let ((ov (make-overlay mb me)))
-                         (push ov isearch-lazy-highlight-overlays)
-                         ;; 1000 is higher than ediff's 100+,
-                         ;; but lower than isearch main overlay's 1001
-                         (overlay-put ov 'priority 1000)
-                         (overlay-put ov 'face lazy-highlight-face)
-                         (overlay-put ov 'window (selected-window))))
+                       (isearch-lazy-highlight-put-overlays mb me))
                      ;; Remember the current position of point for
                      ;; the next call of `isearch-lazy-highlight-update'
                      ;; when `lazy-highlight-max-at-a-time' is too small.
@@ -3128,12 +3192,12 @@
                      (setq isearch-lazy-highlight-wrapped t)
                      (if isearch-lazy-highlight-forward
                          (progn
-                           (setq isearch-lazy-highlight-end (window-start))
+                           (setq isearch-lazy-highlight-end 
isearch-lazy-highlight-windows-start)
                            (goto-char (max (or 
isearch-lazy-highlight-start-limit (point-min))
-                                           (window-start))))
-                       (setq isearch-lazy-highlight-start (window-end))
+                                           
isearch-lazy-highlight-windows-start)))
+                       (setq isearch-lazy-highlight-start 
isearch-lazy-highlight-windows-end)
                        (goto-char (min (or isearch-lazy-highlight-end-limit 
(point-max))
-                                       (window-end))))))))
+                                       
isearch-lazy-highlight-windows-end)))))))
            (unless nomore
              (setq isearch-lazy-highlight-timer
                    (run-at-time lazy-highlight-interval nil
@@ -3151,6 +3215,7 @@
   (setq isearch-string string
        isearch-message message
        isearch-case-fold-search case-fold)
+  (isearch-call-message nil t)
   (isearch-search)
   (isearch-update))
 



-- 
Alan Mackenzie (Nuremberg, Germany).