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

[Stefan Monnier]

>> follow-mode.el doesn't, indeed, but since you're moving some follow mode
>> code to isearch.el, ....
> Am I?  No code that currently is in follow-mode.el will cease to be in
> follow-mode.el.

Please replace "moving" with "adding" in my phrase, then.

> No, Follow Mode will know nothing about Isearch.  Isearch will know about
> follow-mode's internal structures, namely its list of windows, and what
> their sequencing means.  In that sense, the two libraries are coupled,
> which isn't, other things being equal, good, but I can't see how to
> avoid it.  Can you see a way of avoiding this coupling?

I gave one suggestion to uncouple them in one specific spot, yes.
There's no general answer for such problems, so we have to look at
every part of the coupling and try to find solutions for each part.

>> 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.

> 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 didn't know about pre-redisplay-function.  It looks new.  As an aside,
> its documentation is unclear (what is a "set" of windows, for example,

As is common in (Emacs) Lisp, sets are represented as a list.

> and what sort of things are allowed/not allowed in a hook function
> (deleting windows, for example)?)

Noone knows, really.

> 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.

> OK.  So you're thinking of somewhere like subr.el, with something like
>     (defun scroll-region-into-window/s (start end window/s above
>     opposite-important) ....)
> where WINDOW/S is either a single window or a list of them, START END
> bound the region, OPPOSITE-IMPORTANT is t when the region boundary
> "nearest the window" is to be preferred for display when the region's too
> big.  Or something like that.  Maybe we could add a parameter for desired
> margin at the top/bottom of the window.

Something like that, yes.

> OK.  Formulating a good description of that parameter is tricky, though.

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.
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