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

[Alan Mackenzie]

Hello, Artur.

On Sun, Nov 01, 2015 at 12:23:52PM +0000, Artur Malabarba wrote:
> 2015-11-01 12:20 GMT+00:00 Artur Malabarba <address@hidden>:
> > You have a better understanding of the issue and the code than I do,
> > don't feel obliged to try my suggestion.

> Anyway, could we see this code? :-)
> Can you send it as patch or push it to a scratch/window-groups branch?

No problem!  The patch below doesn't actually apply at the moment, due
to some (?very) recent change in isearch.el.  But it wouldn't run
anyway, because window*-start and friends aren't there yet.



diff --git a/lisp/isearch.el b/lisp/isearch.el
index 4fc9b38..07ec534 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -184,6 +184,11 @@ isearch-message-function
   "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,
@@ -615,6 +620,7 @@ isearch-adjusted
 (defvar isearch-slow-terminal-mode nil)
 ;; If t, using a small window.
 (defvar isearch-small-window nil)
+(defvar isearch-suspended-follow-mode nil) ; only used with a small window.
 (defvar isearch-opoint 0)
 ;; The window configuration active at the beginning of the search.
 (defvar isearch-window-configuration nil)
@@ -885,6 +891,7 @@ isearch-mode
                                              (abs search-slow-window-lines))))
        isearch-other-end nil
        isearch-small-window nil
+        isearch-suspended-follow-mode nil
        isearch-just-started t
        isearch-start-hscroll (window-hscroll)
 
