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

[Alan Mackenzie <acm@muc.de>]

Hello, Stefan.

On Sun, May 11, 2014 at 03:05:17PM -0400, Stefan Monnier wrote:
> >> It shouldn't be too difficult.  It's just a matter of refactoring:
> >> change your patch so that on isearch.el's side it only adds some hooks,
> >> which are then set follow-mode.el.
> > Do you mean "set to a function in follow-mode.el".  I think you're
> > suggesting writing more functions in follow-mode.el.  Do you mean
> > something like a Follow Mode equivalent of `window-start'?

> Right, I think introducing new functions/hooks to get the beginning/end of the
> visible part of the buffer, which can then be overridden by follow-mode
> would probably be part of the solution.

OK, but it really needs a window list as a parameter, and that needs to
be stored in a variable somewhere.

> > I know the motivation for the change.  For what functionalities should
> > code be come up with that will be incorporated into these hooks?  What
> > will the hooks do, specifically?

> I don't know.  You know the code better than I do, so hopefully you can
> come up with good ideas.

I don't really see opportunities for this whose costs don't outweigh
them.  We can make a variable called `window-start-function', but then
everybody has to start using

    (funcall window-start-function w)

which is a little more obfuscated and confusing than

    (window-start w)

just to save a (very) few calls of

    (window-start (car foo-windows))

.  The last form is typically going to be much faster when Follow Mode is
active (see below).

[ Snipped bit about pre-redisplay-function.  Thanks. ]

> > Is it for "bring-region-into-sight" and friends that you envisage adding
> > hooks?

> Yes.

> > I'm not sure how well a hook would work for this functionality, since in
> > the "plain" hook function you'd want to pass it a window, whereas in the
> > "follow" hook function you'd want to pass it a list of windows.

> I don't see why you'd need to pass anything like a window or a list
> of windows.  All it needs is a region and a point.  The window (or set
> thereof) would be passed implicitly via selected-window, as usual.

Because each time the Follow Mode window list would have to be
recalculated, and this is a moderately expensive calculation, involving
filtering and sorting the windows in a frame (`follow-all-followers').
This is no big deal once per command, but if done much more often will
start to drag.

[ .... ]

> Right, designing an API is often tricky.

:-)

> > The whole patch addresses the main bug; forming generally useful
> > subroutines out of isearch-string-out-of-window and friends is the other
> > bug, which I'm suggesting be dealt with separately.

> I don't want to add code specific to follow-mode directly in Isearch.

There is one place where I think it is unavoidable, and that is adding
lazy highlight overlays.  These are currently given a `window' property
to restrict their effect to the current window.  In my patch, a
particular match gets _two_ (or even several) overlays when it spans two
or more Follow Mode windows.  This can surely not be abstracted away in
any sensible way.

> So the way to fix the bug is in 2 steps:
> - refactor Isearch so that it adds the hooks we need (some of this,
>   may involve creating new functions in subr.el such as discussed above).
> - change follow-mode.el to set those hooks as appropriate.
> There's then a 3rd step which is to make other packages than Isearch use
> those same new functions/hooks, but that one can be postponed.


>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).