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

[Artur Malabarba]

2015-11-04 9:01 GMT+00:00 Alan Mackenzie <acm@muc.de>:
>> It still might help to synchronise the windows from isearch-update-post-hook
>> if we'll call it before calling isearch-lazy-highlight-new-loop with sit-for.
>
> I still say, wait until we really need it before we do anything so
> drastic.  As Eli noted, follow-post-command-hook is SLOW, SLOW, SLOW.
> If we call it twice per command, it will be twice as slow.
>
> Also, why is the "(sit-for 0)" there at all?  As its comment says, It
> is there for one purpose, and one purpose only: it is so that
> (window-start) is valid, and the check
>
>     (not (= (window-start)
>             isearch-lazy-highlight-window-start))
>
> will work.  This check means exactly "has the window scrolled?"

Have you tested redisplay-would-scroll-window-p in a fully folded
org-mode buffer with 100 headlines where each headline has 100 lines
of content?

If you report that it works in this context and isn't slow then I'm ok
with going with this solution. Like I said, as long as it doesn't
cause regressions, this would be a fine refactoring to do in
isearch.el. So we might as well apply it now and give any possible
issues some time to surface (isearch is used by tons of people, so I'm
sure they'll surface if any exist).

While you're at it, if you could also refactor all those `(not (equal
...))' tests into a single `isearch--lazy-highlight-needs-update-p'
function that would be very welcome (and if you do, do it as a
separate commit before everything else so the other stuff is easy to
revert separately).

And if it looks like I'm saying "I'm fine with this" to everything
here, that's because I am. It sounds like we're debating two fine
options and bordering on bikeshedding. So I say: merge one and see how
it goes.


Cheers