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

[Alan Mackenzie <acm@muc.de>]

Hello again, Artur.

On Mon, Nov 02, 2015 at 04:26:23PM +0000, Artur Malabarba wrote:
> Hi Alan,

> 2015-11-02 15:44 GMT+00:00 Alan Mackenzie <acm@muc.de>:
> > So how about us just moving all these checks to where they really
> > belong, in isearch-lazy-highlight-update?  I've a feeling that if we do
> > this, then your function follow--search-function becomes unneeded.

> Yes, I think this might work too. And I like the idea of eliminating a
> redisplay inside the command-loop.
> Could you test it?

Of course, nothing is ever that simple.  ;-).  I've got a working
version which doesn't use "(sit-for 0)", thus allowing point to move
properly over Follow Mode windows.

However, this meant the "old" highlighting not being erased until the
0.25 seconds had passed.  So I tried testing the conditions for a new
lazy-highlight loop (apart from window-start/end changing) in the
command loop bit.  This doesn't work brilliantly well when a C-s causes
a half-screen forward scroll: the "upper" lazy highlighting remains
there, and 0.25s later the "lower" lazy H. appears.

It struck me that instead of all this rigmarole, and instead of the
"(sit-for 0)", we could test whether or not a scroll is about to take
place.  The obvious candidate for this is `pos-visible-in-window-p' - if
this returns nil, obviously a scroll is about to happen.  Unfortunately,
even if p-v-i-w-p returns t, a scroll might still happen because of
`scroll-margin', and the like.

What we could do with is an interface which will tell us whether or not
redisplay would scroll if invoked immediately, with the current value of
point, window-start, etc.  Obviously it would be simple, but messy to
hack a lisp function together to do this, but a more general purpose
function might be apposite here.  Eli, what do you think?

All this is, just for the moment, disregarding any enhancements needed
to support Follow Mode properly.  Just for the moment, that is.

If you're interested, here is a (still scrappy) patch of where I am at
the moment.



diff --git a/lisp/isearch.el b/lisp/isearch.el
index b762884..b59a224 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -3012,13 +3012,52 @@ 'isearch-lazy-highlight-cleanup
                                 "22.1")
 
 (defun isearch-lazy-highlight-new-loop (&optional beg end)
-  "Cleanup any previous `lazy-highlight' loop and begin a new one.
-BEG and END specify the bounds within which highlighting should occur.
-This is called when `isearch-update' is invoked (which can cause the
-search string to change or the window to scroll).  It is also used
-by other Emacs features."
+  "Set an idle timer, which will trigger a new `lazy-highlight' loop.
+BEG and END specify the bounds within which highlighting should
+occur.  This is called when `isearch-update' is invoked (which
+can cause the search string to change or the window(s) to
+scroll).  It is also used by other Emacs features."
+  (setq isearch-lazy-highlight-start-limit beg
+        isearch-lazy-highlight-end-limit end)
+  (when (null executing-kbd-macro)
+    (when (or (not (equal isearch-string
+                          isearch-lazy-highlight-last-string))
+              (not (eq (selected-window)
+                       isearch-lazy-highlight-window))
+              (not (eq isearch-lazy-highlight-case-fold-search
+                       isearch-case-fold-search))
+              (not (eq isearch-lazy-highlight-regexp
+                       isearch-regexp))
+              (not (eq isearch-lazy-highlight-regexp-function
+                       isearch-regexp-function))
+              (not (eq isearch-lazy-highlight-lax-whitespace
+                       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 (eq isearch-forward
+                       isearch-lazy-highlight-forward))
+              ;; In case we are recovering from an error.
+              (not (equal isearch-error
+                          isearch-lazy-highlight-error)))
+      (lazy-highlight-cleanup t))    ;kill old loop & remove overlays.
+    (setq isearch-lazy-highlight-timer
+          (run-with-idle-timer lazy-highlight-initial-delay nil
+                               'isearch-lazy-highlight-maybe-new-loop))))
+
+(defun isearch-lazy-highlight-maybe-new-loop ()
+  "If needed, cleanup the previous `lazy-highlight' loop and begin a new one.
+Return t if we begin a loop, nil otherwise."
+;; "   Cleanup any previous `lazy-highlight' loop and begin a new one.
+;; BEG and END specify the bounds within which highlighting should occur.
+;; This is called when `isearch-update' is invoked (which can cause the
+;; 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
+             ;; (sit-for 0)         ;make sure (window-start) is credible
              (or (not (equal isearch-string
                              isearch-lazy-highlight-last-string))
                  (not (eq (selected-window)
@@ -3048,8 +3087,8 @@ isearch-lazy-highlight-new-loop
     ;; It used to check for `(not isearch-error)' here, but actually
     ;; lazy-highlighting might find matches to highlight even when
     ;; `isearch-error' is non-nil.  (Bug#9918)
-    (setq isearch-lazy-highlight-start-limit beg
-	  isearch-lazy-highlight-end-limit end)
+    ;; (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)
@@ -3070,10 +3109,12 @@ isearch-lazy-highlight-new-loop
 	  isearch-lazy-highlight-regexp-lax-whitespace isearch-regexp-lax-whitespace
 	  isearch-lazy-highlight-regexp-function  isearch-regexp-function
 	  isearch-lazy-highlight-forward      isearch-forward)
-      (unless (equal isearch-string "")
-	(setq isearch-lazy-highlight-timer
-	      (run-with-idle-timer lazy-highlight-initial-delay nil
-				   'isearch-lazy-highlight-update)))))
+      ;; (unless (equal isearch-string "")
+      ;;   (setq isearch-lazy-highlight-timer
+      ;;         (run-with-idle-timer lazy-highlight-initial-delay nil
+      ;;   			   'isearch-lazy-highlight-update)))
+    (unless (equal isearch-string "")
+      (isearch-lazy-highlight-update))))
 
 (defun isearch-lazy-highlight-search ()
   "Search ahead for the next or previous match, for lazy highlighting.



-- 
Alan Mackenzie (Nuremberg, Germany).