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

[Alan Mackenzie <acm@muc.de>]

Hello, Juri.

On Tue, Nov 03, 2015 at 01:22:03AM +0200, Juri Linkov wrote:
> > 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.
> >
> > Juri?

> Right, without (sit-for 0) it's possible to switch focus to adjacent windows
> with just adding 2 lines to follow.el, i.e. I get the desired behavior with:

> diff --git a/lisp/follow.el b/lisp/follow.el
> index 938c59e..0433854 100644
> --- a/lisp/follow.el
> +++ b/lisp/follow.el
> @@ -420,6 +420,7 @@ (define-minor-mode follow-mode
>    (if follow-mode
>        (progn
>  	(add-hook 'compilation-filter-hook 'follow-align-compilation-windows t t)
> +	(add-hook 'isearch-update-post-hook 'follow-align-compilation-windows t t)
>  	(add-hook 'post-command-hook 'follow-post-command-hook t)
>  	(add-hook 'window-size-change-functions 'follow-window-size-change t))
>      ;; Remove globally-installed hook functions only if there is no
> @@ -432,6 +433,7 @@ (define-minor-mode follow-mode
>        (unless following
>  	(remove-hook 'post-command-hook 'follow-post-command-hook)
>  	(remove-hook 'window-size-change-functions 'follow-window-size-change)))
> +    (remove-hook 'isearch-update-post-hook 'follow-align-compilation-windows t)
>      (remove-hook 'compilation-filter-hook 'follow-align-compilation-windows t)))
 
>  (defun follow-find-file-hook ()

This is complicated.  Ideally, the Follow Mode windows should be
synchronised in FM's post-command-hook, not isearch's.  It is not
isearch's job to realign windows.  follow-post-command-hook both realigns
windows and choses an appropriate window to put point in.  We should let
it.

> diff --git a/lisp/isearch.el b/lisp/isearch.el
> index b762884..8edf8b0 100644
> --- a/lisp/isearch.el
> +++ b/lisp/isearch.el
> @@ -3018,7 +3018,7 @@ (defun isearch-lazy-highlight-new-loop (&optional beg end)
>  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)


> So what remains to do is to fix this bug, but I don't understand the logic
> you proposed: how checks could be moved to isearch-lazy-highlight-update
> if isearch-lazy-highlight-update is scheduled by a timer conditionally
> depending on these checks?

What I'm proposing is to schedule the timer always, and do the checks
(for whether we need to start a new lazy highlight loop) in the function
that the timer triggers.  The advantage is that when the timer triggers,
redisplay will already have taken place[*], and thus we can use
`window-start' and `window-end' without forcing an artificial redisplay
with "(sit-for 0)".

[*] and an appropriate FM window will have been selected by FM's
post-command-hook

The disadvantage is that stale lazy highlights are erased after
isearch-lazy-highlight-initial-delay, not immediately, as at present.
I'm not sure if this is really a disadvantage or not.  In my scrappy
patch last night, I attempted to ameliorate this "problem" by repeating
(most of) the tests in the command-loop bit.

-- 
Alan Mackenzie (Nuremberg, Germany).