@@ -971,14 +978,15 @@ isearch-update
           (null executing-kbd-macro))
       (progn
         (if (not (input-pending-p))
-           (if isearch-message-function
-               (funcall isearch-message-function)
-             (isearch-message)))
+            (isearch-call-message))
         (if (and isearch-slow-terminal-mode
                  (not (or isearch-small-window
-                          (pos-visible-in-window-p))))
+                          (pos-visible-in-window*-p))))
             (let ((found-point (point)))
               (setq isearch-small-window t)
+              (when (and (boundp 'follow-mode) follow-mode)
+                (follow-mode 0)
+                (setq isearch-suspended-follow-mode t))
               (move-to-window-line 0)
               (let ((window-min-height 1))
                 (split-window nil (if (< search-slow-window-lines 0)
@@ -997,7 +1005,7 @@ isearch-update
          (let ((current-scroll (window-hscroll))
                visible-p)
            (set-window-hscroll (selected-window) isearch-start-hscroll)
-           (setq visible-p (pos-visible-in-window-p nil nil t))
+           (setq visible-p (pos-visible-in-window*-p nil nil t))
            (if (or (not visible-p)
                    ;; When point is not visible because of hscroll,
                    ;; pos-visible-in-window-p returns non-nil, but
@@ -1051,17 +1059,20 @@ isearch-done
   (setq minibuffer-message-timeout isearch-original-minibuffer-message-timeout)
   (isearch-dehighlight)
   (lazy-highlight-cleanup lazy-highlight-cleanup)
-  (let ((found-start (window-start))
+  (let ((found-start (window*-start))
        (found-point (point)))
     (when isearch-window-configuration
       (set-window-configuration isearch-window-configuration)
       (if isearch-small-window
-         (goto-char found-point)
+          (progn
+            (when isearch-suspended-follow-mode
+              (follow-mode 1))
+            (goto-char found-point))
        ;; set-window-configuration clobbers window-start; restore it.
        ;; This has an annoying side effect of clearing the last_modiff
        ;; field of the window, which can cause unwanted scrolling,
        ;; so don't do it unless truly necessary.
-       (set-window-start (selected-window) found-start t))))
+       (set-window*-start (selected-window) found-start t))))
 
   (setq isearch-mode nil)
   (if isearch-input-method-local-p
@@ -1284,13 +1295,6 @@ with-isearch-suspended
          (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
@@ -1303,7 +1307,17 @@ with-isearch-suspended
                  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))
@@ -1898,6 +1912,7 @@ isearch-del-char
                                            (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,
@@ -2074,6 +2089,7 @@ isearch-search-and-update
              (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
@@ -2234,16 +2250,23 @@ isearch-allow-prefix
   :type 'boolean
   :group 'isearch)
 
+;; (defmacro save-excursion-and-window (&rest body)
+;;   "FIXME!!!"
+;;   `(let ((-old-win- (selected-window)))
+;;      (save-excursion
+;;        (unwind-protect (progn ,@body)
+;;          (select-window -old-win- t)))))
+
 (defun isearch-string-out-of-window (isearch-point)
   "Test whether the search string is currently outside of the window.
 Return nil if it's completely visible, or if point is visible,
 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 (window*-start))
+        (w-end (window*-end nil t))
+        (w-L1 (save-selected-window (move-to-window*-line 1) (point)))
+        (w-L-1 (save-selected-window (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))
@@ -2270,15 +2293,15 @@ isearch-back-into-window
     (if above
         (progn
           (goto-char start)
-          (recenter 0)
-          (when (>= isearch-point (window-end nil t))
+          (recenter* 0)
+          (when (>= isearch-point (window*-end nil t))
             (goto-char isearch-point)
-            (recenter -1)))
+            (recenter* -1)))
       (goto-char end)
-      (recenter -1)
-      (when (< isearch-point (window-start))
+      (recenter* -1)
+      (when (< isearch-point (window*-start))
         (goto-char isearch-point)
-        (recenter 0))))
+        (recenter* 0))))
   (goto-char isearch-point))
 
 (defvar isearch-pre-scroll-point nil)
@@ -2425,6 +2448,7 @@ isearch-ring-adjust
   (isearch-ring-adjust1 advance)
   (if search-ring-update
       (progn
+       (isearch-call-message nil t)
        (isearch-search)
        (isearch-push-state)
        (isearch-update))
@@ -2497,6 +2521,13 @@ isearch-complete-edit
 
 (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)))
@@ -2679,9 +2710,6 @@ isearch-search-string
 
 (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)))
@@ -3015,11 +3043,13 @@ isearch-lazy-highlight-new-loop
 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 and windows are
+             ;; properly synchronised.
+             (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)
+                            (window*-windows isearch-lazy-highlight-window)))
                 (not (eq isearch-lazy-highlight-case-fold-search
                          isearch-case-fold-search))
                 (not (eq isearch-lazy-highlight-regexp
@@ -3030,9 +3060,9 @@ isearch-lazy-highlight-new-loop
                          isearch-lax-whitespace))
                 (not (eq isearch-lazy-highlight-regexp-lax-whitespace
                          isearch-regexp-lax-whitespace))
-                 (not (= (window-start)
+                 (not (= (window*-start)
                          isearch-lazy-highlight-window-start))
-                 (not (= (window-end)   ; Window may have been split/joined.
+                 (not (= (window*-end)   ; Window may have been split/joined.
                         isearch-lazy-highlight-window-end))
                 (not (eq isearch-forward
                          isearch-lazy-highlight-forward))
@@ -3048,8 +3078,8 @@ isearch-lazy-highlight-new-loop
     (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-window-start (window*-start)
+         isearch-lazy-highlight-window-end   (window*-end)
          ;; 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)
@@ -3093,13 +3123,13 @@ isearch-lazy-highlight-search
                                (+ isearch-lazy-highlight-start
                                   ;; Extend bound to match whole string at 
point
                                   (1- (length 
isearch-lazy-highlight-last-string)))
-                             (window-end)))
+                             (window*-end)))
                     (max (or isearch-lazy-highlight-start-limit (point-min))
                          (if isearch-lazy-highlight-wrapped
                              (- isearch-lazy-highlight-end
                                 ;; Extend bound to match whole string at point
                                 (1- (length 
isearch-lazy-highlight-last-string)))
-                           (window-start))))))
+                           (window*-start))))))
        ;; Use a loop like in `isearch-search'.
        (while retry
          (setq success (isearch-search-string
@@ -3115,6 +3145,24 @@ isearch-lazy-highlight-search
        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 the group
+of windows (e.g. in Follow Mode) 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))))
+       (window*-windows)))
+
 (defun isearch-lazy-highlight-update ()
   "Update highlighting of other matches for current search."
   (let ((max lazy-highlight-max-at-a-time)
@@ -3143,23 +3191,15 @@ isearch-lazy-highlight-update
                          (if isearch-lazy-highlight-forward
                              (if (= mb (if isearch-lazy-highlight-wrapped
                                            isearch-lazy-highlight-start
-                                         (window-end)))
+                                         (window*-end)))
                                  (setq found nil)
                                (forward-char 1))
                            (if (= mb (if isearch-lazy-highlight-wrapped
                                          isearch-lazy-highlight-end
-                                       (window-start)))
+                                       (window*-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.
@@ -3175,12 +3215,12 @@ isearch-lazy-highlight-update
                      (setq isearch-lazy-highlight-wrapped t)
                      (if isearch-lazy-highlight-forward
                          (progn
-                           (setq isearch-lazy-highlight-end (window-start))
+                           (setq isearch-lazy-highlight-end (window*-start))
                            (goto-char (max (or 
isearch-lazy-highlight-start-limit (point-min))
-                                           (window-start))))
-                       (setq isearch-lazy-highlight-start (window-end))
+                                           (window*-start))))
+                       (setq isearch-lazy-highlight-start (window*-end))
                        (goto-char (min (or isearch-lazy-highlight-end-limit 
(point-max))
-                                       (window-end))))))))
+                                       (window*-end))))))))
            (unless nomore
              (setq isearch-lazy-highlight-timer
                    (run-at-time lazy-highlight-interval nil
@@ -3198,6 +3238,7 @@ isearch-resume
   (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).