[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#17453: Isearch doesn't work properly with Follow Mode.
From: |
Alan Mackenzie |
Subject: |
bug#17453: Isearch doesn't work properly with Follow Mode. |
Date: |
Sun, 11 May 2014 20:40:17 +0000 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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).