Framework extending window functions for Follow Mode (etc.).

Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Emacs.

Partly out of a need to nail down bug #17453, partly out of a need to
make it easier for arbitrary libraries to work with Follow Mode, partly
at the suggestion of Eli, I now propose the following.

The six functions window-start, window-end, set-window-start, recenter,
pos-visible-in-window-p, and move-to-window-line-function will acquire
an extra optional parameter GROUP.  The notion is that "this call is
interested in groups of windows, not just single ones".

Each of these functions will get an associated variable, e.g.
"window-start-function".  The function will call the value of that
variable instead of doing its normal actions, when GROUP is non-nil.
Typically, the `window-start-function' will recursively call
window-start (on some window) to perform its operation.

window-start-function, and friends, will be set by Follow Mode in its
initialisation, and unset when it is disabled.

Here is one sixth of the patch for this change (excluding changes to the
manual).  As a patch, it probably won't work because of all the
deletions.  (The whole of the patch would quickly become tedious.)



diff --git a/src/window.c b/src/window.c
index d61f586..46adda7 100644
--- a/src/window.c
+++ b/src/window.c
@@ -1655,30 +1671,39 @@ Return POS.  */)
   return pos;
 }
 
-DEFUN ("set-window-start", Fset_window_start, Sset_window_start, 2, 3, 0,
+DEFUN ("set-window-start", Fset_window_start, Sset_window_start, 2, 4, 0,
        doc: /* Make display in WINDOW start at position POS in WINDOW's buffer.
 WINDOW must be a live window and defaults to the selected one.  Return
 POS.  Optional third arg NOFORCE non-nil inhibits next redisplay from
-overriding motion of point in order to display at this exact start.  */)
-  (Lisp_Object window, Lisp_Object pos, Lisp_Object noforce)
+overriding motion of point in order to display at this exact start.
+
+If GROUP is non-nil, and `set-window-start-function' is set to a function,
+then instead of the above, that function is called with the three arguments
+WINDOW, POS, and NOFORCE, and its result returned.  */)
+  (Lisp_Object window, Lisp_Object pos, Lisp_Object noforce, Lisp_Object group)
 {
-  register struct window *w = decode_live_window (window);
+  if (!NILP (group)
+      && FUNCTIONP (Vset_window_start_function))
+    return call3 (Vset_window_start_function, window, pos, noforce);
+  {
+    register struct window *w = decode_live_window (window);
 
-  set_marker_restricted (w->start, pos, w->contents);
-  /* This is not right, but much easier than doing what is right.  */
-  w->start_at_line_beg = false;
-  if (NILP (noforce))
-    w->force_start = true;
-  w->update_mode_line = true;
-  /* Bug#15957.  */
-  w->window_end_valid = false;
-  wset_redisplay (w);
+    set_marker_restricted (w->start, pos, w->contents);
+    /* This is not right, but much easier than doing what is right.  */
+    w->start_at_line_beg = false;
+    if (NILP (noforce))
+      w->force_start = true;
+    w->update_mode_line = true;
+    /* Bug#15957.  */
+    w->window_end_valid = false;
+    wset_redisplay (w);
 
-  return pos;
+    return pos;
+  }
 }
 
 DEFUN ("pos-visible-in-window-p", Fpos_visible_in_window_p,
@@ -7159,7 +7212,15 @@ syms_of_window (void)
   DEFSYM (Qclone_of, "clone-of");
   DEFSYM (Qfloor, "floor");
   DEFSYM (Qceiling, "ceiling");
-
+  DEFSYM (Qwindow_start_function, "window-start-function");
+  DEFSYM (Qwindow_end_function, "window-end-function");
+  DEFSYM (Qset_window_start_function, "set-window-start-function");
+  DEFSYM (Qrecenter_function, "recenter-function");
+  DEFSYM (Qpos_visible_in_window_p_function, "pos-visible-in-window-p-function");
+  DEFSYM (Qmove_to_window_line_function, "move-to-window-line-function");
+  
   staticpro (&Vwindow_list);
 
   minibuf_selected_window = Qnil;
@@ -7330,6 +7391,70 @@ Note that this optimization can cause the portion of the buffer
+  DEFVAR_LISP ("set-window-start-function", Vset_window_start_function,
+               doc: /* The function to call for `set-window-start' when its GROUP parameter is non-nil.
+When this variable contains a function, and `set-window-start' is called
+with a non-nil GROUP parameter, the function is called instead of
+`set-window-start''s normal action.  `set-window-start' passes the function
+its three parameters WINDOW, POS, and NOFORCE.  The function may call
+`set-window-start' recursively.  */);
+  Vset_window_start_function = Qnil;
+  Fmake_variable_buffer_local (Qset_window_start_function);
+  Fput (Qset_window_start_function, Qpermanent_local, Qt);
+



-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[John Wiegley]

>>>>> Alan Mackenzie <acm@muc.de> writes:

> Each of these functions will get an associated variable, e.g.
> "window-start-function".  The function will call the value of that
> variable instead of doing its normal actions, when GROUP is non-nil.
> Typically, the `window-start-function' will recursively call
> window-start (on some window) to perform its operation.

This is unrelated to the material content of your patch, but I'd call this
window-start-group-function, since it's not really the "function that goes to
window start", but the "function that goes to window start when groups are to
be considered".

John

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, John.

On Thu, Nov 05, 2015 at 02:36:40PM -0500, John Wiegley wrote:
> >>>>> Alan Mackenzie <acm@muc.de> writes:

> > Each of these functions will get an associated variable, e.g.
> > "window-start-function".  The function will call the value of that
> > variable instead of doing its normal actions, when GROUP is non-nil.
> > Typically, the `window-start-function' will recursively call
> > window-start (on some window) to perform its operation.

> This is unrelated to the material content of your patch, but I'd call this
> window-start-group-function, since it's not really the "function that goes to
> window start", but the "function that goes to window start when groups are to
> be considered".

Thanks, that's an excellent idea.  It's so easy to miss things like that
when embroiled in something tedious.

> John

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Emacs.

On Thu, Nov 05, 2015 at 07:29:05PM +0000, Alan Mackenzie wrote:
> Partly out of a need to nail down bug #17453, partly out of a need to
> make it easier for arbitrary libraries to work with Follow Mode, partly
> at the suggestion of Eli, I now propose the following.

> The six functions window-start, window-end, set-window-start, recenter,
> pos-visible-in-window-p, and move-to-window-line-function will acquire
> an extra optional parameter GROUP.  The notion is that "this call is
> interested in groups of windows, not just single ones".

> Each of these functions will get an associated variable, e.g.
> "window-start-function".  The function will call the value of that
> variable instead of doing its normal actions, when GROUP is non-nil.
> Typically, the `window-start-function' will recursively call
> window-start (on some window) to perform its operation.

Now renamed to "window-start-group-function", at John's suggestion.

> window-start-function, and friends, will be set by Follow Mode in its
> initialisation, and unset when it is disabled.

I've now updated the requisite two pages in Elisp.

This suggestion doesn't seem to have sparked off too much debate.  Can I
take it that people are generally happy about it?  In that case, I will
commit the change.

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[John Wiegley]

>>>>> Alan Mackenzie <acm@muc.de> writes:

> This suggestion doesn't seem to have sparked off too much debate. Can I take
> it that people are generally happy about it? In that case, I will commit the
> change.

Hi Alan,

First, I want to say that I'm very sorry this change has generated so much
effort for you. This is highly suboptimal, since you could have spent that
time doing other things for Emacs.

Second, I don't know that your change is ready for master today, but I *am*
ready to exempt it from the feature freeze. So please keep it on a branch for
now, and we will continue the discussion. If we can reach a satisfying
implementation, it will be in 25.1.

Would you be willing, one last time, to summarize the impact (from a
documentation perspective) of this change for Emacs users and for Emacs Lisp
developers? I think the *content* of the change is without reproach -- making
Follow Mode more valuable -- it is simply the form of the change that we need
more consensus on.

Adding your proposal to the Emacs Wiki Proposals page
(http://www.emacswiki.org/emacs/Proposals) would be a good way to liberate the
current state of your work from this lengthy discussion, so I can look at it
again with fresh eyes.

With gratitude for your perseverance,
  John

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, John.

On Mon, Nov 09, 2015 at 02:30:09PM -0800, John Wiegley wrote:
> >>>>> Alan Mackenzie <acm@muc.de> writes:

> > This suggestion doesn't seem to have sparked off too much debate. Can I take
> > it that people are generally happy about it? In that case, I will commit the
> > change.

> Hi Alan,

> First, I want to say that I'm very sorry this change has generated so much
> effort for you. This is highly suboptimal, since you could have spent that
> time doing other things for Emacs.

Thanks.  I think that the way I've presented this change has been less
than optimal in some way.  If I were on the other side, I think I might
be as wary/concerned/sceptical about the change as other people
currently are.  I've had some tips on this aspect from Juri.  I'll see
if I can follow them a bit better in the future.

> Second, I don't know that your change is ready for master today, but I *am*
> ready to exempt it from the feature freeze. So please keep it on a branch for
> now, and we will continue the discussion. If we can reach a satisfying
> implementation, it will be in 25.1.

The change works.  I have also amended several elisp manual pages,
though these doc changes could use a review from other people.
Technically, I believe the change is ready.

> Would you be willing, one last time, to summarize the impact (from a
> documentation perspective) of this change for Emacs users and for Emacs Lisp
> developers? I think the *content* of the change is without reproach -- making
> Follow Mode more valuable -- it is simply the form of the change that we need
> more consensus on.

Oh, OK.  ;-)

> Adding your proposal to the Emacs Wiki Proposals page
> (http://www.emacswiki.org/emacs/Proposals) would be a good way to liberate the
> current state of your work from this lengthy discussion, so I can look at it
> again with fresh eyes.

> With gratitude for your perseverance,
>   John

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello again, John.

On Mon, Nov 09, 2015 at 02:30:09PM -0800, John Wiegley wrote:
> >>>>> Alan Mackenzie <acm@muc.de> writes:

> Would you be willing, one last time, to summarize the impact (from a
> documentation perspective) of this change for Emacs users and for Emacs Lisp
> developers? I think the *content* of the change is without reproach -- making
> Follow Mode more valuable -- it is simply the form of the change that we need
> more consensus on.

> Adding your proposal to the Emacs Wiki Proposals page
> (http://www.emacswiki.org/emacs/Proposals) would be a good way to liberate the
> current state of your work from this lengthy discussion, so I can look at it
> again with fresh eyes.

Done.

> With gratitude for your perseverance,
>   John

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[John Wiegley]

>>>>> Alan Mackenzie <acm@muc.de> writes:

>> Adding your proposal to the Emacs Wiki Proposals page
>> (http://www.emacswiki.org/emacs/Proposals) would be a good way to liberate the
>> current state of your work from this lengthy discussion, so I can look at it
>> again with fresh eyes.

> Done.

You are my hero today. :)

I will go through it and respond back, hopefully before day's end.

John

Re: Framework extending window functions for Follow Mode (etc.).

[John Wiegley]

>>>>> Alan Mackenzie <acm@muc.de> writes:

>> Adding your proposal to the Emacs Wiki Proposals page
>> (http://www.emacswiki.org/emacs/Proposals) would be a good way to liberate the
>> current state of your work from this lengthy discussion, so I can look at it
>> again with fresh eyes.

> Done.

Great proposal. I have an idea about this functionality I'd like to suggest.

Right now we have a set of functions with well-defined behavior. Under certain
conditions of use, those functions are too specific. You'd like to change the
functions' API so that some packages (follow-mode) can change what they mean,
and requiring other packages (isearch) to explicitly allow the override.

Maybe the problem is that we've turned object-orientation inside out. What we
really care about is not the set of functions, but the notion of what "current
window" means. It needs to be broader in scope when follow-mode is active, to
address the notion that multiple windows are being knit together, visually. It
has nothing to do with isearch, so ideally the change shouldn't affect it.

What if we just had `window-start-function', receiving the window as its
argument -- effectively having getter and setters on window objects. Could
follow-mode determine from the argument whether the window was involved in its
"group", and act accordingly? That would avoid API changes, is cheaper to do,
and no modes would have to change.

That is:

          window-start WIND
    calls (funcall window-start-function WIND)
    calls window-object-start WIND

So the current window-start is renamed to window-object-start (or some
internal sounding name), which is reached by indirecting through the variable
window-start-function.

I wonder, though, if such extensibility is worth the extra layer of
indirection. It feels a bit... special-cased and not very elegant, maybe? Do
we care enough about follow-mode to make a core change like this?

It could also be that we're looking too much at a specific solution, by
needing window-start to change its meaning in order to support correct
behavior by isearch. Is there a screencast anywhere, so I could see what
follow-mode with isearch looks like using your patch, and what it looks like
without the patch? Is anyone else using this code?

John

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, John.

It's getting late, but seeing as how we're nine hours' timezones apart,
I'd like to get this out tonight.

On Tue, Nov 10, 2015 at 02:29:18PM -0800, John Wiegley wrote:
> >>>>> Alan Mackenzie <acm@muc.de> writes:

> >> Adding your proposal to the Emacs Wiki Proposals page
> >> (http://www.emacswiki.org/emacs/Proposals) would be a good way to liberate the
> >> current state of your work from this lengthy discussion, so I can look at it
> >> again with fresh eyes.

> > Done.

> Great proposal. I have an idea about this functionality I'd like to suggest.

> Right now we have a set of functions with well-defined behavior. Under certain
> conditions of use, those functions are too specific. You'd like to change the
> functions' API so that some packages (follow-mode) can change what they mean,
> and requiring other packages (isearch) to explicitly allow the override.

> Maybe the problem is that we've turned object-orientation inside out. What we
> really care about is not the set of functions, but the notion of what "current
> window" means. It needs to be broader in scope when follow-mode is active, to
> address the notion that multiple windows are being knit together, visually. It
> has nothing to do with isearch, so ideally the change shouldn't affect it.

If only we lived in an ideal world.  ;-)

> What if we just had `window-start-function', receiving the window as its
> argument -- effectively having getter and setters on window objects. Could
> follow-mode determine from the argument whether the window was involved in its
> "group", and act accordingly? That would avoid API changes, is cheaper to do,
> and no modes would have to change.

Follow Mode could indeed determine this.  However, expecting everything
else just to work is being too optimistic.  The display engine and Follow
Mode fight each other, to some extent.  Follow Mode, for example,
continually monitors where point it, and thus which window it is
logically in.  It then puts point physically into that window, preventing
the display engine from messing everything up.  For example.

If things aren't done very carefully, display artefacts ruin the effect.
I had to diagnose and correct one of these in Isearch.  The cause was
point being in the wrong place at the wrong time.

Also, Follow Mode would need access to the raw `window-start', etc.,
since it has to manipulate each window individually, so this would
need some extension of the existing interface anyway.

> That is:

>           window-start WIND
>     calls (funcall window-start-function WIND)
>     calls window-object-start WIND

> So the current window-start is renamed to window-object-start (or some
> internal sounding name), which is reached by indirecting through the variable
> window-start-function.

> I wonder, though, if such extensibility is worth the extra layer of
> indirection. It feels a bit... special-cased and not very elegant, maybe? Do
> we care enough about follow-mode to make a core change like this?

Things like window-start are C primitives, and called a lot from other C
code.  They really have to stay basically as they are.

The abstraction created by Follow Mode is really rather fragile.  I do
not feel that trying to make such an automatic "object-like" solution
would be a good idea, though I haven't thought it through thoroughly,
yet.  (A lovely bit of linguistic contrast, no?)

Still, you've been giving me ideas.  The ideal place to implement Follow
Mode would be inside the display engine, mainly in C.  Then we could
truly treat a "multiple" window as a single object.  Such a "multiple"
window could, for example, have a single (vertical) scroll bar and
possibly a single mode line.  Although it wouldn't be all that easy to
implement, it probably wouldn't be as difficult as all that.  I'm
guessing it would take longer than Friday 13th November to finish,
though.

> It could also be that we're looking too much at a specific solution, by
> needing window-start to change its meaning in order to support correct
> behavior by isearch. Is there a screencast anywhere, so I could see what
> follow-mode with isearch looks like using your patch, and what it looks like
> without the patch? Is anyone else using this code?

No, but a screen shot wouldn't really be helpful.  It's behaviour rather
than appearance that matters here.  A video would be the best thing, if I
had the technology to make one.  Maybe the best thing would be for me to
make the patch available in some fashion so that you can try it out.  The
total size of the two patches is currently ~1700 lines (About 350 lines
for Isearch, and 1350 lines for the framework, including documentation
changes).  As yet, nobody else is using it.  The patch has not yet left
my PC (apart from backup copies).

> John

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[John Wiegley]

>>>>> Alan Mackenzie <acm@muc.de> writes:

> Still, you've been giving me ideas. The ideal place to implement Follow Mode
> would be inside the display engine, mainly in C. Then we could truly treat a
> "multiple" window as a single object. Such a "multiple" window could, for
> example, have a single (vertical) scroll bar and possibly a single mode
> line. Although it wouldn't be all that easy to implement, it probably
> wouldn't be as difficult as all that. I'm guessing it would take longer than
> Friday 13th November to finish, though.

These are some intriguing thoughts. The 25.1 release cycle may last a while,
so this could still land in a few weeks (beyond the freeze), out of respect
for your efforts to do this right. If it gets too late, though, it would be
better in 25.2 (which I'm hoping will follow just a few months later).

> No, but a screen shot wouldn't really be helpful. It's behaviour rather than
> appearance that matters here. A video would be the best thing, if I had the
> technology to make one. Maybe the best thing would be for me to make the
> patch available in some fashion so that you can try it out. The total size
> of the two patches is currently ~1700 lines (About 350 lines for Isearch,
> and 1350 lines for the framework, including documentation changes). As yet,
> nobody else is using it. The patch has not yet left my PC (apart from backup
> copies).

Would you be available via Firefox Hello some evening, so that you could share
you screen and give me a demo? I'd like to get a better feel for this than my
imagination. I'm available right now in fact. :)

John

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, John.

On Tue, Nov 10, 2015 at 04:34:13PM -0800, John Wiegley wrote:
> >>>>> Alan Mackenzie <acm@muc.de> writes:

> > Still, you've been giving me ideas. The ideal place to implement Follow Mode
> > would be inside the display engine, mainly in C. Then we could truly treat a
> > "multiple" window as a single object. Such a "multiple" window could, for
> > example, have a single (vertical) scroll bar and possibly a single mode
> > line. Although it wouldn't be all that easy to implement, it probably
> > wouldn't be as difficult as all that. I'm guessing it would take longer than
> > Friday 13th November to finish, though.

> These are some intriguing thoughts. The 25.1 release cycle may last a while,
> so this could still land in a few weeks (beyond the freeze), out of respect
> for your efforts to do this right. If it gets too late, though, it would be
> better in 25.2 (which I'm hoping will follow just a few months later).

Adapting the display engine to handle Follow Mode type functionality is
something that, perhaps, Eli could implement in a few days.  It would
take me many weeks, at best.  Even specifying precisely what it would
have to do is far from trivial.

> > No, but a screen shot wouldn't really be helpful. It's behaviour rather than
> > appearance that matters here. A video would be the best thing, if I had the
> > technology to make one. Maybe the best thing would be for me to make the
> > patch available in some fashion so that you can try it out. The total size
> > of the two patches is currently ~1700 lines (About 350 lines for Isearch,
> > and 1350 lines for the framework, including documentation changes). As yet,
> > nobody else is using it. The patch has not yet left my PC (apart from backup
> > copies).

> Would you be available via Firefox Hello some evening, so that you could share
> you screen and give me a demo? I'd like to get a better feel for this than my
> imagination. I'm available right now in fact. :)

I don't seem to have Hello in my Firefox, though I've got version 38.4.
Perhaps the folk at Gentoo stripped it out for some reasone, or I built
it with some "use flag" (sorry for the Gentoo jargon) not set.  Or
something.

I have finally uploaded the source to savannah, in branch
scratch/follow (assuming git hasn't fooled me about this).  Why don't
you try it out yourself?

Successively, in standard master and the scratch/follow branch, do this:
1. emacs -Q

2. (global-set-key [next] 'follow-scroll-up)
   (global-set-key [prior] 'follow-scroll-down)
   (setq isearch-allow-scroll t)

3. C-x C-f path/to/src/xdisp.c

4. C-x 3,  C-x 3,  C-x +,  M-x follow-mode  (if necessary, use the mouse
to make the frame wide).

5. C-s t e x t.  Pause between the letters, and note the lazy
highlighting.  It should extend over all 3 windows.  Play around with
various search texts.

6. (Still in C-s t e x t).  Repeatedly do C-s.  Point should move onto
the middle window, then the RH window, then (and only then) the whole
collection should scroll.

7. (Not in C-s): Move point some distance away from the beginning of
buffer.  Start a C-s, and manipulate things such that the current match
is in the middle window.  <PageUp> and <PageDown> should scroll the
current match BETWEEN WINDOWS.

8. Start a C-s near the bottom of the LH window.  Continually do M-s
C-e, until the highlighted search area is split between LH and middle
windows.  Everything should display properly at this point.  Continue
doing M-s C-e, until the echo area expands by one line.  Everything
should continue to display properly.

#########################################################################

9. (Inside the xdisp.c code area): Find a pair of braces not too far
apart, on the same window.  Delete the closing brace, then retype it.
Note how the cursor jumps to the matching opening brace.  Now scroll the
text such that the two braces are on neighbouring windows.  When you do
the same, instead of the momentary jump to the opening brace, you get a
message instead.

This is a bug, if a small one.  It is the sort of thing that can be
corrected trivially by the new framework; adding a 't' argument to the
invocation of a primitive is all it would take.


> John

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[John Wiegley]

>>>>> Alan Mackenzie <acm@muc.de> writes:

> Even specifying precisely what it would have to do is far from trivial.

It will still be very good to have a specification. If we try to support a
feature we can't properly define.. much hard.

> I don't seem to have Hello in my Firefox, though I've got version 38.4.
> Perhaps the folk at Gentoo stripped it out for some reasone, or I built it
> with some "use flag" (sorry for the Gentoo jargon) not set. Or something.

If you have anything else available, I use many means for conferencing.

> I have finally uploaded the source to savannah, in branch
> scratch/follow (assuming git hasn't fooled me about this).  Why don't
> you try it out yourself?

I would love to, but at the moment I'm having difficulties just building
master. If you could show me, then you won't be stuck on me.

John

Comms and building Emacs. [Re: Framework extending window functions for Follow Mode (etc.).]

[Alan Mackenzie]

Hello, John.

On Wed, Nov 11, 2015 at 08:50:41AM -0800, John Wiegley wrote:
> >>>>> Alan Mackenzie <acm@muc.de> writes:

> > Even specifying precisely what it would have to do is far from trivial.

> It will still be very good to have a specification. If we try to support a
> feature we can't properly define.. much hard.

It would be essential to have such a spec, even if we ended up having to
change the spec as we went along.

> > I don't seem to have Hello in my Firefox, though I've got version 38.4.
> > Perhaps the folk at Gentoo stripped it out for some reasone, or I built it
> > with some "use flag" (sorry for the Gentoo jargon) not set. Or something.

> If you have anything else available, I use many means for conferencing.

I've never done anything like that before.  Sorry if it has appeared
that I was just being awkward.  I do have a (probably working)
earphones/microphone combination, though.  I think I'll ask on the
Gentoo list if anybody knows what might have happened to my Firefox
Hello.

> > I have finally uploaded the source to savannah, in branch
> > scratch/follow (assuming git hasn't fooled me about this).  Why don't
> > you try it out yourself?

> I would love to, but at the moment I'm having difficulties just building
> master. If you could show me, then you won't be stuck on me.

Is the emphasis on "at the moment" or on the "I'm having difficulties"?
If the former, hopefully the person who committed the build-breaking
commit will fix it.

If you've been so preoccupied by the duties of maintainer that you
haven't yet got a build to succeed, here are some tips:

1. The process is described in INSTALL.REPO in our top level directory.
2. This process is basically:
  (i) From the top level directory ...
  (ii)  ./autogen.sh
  (iii) ./configure    # You can put options here, such as for
                       # debugging, or a host of other things.
  (iv) make -j5 bootstrap # This step is critical the first time you
                          # build a repository.  The "-j5" is for the
			  # number of make jobs you have running at a
			  # time.  Recommended: # of cores + 1.
  (v) make -j5            # For the second and subsequent times you
                          # build.
3. The executable "emacs", hard-linked to "emacs-25.0.50.n", ends up in
the src directory.  To run it, just type "src/emacs -Q" (or whatever).


> John

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Comms and building Emacs. [Re: Framework extending window functions for Follow Mode (etc.).]

[Alan Mackenzie]

Hello, John.

On Wed, Nov 11, 2015 at 05:15:48PM +0000, Alan Mackenzie wrote:

[ .... ]

> 1. The process is described in INSTALL.REPO in our top level directory.
> 2. This process is basically:
>   (i) From the top level directory ...
>   (ii)  ./autogen.sh
>   (iii) ./configure    # You can put options here, such as for
>                        # debugging, or a host of other things.
>   (iv) make -j5 bootstrap # This step is critical the first time you
>                           # build a repository.  The "-j5" is for the
> 			  # number of make jobs you have running at a
> 			  # time.  Recommended: # of cores + 1.

Correction:  The make bootstrap is not needed on a new repository.  I've
just tried it.  Apologies.

>   (v) make -j5            # For the second and subsequent times you
>                           # build.
> 3. The executable "emacs", hard-linked to "emacs-25.0.50.n", ends up in
> the src directory.  To run it, just type "src/emacs -Q" (or whatever).


> > John

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[martin rudalics]

 > The six functions window-start, window-end, set-window-start, recenter,
 > pos-visible-in-window-p, and move-to-window-line-function will acquire
 > an extra optional parameter GROUP.  The notion is that "this call is
 > interested in groups of windows, not just single ones".
 >
 > Each of these functions will get an associated variable, e.g.
 > "window-start-function".  The function will call the value of that
 > variable instead of doing its normal actions, when GROUP is non-nil.
 > Typically, the `window-start-function' will recursively call
 > window-start (on some window) to perform its operation.

Instead of an extra argument I would prefer to have these functions
check for the presence of a ‘window-start’, ‘window-end’, ... window
parameter just like ‘delete-window’ or ‘other-window’ do.

martin

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Martin.

On Sat, Nov 07, 2015 at 02:26:16PM +0100, martin rudalics wrote:
>  > The six functions window-start, window-end, set-window-start, recenter,
>  > pos-visible-in-window-p, and move-to-window-line-function will acquire
>  > an extra optional parameter GROUP.  The notion is that "this call is
>  > interested in groups of windows, not just single ones".

>  > Each of these functions will get an associated variable, e.g.
>  > "window-start-function".  The function will call the value of that
>  > variable instead of doing its normal actions, when GROUP is non-nil.
>  > Typically, the `window-start-function' will recursively call
>  > window-start (on some window) to perform its operation.

> Instead of an extra argument I would prefer to have these functions
> check for the presence of a ‘window-start’, ‘window-end’, ... window
> parameter just like ‘delete-window’ or ‘other-window’ do.

I don't think that would be the Right Thing (if I've understood it
properly, that is).  With Follow Mode active, which window start
window-start returns is primarily about the context of the window-start
invocation; it's not about some windows always doing one thing, some
always doing another.

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[martin rudalics]

 > I don't think that would be the Right Thing (if I've understood it
 > properly, that is).  With Follow Mode active, which window start
 > window-start returns is primarily about the context of the window-start
 > invocation; it's not about some windows always doing one thing, some
 > always doing another.

I don't understand why we would want to hardcode a GROUP argument.  When
we encounter a similar problem with foo-mode what are we supposed to do?
Use the GROUP argument?  Add a FOO argument?

martin

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Martin.

On Sat, Nov 07, 2015 at 04:18:18PM +0100, martin rudalics wrote:
>  > I don't think that would be the Right Thing (if I've understood it
>  > properly, that is).  With Follow Mode active, which window start
>  > window-start returns is primarily about the context of the window-start
>  > invocation; it's not about some windows always doing one thing, some
>  > always doing another.

> I don't understand why we would want to hardcode a GROUP argument.

It would appear to be a good way of solving the immediate problem, that
is, of being able to address a Follow Mode group of windows and a single
window in a unified fashion.  It thus avoids having explicitly to code
Follow Mode stuff into random .el (or .c) files.

The immediate use case is in Isearch, where currently Isearch doesn't
play well with Follow Mode.  See the stuff relating to bug #17453 in
debbugs.

> When we encounter a similar problem with foo-mode what are we supposed
> to do?  Use the GROUP argument?  Add a FOO argument?

Yes, I would say.  I'm guessing your concern is that such arguments on
some functions could proliferate, giving an unmanageable number of
them.  But what is the likelihood of this happening?  Or, in particular,
of it happening to these particular window handling functions?

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[martin rudalics]

 > Yes, I would say.  I'm guessing your concern is that such arguments on
 > some functions could proliferate, giving an unmanageable number of
 > them.  But what is the likelihood of this happening?  Or, in particular,
 > of it happening to these particular window handling functions?

I can't tell.  But adding a special argument to six primitives for
handling one particular case in the interaction between isearch and
follow mode already appears like a proliferation to me.

If the peculiar behavior is tied to isearch, then I have no idea why you
can't devise a function like ‘isearch-set-window-start’ and special code
the follow mode behavior there.

martin

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Martin.

On Sat, Nov 07, 2015 at 06:07:35PM +0100, martin rudalics wrote:
>  > Yes, I would say.  I'm guessing your concern is that such arguments on
>  > some functions could proliferate, giving an unmanageable number of
>  > them.  But what is the likelihood of this happening?  Or, in particular,
>  > of it happening to these particular window handling functions?

> I can't tell.  But adding a special argument to six primitives for
> handling one particular case in the interaction between isearch and
> follow mode already appears like a proliferation to me.

A bit about the history of bug #17453: I've presented three solutions so
far, and none has met with universal approval.

The first one, ~18 months ago, simply adapted isearch.el making direct
calls to Follow Mode's functions.  This was rejected by Stefan, who
asked for a more general solution, one that would enable other libraries
to access Follow Mode more easily.  In his own words,

> IOW we should try harder to come up with more general hooks.

What Stefan asked for is what you are criticising here.

The second attempt, a week or two ago, invented new functions with new
names to perform the functions of `window-start' etc., on either a
group of windows or a single window.  Eli criticised this, saying:

> Btw, I see no reason to introduce new functions.  Instead, we could
> have the original ones accept an additional optional argument.

The third attempt, yesterday/today, was in response to that comment from
Eli.

> If the peculiar behavior is tied to isearch, then I have no idea why you
> can't devise a function like ‘isearch-set-window-start’ and special code
> the follow mode behavior there.

Because the behaviour isn't to be restricted to isearch.  Also, the
facilities needed from Follow mode far exceed what could be sensibly
coded in such a function.  In any case, what you're suggesting is pretty
much my second solution attempt, which created six functions like
`set-window*-start', but restricted to Isearch.

There are currently up to 132 occurrences of set-window-start in the
lisp sources.  Some of these would likely be more useful if they took
into account FM, and called set-window-start with the GROUP argument
non-nil.  These latest changes of mine would allow any such library to
be quickly adapted for Follow Mode.

Also, were Follow Mode to be superseded at any time, then the interface
I'm proposing would not need adapting to FM's successor.

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[martin rudalics]

 > A bit about the history of bug #17453: I've presented three solutions so
 > far, and none has met with universal approval.
 >
 > The first one, ~18 months ago, simply adapted isearch.el making direct
 > calls to Follow Mode's functions.  This was rejected by Stefan, who
 > asked for a more general solution, one that would enable other libraries
 > to access Follow Mode more easily.

I don't think that other libraries should pay attention to Follow Mode.

 > In his own words,
 >
 >> IOW we should try harder to come up with more general hooks.
 >
 > What Stefan asked for is what you are criticising here.

Why do you think so?  You want to give ‘set-window-start’ an additional,
optional argument GROUP.  If it's non-nil, ‘set-window-start’ should
call the function stored in ‘set-window-start-function’.  Correct so
far?

If so, then how comes that this argument is called "GROUP"?  An argument
with such a name should be non-nil only in calls by ‘follow-mode’ or
other packages that know that WINDOW is part of some "group".
‘isearch-mode’ has no idea whether WINDOW is part of a group and should
not have to.  OTOH somebody might want ‘set-window-start’ to behave
specially even when WINDOW is not part of a group.

For me a general solution means that any mode that forms a group of
windows (like ‘follow-mode’) adds window parameters to all windows that
are part of that group.  And any function like ‘set-window-start’ that
should behave specially when the window it operates on is part of a
group of windows will have to inspect that parameter (and maybe other
parameters as well).

 > The second attempt, a week or two ago, invented new functions with new
 > names to perform the functions of `window-start' etc., on either a
 > group of windows or a single window.  Eli criticised this, saying:
 >
 >> Btw, I see no reason to introduce new functions.  Instead, we could
 >> have the original ones accept an additional optional argument.

Above I explained why IMO the new argument is not about "groups".

 > The third attempt, yesterday/today, was in response to that comment from
 > Eli.
 >
 >> If the peculiar behavior is tied to isearch, then I have no idea why you
 >> can't devise a function like ‘isearch-set-window-start’ and special code
 >> the follow mode behavior there.
 >
 > Because the behaviour isn't to be restricted to isearch.  Also, the
 > facilities needed from Follow mode far exceed what could be sensibly
 > coded in such a function.  In any case, what you're suggesting is pretty
 > much my second solution attempt, which created six functions like
 > `set-window*-start', but restricted to Isearch.

Why such a conclusion?  Window parameters are far more flexible than
arguments.

 > There are currently up to 132 occurrences of set-window-start in the
 > lisp sources.  Some of these would likely be more useful if they took
 > into account FM,

I fully agree here ...

 > and called set-window-start with the GROUP argument
 > non-nil.

... and fully disagree here.  All these calls should be completely
unaware of whether ‘follow-mode’ is active or not.  How
‘set-window-start’ behaves would be encapsulated in the special function
prescribed by the ‘set-window-start’ parameter.

 > These latest changes of mine would allow any such library to
 > be quickly adapted for Follow Mode.

There should be no need for a library to adapt to Follow Mode.

 > Also, were Follow Mode to be superseded at any time, then the interface
 > I'm proposing would not need adapting to FM's successor.

The interface you propose already needs adapting up to 132 libraries to
FM.

martin

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Martin.

On Sun, Nov 08, 2015 at 10:22:44AM +0100, martin rudalics wrote:
>  > A bit about the history of bug #17453: I've presented three solutions so
>  > far, and none has met with universal approval.

>  > The first one, ~18 months ago, simply adapted isearch.el making direct
>  > calls to Follow Mode's functions.  This was rejected by Stefan, who
>  > asked for a more general solution, one that would enable other libraries
>  > to access Follow Mode more easily.

> I don't think that other libraries should pay attention to Follow Mode.

>  > In his own words,

>  >> IOW we should try harder to come up with more general hooks.

>  > What Stefan asked for is what you are criticising here.

> Why do you think so?  You want to give ‘set-window-start’ an additional,
> optional argument GROUP.  If it's non-nil, ‘set-window-start’ should
> call the function stored in ‘set-window-start-function’.  Correct so
> far?

Well, it's now been renamed `set-window-start-group-function', but
basically, yes.  The caller would call `set-window-start' without having
to know whether FM is active or not.

> If so, then how comes that this argument is called "GROUP"?

The idea is that a general package, like isearch, can call

    (set-window-start win ws nil 'group)

and this will do the Right Thing regardless of whether Follow Mode (or
whatever) is active or not.  GROUP non-nil just says "work on the whole
group of windows if there is one".

> An argument with such a name should be non-nil only in calls by
> ‘follow-mode’ or other packages that know that WINDOW is part of some
> "group".

Absolutely the reverse.  See above.  Actually, most calls from Follow
Mode, which will be about rearranging the individual windows, will be
called with GROUP set to nil.

> ‘isearch-mode’ has no idea whether WINDOW is part of a group and should
> not have to.

By my proposal, it wouldn't have to.  But it would have to be aware of
the possibility.

> OTOH somebody might want ‘set-window-start’ to behave specially even
> when WINDOW is not part of a group.

This sounds a bit hypothetical.  Do you have an example of the sort of
thing this might be?

> For me a general solution means that any mode that forms a group of
> windows (like ‘follow-mode’) adds window parameters to all windows that
> are part of that group.  And any function like ‘set-window-start’ that
> should behave specially when the window it operates on is part of a
> group of windows will have to inspect that parameter (and maybe other
> parameters as well).

I think what you mean is that `set-window-start' would first test the
window-parameter, and if that is a function, would call the function
instead of doing set-window-start's normal stuff.

The problem I see with that is that when some code wants to set the
window start of an individual window, it's going to have to do something
like this:

    (let ((save-ws-param (window-parameter win 'set-window-start)))
      (set-window-parameter win 'window-start nil)
      (set-window-start win ws)
      (set-window-parameter win 'window-start save-ws-param))

, which is rather clumsy.  In fact, the situation almost calls for a
macro which would look something like:

    (with-window-parameters-set win '((set-window-start . nil))
      (set-window-start win ws))

, but even this isn't very pretty.

>  > The second attempt, a week or two ago, invented new functions with new
>  > names to perform the functions of `window-start' etc., on either a
>  > group of windows or a single window.  Eli criticised this, saying:

>  >> Btw, I see no reason to introduce new functions.  Instead, we could
>  >> have the original ones accept an additional optional argument.

> Above I explained why IMO the new argument is not about "groups".

I still think it is.

>  > The third attempt, yesterday/today, was in response to that comment from
>  > Eli.

>  >> If the peculiar behavior is tied to isearch, then I have no idea why you
>  >> can't devise a function like ‘isearch-set-window-start’ and special code
>  >> the follow mode behavior there.

>  > Because the behaviour isn't to be restricted to isearch.  Also, the
>  > facilities needed from Follow mode far exceed what could be sensibly
>  > coded in such a function.  In any case, what you're suggesting is pretty
>  > much my second solution attempt, which created six functions like
>  > `set-window*-start', but restricted to Isearch.

> Why such a conclusion?  Window parameters are far more flexible than
> arguments.

But the criterion as to whether a `set-window-start' call wants to
operate on an individual window or the group (if there is one) would be
attached to the window rather than to the call.  I don't think this is
right.

>  > There are currently up to 132 occurrences of set-window-start in the
>  > lisp sources.  Some of these would likely be more useful if they took
>  > into account FM,

> I fully agree here ...

>  > and called set-window-start with the GROUP argument
>  > non-nil.

> ... and fully disagree here.  All these calls should be completely
> unaware of whether ‘follow-mode’ is active or not.

They cannot be.  They are packages which do their own window
manipulation, and so will have to make up their minds whether a
particular `window-start' or `pos-visiblie-in-window-p' refers to an
individual window or to the group (if any).  This distinction is
essentially encapsulated in the package.  It is more convenient for a
package to set an optional parameter to nil or non-nil than to have to
"bind" a window parameter to nil.

> How ‘set-window-start’ behaves would be encapsulated in the special
> function prescribed by the ‘set-window-start’ parameter.

>  > These latest changes of mine would allow any such library to
>  > be quickly adapted for Follow Mode.

> There should be no need for a library to adapt to Follow Mode.

Distorting your meaning for a paragraph: Under my proposal, there is no
urgent need to update any package which uses `set-window-start' and
friends.  If it currently fails to work well with FM, it will continue to
work just the same until somebody amends it.  Your proposal is more
radical: we'd have to check each of the 124 uses of
`pos-visible-in-window-p' to make sure they actually should work on
entire window groups.  I would guess most of them will, but some won't.
It would, of course, also affect external code.

>  > Also, were Follow Mode to be superseded at any time, then the interface
>  > I'm proposing would not need adapting to FM's successor.

> The interface you propose already needs adapting up to 132 libraries to
> FM.

No.  There is no need, see above, even though it would be beneficial.
With your proposal, we'd need to check a lot of code.

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[martin rudalics]

 > Well, it's now been renamed `set-window-start-group-function', but
 > basically, yes.  The caller would call `set-window-start' without having
 > to know whether FM is active or not.
 >
 >> If so, then how comes that this argument is called "GROUP"?
 >
 > The idea is that a general package, like isearch, can call
 >
 >      (set-window-start win ws nil 'group)
 >
 > and this will do the Right Thing regardless of whether Follow Mode (or
 > whatever) is active or not.  GROUP non-nil just says "work on the whole
 > group of windows if there is one".

So the idea is that ‘follow-mode’ sets `set-window-start-group-function'
to say ‘follow-mode-set-window-start’ which is then called whenever
‘isearch-mode’ calls ‘set-window-start’.  Since you don't do this on a
per window basis, ‘follow-mode-set-window-start' can be also called for
windows that are not managed by ‘follow-mode’ probably in the hope that
‘follow-mode-set-window-start’ temporarily binds
‘set-window-start-group-function’ to nil around a nested call to
‘set-window-start’.  It strikes me as a bad idea that
‘follow-mode-set-window-start’ has to take care of windows that are not
in ‘follow-mode’.

And you preclude the simultaneous use of a second mode operating on a
disjoint group of windows maybe even on another frame - there's only one
global ‘set-window-start-group-function’ variable.  Your approach
doesn't scale.

 >> An argument with such a name should be non-nil only in calls by
 >> ‘follow-mode’ or other packages that know that WINDOW is part of some
 >> "group".
 >
 > Absolutely the reverse.  See above.  Actually, most calls from Follow
 > Mode, which will be about rearranging the individual windows, will be
 > called with GROUP set to nil.

Obviously so.  I consider this a drawback.

 >> ‘isearch-mode’ has no idea whether WINDOW is part of a group and should
 >> not have to.
 >
 > By my proposal, it wouldn't have to.  But it would have to be aware of
 > the possibility.

By my proposal it wouldn't even have to be aware of the possibility.

 >> OTOH somebody might want ‘set-window-start’ to behave specially even
 >> when WINDOW is not part of a group.
 >
 > This sounds a bit hypothetical.  Do you have an example of the sort of
 > thing this might be?

I can easily devise one: Suppose I want text-mode windows to always
start at the beginning of a paragraph and prog-mode windows at the
beginning of a defun provided it is within easy reach, say at most three
lines, from the position proposed by ‘set-window-start’.  Do we really
want to tell people that in such case they should use an argument called
"GROUPS"?

 > I think what you mean is that `set-window-start' would first test the
 > window-parameter, and if that is a function, would call the function
 > instead of doing set-window-start's normal stuff.

Just as in `delete-window', yes.

 > The problem I see with that is that when some code wants to set the
 > window start of an individual window, it's going to have to do something
 > like this:
 >
 >      (let ((save-ws-param (window-parameter win 'set-window-start)))
 >        (set-window-parameter win 'window-start nil)
 >        (set-window-start win ws)
 >        (set-window-parameter win 'window-start save-ws-param))
 >
 > , which is rather clumsy.  In fact, the situation almost calls for a
 > macro which would look something like:
 >
 >      (with-window-parameters-set win '((set-window-start . nil))
 >        (set-window-start win ws))
 >
 > , but even this isn't very pretty.

It would do

(let ((ignore-window-parameters t))
   (set-window-start win ws))

 >>   > The second attempt, a week or two ago, invented new functions with new
 >>   > names to perform the functions of `window-start' etc., on either a
 >>   > group of windows or a single window.  Eli criticised this, saying:
 >
 >>   >> Btw, I see no reason to introduce new functions.  Instead, we could
 >>   >> have the original ones accept an additional optional argument.
 >
 >> Above I explained why IMO the new argument is not about "groups".
 >
 > I still think it is.

I still think it is not.

 > But the criterion as to whether a `set-window-start' call wants to
 > operate on an individual window or the group (if there is one) would be
 > attached to the window rather than to the call.  I don't think this is
 > right.

Above I tried to explain why attaching the criterion to a call is
insufficient.

 > They cannot be.  They are packages which do their own window
 > manipulation, and so will have to make up their minds whether a
 > particular `window-start' or `pos-visiblie-in-window-p' refers to an
 > individual window or to the group (if any).  This distinction is
 > essentially encapsulated in the package.

Which package?

 > It is more convenient for a
 > package to set an optional parameter to nil or non-nil than to have to
 > "bind" a window parameter to nil.

I suppose the package you care about is ‘follow-mode’.  As far as
isearch and all other packages are concerned, it's obviously easier to
neither set a window parameter nor an optional argument.

 > Distorting your meaning for a paragraph: Under my proposal, there is no
 > urgent need to update any package which uses `set-window-start' and
 > friends.  If it currently fails to work well with FM, it will continue to
 > work just the same until somebody amends it.  Your proposal is more
 > radical: we'd have to check each of the 124 uses of
 > `pos-visible-in-window-p' to make sure they actually should work on
 > entire window groups.  I would guess most of them will, but some won't.
 > It would, of course, also affect external code.

‘follow-mode-set-window-start’ would have to take care of that.  If you
really care about this have ‘follow-mode-set-window-start’ check a
variable like ‘isearch-mode’ to make sure that it affects isearch calls
only.

 >>   > Also, were Follow Mode to be superseded at any time, then the interface
 >>   > I'm proposing would not need adapting to FM's successor.
 >
 >> The interface you propose already needs adapting up to 132 libraries to
 >> FM.
 >
 > No.  There is no need, see above, even though it would be beneficial.
 > With your proposal, we'd need to check a lot of code.

If we changed up to 132 arguments in calls of ‘set-window-start’ and
decided that ‘follow-mode’ is obsolete we'd have up to 132 calls to
change back.

martin

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Martin.

On Sun, Nov 08, 2015 at 07:10:51PM +0100, martin rudalics wrote:
>  > Well, it's now been renamed `set-window-start-group-function', but
>  > basically, yes.  The caller would call `set-window-start' without having
>  > to know whether FM is active or not.

>  >> If so, then how comes that this argument is called "GROUP"?

>  > The idea is that a general package, like isearch, can call

>  >      (set-window-start win ws nil 'group)

>  > and this will do the Right Thing regardless of whether Follow Mode (or
>  > whatever) is active or not.  GROUP non-nil just says "work on the whole
>  > group of windows if there is one".

> So the idea is that ‘follow-mode’ sets `set-window-start-group-function'
> to say ‘follow-mode-set-window-start’ which is then called whenever
> ‘isearch-mode’ calls ‘set-window-start’.  Since you don't do this on a
> per window basis, ‘follow-mode-set-window-start' can be also called for
> windows that are not managed by ‘follow-mode’ ....

No.  `set-window-start-group-function' is buffer local.  The state of
being managed by Follow Mode is also a buffer local thing - in any given
Emacs session, either all windows displaying foo.el or none of them have
Follow Mode active.  If they are in different frames, FM manages them as
disjoint groups.

> .... probably in the hope that ‘follow-mode-set-window-start’
> temporarily binds ‘set-window-start-group-function’ to nil around a
> nested call to ‘set-window-start’.

The recursive call to `set-window-start' has its GROUP parameter nil.
There is no need for such temporary binding.

> It strikes me as a bad idea that ‘follow-mode-set-window-start’ has to
> take care of windows that are not in ‘follow-mode’.

It doesn't, because of the buffer localness of `s-w-s-g-f', combined with
the all-or-nothingness of Follow Mode on a buffer.

> And you preclude the simultaneous use of a second mode operating on a
> disjoint group of windows maybe even on another frame - there's only one
> global ‘set-window-start-group-function’ variable.  Your approach
> doesn't scale.

If the disjoint group is displaying a different buffer, there is no
problem, the xx-group-function's all being buffer local.  If it's the
same buffer - well, it sounds like rather a hypothetical case to me.  It
would involve the set-window-start-group-function having to chose which
of the two schemes it would use, depending on some flag, frame parameter,
or what have you.  Nothing would be precluded.

>  >> An argument with such a name should be non-nil only in calls by
>  >> ‘follow-mode’ or other packages that know that WINDOW is part of some
>  >> "group".

>  > Absolutely the reverse.  See above.  Actually, most calls from Follow
>  > Mode, which will be about rearranging the individual windows, will be
>  > called with GROUP set to nil.

> Obviously so.  I consider this a drawback.

If things were to be implemented with a window parameters as you suggest,
this would add a lot to the complexity of Follow Mode, since each call to
one of the window primitives would have to be surrounded by code
nullifying a window parameter.  Adding GROUP optional arguments is, by
comparison, trivial and non-intrusive.  I've done it.

>  >> ‘isearch-mode’ has no idea whether WINDOW is part of a group and should
>  >> not have to.

>  > By my proposal, it wouldn't have to.  But it would have to be aware of
>  > the possibility.

> By my proposal it wouldn't even have to be aware of the possibility.

It would have to be.  For a hypothetical example, say there's some minor
mode which keeps something or other in a buffer at least n lines away
from a window boundary.  Its maintainer would need to decide whether that
just means n lines from the top/bottom of the entire FM group, or from
the "internal" boundaries too.

>  >> OTOH somebody might want ‘set-window-start’ to behave specially even
>  >> when WINDOW is not part of a group.

>  > This sounds a bit hypothetical.  Do you have an example of the sort of
>  > thing this might be?

> I can easily devise one: Suppose I want text-mode windows to always
> start at the beginning of a paragraph and prog-mode windows at the
> beginning of a defun provided it is within easy reach, say at most three
> lines, from the position proposed by ‘set-window-start’.  Do we really
> want to tell people that in such case they should use an argument called
> "GROUPS"?

How would you program that now, without the GROUP facility?  That's how
you'd still do it with GROUP in existence.  To use the
xx-group-function's for this sort of thing would be an abuse of them.

[ .... ]

>  > The problem I see with that is that when some code wants to set the
>  > window start of an individual window, it's going to have to do something
>  > like this:

>  >      (let ((save-ws-param (window-parameter win 'set-window-start)))
>  >        (set-window-parameter win 'window-start nil)
>  >        (set-window-start win ws)
>  >        (set-window-parameter win 'window-start save-ws-param))

>  > , which is rather clumsy.  In fact, the situation almost calls for a
>  > macro which would look something like:

>  >      (with-window-parameters-set win '((set-window-start . nil))
>  >        (set-window-start win ws))

>  > , but even this isn't very pretty.

> It would do

> (let ((ignore-window-parameters t))
>    (set-window-start win ws))

That's assuming that _all_ window parameters have to be ignored at once.
But even that pair of lines is over twice as bulky as simply writing

    (set-windows-start win ws)

.

I see a further problem with the window parameters approach.  When a
different buffer becomes displayed in such a window, the parameter would
stay set.  There is a fundamental mismatch here: Follow Modishness is
associated with a buffer (not a set of windows); window parameters are
fundamentally attached to windows.  This clash is bound to cause
awkwardnesses.  So, to cope with temporary buffer changes, one would have
to build whole complexes of mechanisms to temporarily delete the window
parameters, and so on.

[ .... ]

>  > But the criterion as to whether a `set-window-start' call wants to
>  > operate on an individual window or the group (if there is one) would be
>  > attached to the window rather than to the call.  I don't think this is
>  > right.

> Above I tried to explain why attaching the criterion to a call is
> insufficient.

It may or may not be sufficient, but it is necessary.  I'm trying to
imagine things if `set-window-start''s NOFORCE argument had instead been
implemented as a window parameter.  It would make programming windows
that bit more clumsy than it is at the moment.  I can't see at all where
the GROUP argument is any different.

>  > They cannot be.  They are packages which do their own window
>  > manipulation, and so will have to make up their minds whether a
>  > particular `window-start' or `pos-visiblie-in-window-p' refers to an
>  > individual window or to the group (if any).  This distinction is
>  > essentially encapsulated in the package.

> Which package?

Edebug, for example.  It gathers things like `window-start's, so that it
can rebuild a window layout when switching between modes.  Or something.
Here, these calls clearly call for GROUP to be nil.  If the window
parameter approach were chosen, edebug.el would need modifying.  As
would, potentially, lots of other packages.

>  > It is more convenient for a
>  > package to set an optional parameter to nil or non-nil than to have to
>  > "bind" a window parameter to nil.

> I suppose the package you care about is ‘follow-mode’.  As far as
> isearch and all other packages are concerned, it's obviously easier to
> neither set a window parameter nor an optional argument.

Yes, I care about Follow Mode.  And I care about Isearch, a great deal.
The two currently don't work together.  Something like an optional
argument or a window parameter is absolutely needed to make them work
together.

>  > Distorting your meaning for a paragraph: Under my proposal, there is no
>  > urgent need to update any package which uses `set-window-start' and
>  > friends.  If it currently fails to work well with FM, it will continue to
>  > work just the same until somebody amends it.  Your proposal is more
>  > radical: we'd have to check each of the 124 uses of
>  > `pos-visible-in-window-p' to make sure they actually should work on
>  > entire window groups.  I would guess most of them will, but some won't.
>  > It would, of course, also affect external code.

> ‘follow-mode-set-window-start’ would have to take care of that.

It can't do, any more than `set-window-start' could take care of an
implicit NOFORCE parameter.

> If you really care about this have ‘follow-mode-set-window-start’ check
> a variable like ‘isearch-mode’ to make sure that it affects isearch
> calls only.

I take it that's a bit of sarcasm?

>  >>   > Also, were Follow Mode to be superseded at any time, then the interface
>  >>   > I'm proposing would not need adapting to FM's successor.

>  >> The interface you propose already needs adapting up to 132 libraries to
>  >> FM.

>  > No.  There is no need, see above, even though it would be beneficial.
>  > With your proposal, we'd need to check a lot of code.

> If we changed up to 132 arguments in calls of ‘set-window-start’ and
> decided that ‘follow-mode’ is obsolete we'd have up to 132 calls to
> change back.

Not at all.  The interface would remain unchanged, only the setting of
`set-window-start-group-function' would need to change.

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[martin rudalics]

 > I see a further problem with the window parameters approach.  When a
 > different buffer becomes displayed in such a window, the parameter would
 > stay set.  There is a fundamental mismatch here: Follow Modishness is
 > associated with a buffer (not a set of windows); window parameters are
 > fundamentally attached to windows.  This clash is bound to cause
 > awkwardnesses.

One of the worst things I've ever seen in the windows code was the
mixing in of buffer-local variables, the most notorious representative
of them being ‘window-size-fixed’.  I meanwhile found a workaround so it
shouldn't be needed any more.  Still that variable is a pain.

 > So, to cope with temporary buffer changes, one would have
 > to build whole complexes of mechanisms to temporarily delete the window
 > parameters, and so on.

All this would be restricted to Follow Mode without affecting anyone
else.

 >> If you really care about this have ‘follow-mode-set-window-start’ check
 >> a variable like ‘isearch-mode’ to make sure that it affects isearch
 >> calls only.
 >
 > I take it that's a bit of sarcasm?

Sorry.  It wasn't meant that way.

I think I have bothered you long enough with my objections.  I consider
it most unfortunate that your previous solutions were not accepted.  I
could have easily lived with all of them.  But I neither use isearch nor
follow mode so I'm probably not in a good position to judge.  The
current solution introduces a new approach which IMHO doesn't fit with
the rest of the windows code but let's live with it.

So thanks for putting so much care into your responses and good luck (no
sarcasm) with the current scheme.

martin

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Martin.

On Mon, Nov 09, 2015 at 09:25:52AM +0100, martin rudalics wrote:
>  > I see a further problem with the window parameters approach.  When a
>  > different buffer becomes displayed in such a window, the parameter would
>  > stay set.  There is a fundamental mismatch here: Follow Modishness is
>  > associated with a buffer (not a set of windows); window parameters are
>  > fundamentally attached to windows.  This clash is bound to cause
>  > awkwardnesses.

> One of the worst things I've ever seen in the windows code was the
> mixing in of buffer-local variables, the most notorious representative
> of them being ‘window-size-fixed’.  I meanwhile found a workaround so it
> shouldn't be needed any more.  Still that variable is a pain.

Presumably a lot of the pain came from the fact that that variable is
attached to a buffer, rather than a window (as it should have been).

> I think I have bothered you long enough with my objections.  I consider
> it most unfortunate that your previous solutions were not accepted.  I
> could have easily lived with all of them.  But I neither use isearch nor
> follow mode so I'm probably not in a good position to judge.  The
> current solution introduces a new approach which IMHO doesn't fit with
> the rest of the windows code but let's live with it.

> So thanks for putting so much care into your responses and good luck (no
> sarcasm) with the current scheme.

Thank you for jousting with me!  I learnt something (the existence of
window parameters).

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[Artur Malabarba]

[-- Attachment #1: Type: text/plain, Size: 813 bytes --]

On 5 Nov 2015 7:29 pm, "Alan Mackenzie" <acm@muc.de> wrote:
>
> Hello, Emacs.

Hi Alan,

> Partly out of a need to nail down bug #17453, partly out of a need to
> make it easier for arbitrary libraries to work with Follow Mode, partly
> at the suggestion of Eli, I now propose the following.
>
> The six functions window-start, window-end, set-window-start, recenter,
> pos-visible-in-window-p, and move-to-window-line-function will acquire
> an extra optional parameter GROUP.  The notion is that "this call is
> interested in groups of windows, not just single ones".

I admit I am confused. I thought we had narrowed down three possible
solutions to this issue (which you just listed over at the bug thread), all
of which are simpler than this code.
Is this functionality here still necessary to fix that bug?

[-- Attachment #2: Type: text/html, Size: 1030 bytes --]

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Artur.

On Sat, Nov 07, 2015 at 05:54:20PM +0000, Artur Malabarba wrote:
> On 5 Nov 2015 7:29 pm, "Alan Mackenzie" <acm@muc.de> wrote:
> Hi Alan,

> > Partly out of a need to nail down bug #17453, partly out of a need to
> > make it easier for arbitrary libraries to work with Follow Mode, partly
> > at the suggestion of Eli, I now propose the following.

> > The six functions window-start, window-end, set-window-start, recenter,
> > pos-visible-in-window-p, and move-to-window-line-function will acquire
> > an extra optional parameter GROUP.  The notion is that "this call is
> > interested in groups of windows, not just single ones".

> I admit I am confused. I thought we had narrowed down three possible
> solutions to this issue (which you just listed over at the bug thread), all
> of which are simpler than this code.

Those solutions are to merely one part of the bug, namely C-s wrongly
scrolling a window instead of moving onto the next one.

The other parts of #17453 are:
2: lazy highlighting is confined to one Follow Mode window (I'm a bit
  confused as to the status of this, though);
3: With isearch-allow-scroll enabled, it is not possible to scroll point
  to the next or previous Follow Mode window;
, in addition to which I have a fix for ...
4: With point near the bottom of a Follow Mode window, start an Isearch,
  and repeatedly do M-s C-e, until the highlighted match continues on to
  the next window.  Continue doing M-s C-e until the string in the
  minibuffer expands by a line.  At this point the top of the RH window
  gets spuriously scrolled into the middle of the window, leaving the FM
  windows unsynchronised.

> Is this functionality here still necessary to fix that bug?

It needed to fix items 2, 3, and 4.

Sorry for causing this confusion.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[Juri Linkov]

>> I admit I am confused. I thought we had narrowed down three possible
>> solutions to this issue (which you just listed over at the bug thread), all
>> of which are simpler than this code.
>
> Those solutions are to merely one part of the bug, namely C-s wrongly
> scrolling a window instead of moving onto the next one.
>
> The other parts of #17453 are:
> 2: lazy highlighting is confined to one Follow Mode window (I'm a bit
>   confused as to the status of this, though);

This problem is already solved by enabling lazy-highlighting of the whole
follow-mode buffer, but I postponed installing that patch to not create merge
conflicts with your work in the same functions.

> 3: With isearch-allow-scroll enabled, it is not possible to scroll point
>   to the next or previous Follow Mode window;
> , in addition to which I have a fix for ...

That's because isearch-post-command-hook uses isearch-pre-scroll-point
to move it back, so it's possible to nullify isearch-pre-scroll-point
in follow-mode, but I see that it leaves the highlighted found string
at its old position because the isearch-allow-scroll feature doesn't support
finding the next search hit after scrolling, or something like this
that would make sense.  IOW, this is a limitation of isearch-allow-scroll.

> 4: With point near the bottom of a Follow Mode window, start an Isearch,
>   and repeatedly do M-s C-e, until the highlighted match continues on to
>   the next window.  Continue doing M-s C-e until the string in the
>   minibuffer expands by a line.  At this point the top of the RH window
>   gets spuriously scrolled into the middle of the window, leaving the FM
>   windows unsynchronised.

I see the same behavior even without Isearch.

Honestly, I see here nothing that requires a new multi-window framework.

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Juri.

On Sat, Nov 07, 2015 at 11:58:29PM +0200, Juri Linkov wrote:
> >> I admit I am confused. I thought we had narrowed down three possible
> >> solutions to this issue (which you just listed over at the bug thread), all
> >> of which are simpler than this code.

> > Those solutions are to merely one part of the bug, namely C-s wrongly
> > scrolling a window instead of moving onto the next one.

> > The other parts of #17453 are:
> > 2: lazy highlighting is confined to one Follow Mode window (I'm a bit
> >   confused as to the status of this, though);

> This problem is already solved by enabling lazy-highlighting of the whole
> follow-mode buffer, but I postponed installing that patch to not create merge
> conflicts with your work in the same functions.

My solution is to lazily highlight just the part visible in windows (but
in all pertinent windows).  Isn't highlighting all of it going to be a
bit slow on a large buffer?

> > 3: With isearch-allow-scroll enabled, it is not possible to scroll point
> >   to the next or previous Follow Mode window;
> > , in addition to which I have a fix for ...

> That's because isearch-post-command-hook uses isearch-pre-scroll-point
> to move it back, so it's possible to nullify isearch-pre-scroll-point
> in follow-mode, but I see that it leaves the highlighted found string
> at its old position because the isearch-allow-scroll feature doesn't support
> finding the next search hit after scrolling, or something like this
> that would make sense.  IOW, this is a limitation of isearch-allow-scroll.

In my personal copy of Emacs, I've had the isearch scrolling working
properly with Follow Mode for ~18 months.  It was me that wrote the
isearch scrolling code in the first place, back in 2003.  Part 3 of the
bug is most assuredly NOT an intrinsic limitation of
isearch-allow-scroll.  It's caused by the variables w-L1 and w-L-1, the
bounds for the permissible scrolling range in
isearch-string-out-of-window, being set to the top and botom of the
_single_ window.  When these variables are set to the top of bottom of
the entire FM group of windows, the bug is solved.  This requires my new
framework, or something like it.

> > 4: With point near the bottom of a Follow Mode window, start an Isearch,
> >   and repeatedly do M-s C-e, until the highlighted match continues on to
> >   the next window.  Continue doing M-s C-e until the string in the
> >   minibuffer expands by a line.  At this point the top of the RH window
> >   gets spuriously scrolled into the middle of the window, leaving the FM
> >   windows unsynchronised.

> I see the same behavior even without Isearch.

The buggy behaviour I've described is explicitly and essentially an
Isearch scenario.  What do you mean by "the same behavior" and what
sequence of commands generates it?

The solution to problem 4 involves making sure point is at the right
place at the right time when invoking isearch-message.  I have made
changes to fix it.

> Honestly, I see here nothing that requires a new multi-window framework.

I want these bugs to be solved.  What, then, are the alternatives to
this framework (or something like it)?  Isearch needs information about
the Follow Mode windows, or it can't work properly.

So far, I've written three solutions for these bugs, as I outlined at
length in an email to Martin R. today.  The first of these solutions was
(justifiably) rejected by Stefan because it was a quick and dirty fix,
and he explicitly requested the new framework that I have now built.
Eli didn't like the second attempt and explicitly suggested the way for
my third attempt.  You and Martin dislike this most recent third
attempt.

It seems to me I've spent more time discussing this bug on the bug list
and emacs-devel, and reformulating the fix, than actually tracking down
and fixing the bugs in the first place.  At the moment I feel like I'm
trying to hack down a wall of constant negativity.  I don't recall
anybody else saying positively they want this bug fixed, and I certainly
don't feel I've had much encouragement wrt this bug, in the last few
days and weeks.

I see Follow Mode as being a critically important component of Emacs,
the more so since very wide (240 characters and more) screens displaced
the fairly narrow CRT monitors.  I would like every Emacs user to be
able to use FM as easily as I can.  Right at the moment there is no
suitable interface to FM for libraries that require to do their own
window manipulation.  Such an interface is what Stefan wanted, and it's
what I want, too.  As of yet, there's been practically no discussion of
this interface I've written, beyond Eli suggesting the current version
and John suggesting a name change for some hooks.

So, where do we go from here?  I would like these bugs fixed for 25.1.

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: Framework extending window functions for Follow Mode (etc.).

[martin rudalics]

 > I want these bugs to be solved.  What, then, are the alternatives to
 > this framework (or something like it)?  Isearch needs information about
 > the Follow Mode windows, or it can't work properly.

Disagreed.  But _maybe_ Isearch may have to accept the fact that a
function like ‘set-window-start’ did not behave as expected.  Hopefully,
the substitute did something reasonable instead.

 > So far, I've written three solutions for these bugs, as I outlined at
 > length in an email to Martin R. today.  The first of these solutions was
 > (justifiably) rejected by Stefan because it was a quick and dirty fix,
 > and he explicitly requested the new framework that I have now built.
 > Eli didn't like the second attempt and explicitly suggested the way for
 > my third attempt.  You and Martin dislike this most recent third
 > attempt.
 >
 > It seems to me I've spent more time discussing this bug on the bug list
 > and emacs-devel, and reformulating the fix, than actually tracking down
 > and fixing the bugs in the first place.  At the moment I feel like I'm
 > trying to hack down a wall of constant negativity.  I don't recall
 > anybody else saying positively they want this bug fixed, and I certainly
 > don't feel I've had much encouragement wrt this bug, in the last few
 > days and weeks.

I want this bug fixed and would like to give you all the encouragement
you need.  But you rejected the use of window parameters without telling
me why they are inappropriate in this particular case.

 > I see Follow Mode as being a critically important component of Emacs,
 > the more so since very wide (240 characters and more) screens displaced
 > the fairly narrow CRT monitors.  I would like every Emacs user to be
 > able to use FM as easily as I can.  Right at the moment there is no
 > suitable interface to FM for libraries that require to do their own
 > window manipulation.  Such an interface is what Stefan wanted, and it's
 > what I want, too.

I don't want such an interface.  Other libraries should never have to be
aware of Follow Mode.

 > As of yet, there's been practically no discussion of
 > this interface I've written, beyond Eli suggesting the current version
 > and John suggesting a name change for some hooks.
 >
 > So, where do we go from here?  I would like these bugs fixed for 25.1.

Me too.

martin

Re: Framework extending window functions for Follow Mode (etc.).

[Juri Linkov]

>> >> I admit I am confused. I thought we had narrowed down three possible
>> >> solutions to this issue (which you just listed over at the bug thread), all
>> >> of which are simpler than this code.
>
>> > Those solutions are to merely one part of the bug, namely C-s wrongly
>> > scrolling a window instead of moving onto the next one.
>
>> > The other parts of #17453 are:
>> > 2: lazy highlighting is confined to one Follow Mode window (I'm a bit
>> >   confused as to the status of this, though);
>
>> This problem is already solved by enabling lazy-highlighting of the whole
>> follow-mode buffer, but I postponed installing that patch to not create merge
>> conflicts with your work in the same functions.
>
> My solution is to lazily highlight just the part visible in windows (but
> in all pertinent windows).  Isn't highlighting all of it going to be a
> bit slow on a large buffer?

It shouldn't be slow with a sufficiently low value of lazy-highlight-max-at-a-time.

>> > 3: With isearch-allow-scroll enabled, it is not possible to scroll point
>> >   to the next or previous Follow Mode window;
>> > , in addition to which I have a fix for ...
>
>> That's because isearch-post-command-hook uses isearch-pre-scroll-point
>> to move it back, so it's possible to nullify isearch-pre-scroll-point
>> in follow-mode, but I see that it leaves the highlighted found string
>> at its old position because the isearch-allow-scroll feature doesn't support
>> finding the next search hit after scrolling, or something like this
>> that would make sense.  IOW, this is a limitation of isearch-allow-scroll.
>
> In my personal copy of Emacs, I've had the isearch scrolling working
> properly with Follow Mode for ~18 months.  It was me that wrote the
> isearch scrolling code in the first place, back in 2003.

Yes, I remember, and appreciate your efforts to develop it further.

> Part 3 of the bug is most assuredly NOT an intrinsic limitation of
> isearch-allow-scroll.  It's caused by the variables w-L1 and w-L-1, the
> bounds for the permissible scrolling range in
> isearch-string-out-of-window, being set to the top and botom of the
> _single_ window.  When these variables are set to the top of bottom of
> the entire FM group of windows, the bug is solved.  This requires my new
> framework, or something like it.

I tried to not use isearch-string-out-of-window/isearch-back-into-window
at all, but I can't get a useful behavior in such situation of scrolling
out of the window with the current search hit.  Could you show how you see
it should work in this case in follow-mode?

>> > 4: With point near the bottom of a Follow Mode window, start an Isearch,
>> >   and repeatedly do M-s C-e, until the highlighted match continues on to
>> >   the next window.  Continue doing M-s C-e until the string in the
>> >   minibuffer expands by a line.  At this point the top of the RH window
>> >   gets spuriously scrolled into the middle of the window, leaving the FM
>> >   windows unsynchronised.
>
>> I see the same behavior even without Isearch.
>
> The buggy behaviour I've described is explicitly and essentially an
> Isearch scenario.  What do you mean by "the same behavior" and what
> sequence of commands generates it?
>
> The solution to problem 4 involves making sure point is at the right
> place at the right time when invoking isearch-message.  I have made
> changes to fix it.

I meant such things as 'M-: (message "x\ny\nz") RET' causes jumping too.

>> Honestly, I see here nothing that requires a new multi-window framework.
>
> I want these bugs to be solved.  What, then, are the alternatives to
> this framework (or something like it)?  Isearch needs information about
> the Follow Mode windows, or it can't work properly.
>
> So far, I've written three solutions for these bugs, as I outlined at
> length in an email to Martin R. today.  The first of these solutions was
> (justifiably) rejected by Stefan because it was a quick and dirty fix,
> and he explicitly requested the new framework that I have now built.
> Eli didn't like the second attempt and explicitly suggested the way for
> my third attempt.  You and Martin dislike this most recent third
> attempt.
>
> It seems to me I've spent more time discussing this bug on the bug list
> and emacs-devel, and reformulating the fix, than actually tracking down
> and fixing the bugs in the first place.  At the moment I feel like I'm
> trying to hack down a wall of constant negativity.  I don't recall
> anybody else saying positively they want this bug fixed, and I certainly
> don't feel I've had much encouragement wrt this bug, in the last few
> days and weeks.
>
> I see Follow Mode as being a critically important component of Emacs,
> the more so since very wide (240 characters and more) screens displaced
> the fairly narrow CRT monitors.  I would like every Emacs user to be
> able to use FM as easily as I can.  Right at the moment there is no
> suitable interface to FM for libraries that require to do their own
> window manipulation.  Such an interface is what Stefan wanted, and it's
> what I want, too.  As of yet, there's been practically no discussion of
> this interface I've written, beyond Eli suggesting the current version
> and John suggesting a name change for some hooks.
>
> So, where do we go from here?  I would like these bugs fixed for 25.1.

The problem is that you are trying to design a new framework,
but not demonstrated yet how it would be useful generally in cases
other than a specific combination of isearch/follow-mode.

Re: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Juri.

On Mon, Nov 09, 2015 at 02:50:04AM +0200, Juri Linkov wrote:
> >> This problem is already solved by enabling lazy-highlighting of the whole
> >> follow-mode buffer, but I postponed installing that patch to not create merge
> >> conflicts with your work in the same functions.

> > My solution is to lazily highlight just the part visible in windows (but
> > in all pertinent windows).  Isn't highlighting all of it going to be a
> > bit slow on a large buffer?

> It shouldn't be slow with a sufficiently low value of lazy-highlight-max-at-a-time.

OK, I see it: it would be slow if it had to go to completion, but any
key/mouse/other event will terminate it.  So the only place it might be a
problem is somewhere where CPU cycles are at a premium.

> > Part 3 of the bug is most assuredly NOT an intrinsic limitation of
> > isearch-allow-scroll.  It's caused by the variables w-L1 and w-L-1, the
> > bounds for the permissible scrolling range in
> > isearch-string-out-of-window, being set to the top and botom of the
> > _single_ window.  When these variables are set to the top of bottom of
> > the entire FM group of windows, the bug is solved.  This requires my new
> > framework, or something like it.

> I tried to not use isearch-string-out-of-window/isearch-back-into-window
> at all, but I can't get a useful behavior in such situation of scrolling
> out of the window with the current search hit.  Could you show how you see
> it should work in this case in follow-mode?

To start with, set

  (global-set-key [next] 'follow-scroll-up)
  (global-set-key [prior] 'follow-scroll-down)
  (setq isearch-allow-scroll t)

.  Then start an Isearch not too close to the start of a buffer with
Follow Mode enabled with at least two windows.  Type something to get a
search match highlighted.  Now <PageUp> and <PageDown> should scroll that
match between Follow Mode windows, the boundaries of that scrolling being
the top of the LH window and the bottom of the RH window.

To make this work properly, the four variables in
isearch-string-out-of-window, w-start, w-end, w-L1, w-L-1, are set to the
positions in the entire group of windows, by setting the proposed
&optional argument GROUP to t in the calls to certain window functions,
e.g.

     (let ((w-start (window-start nil t))
                                      ^

> >> > 4: With point near the bottom of a Follow Mode window, start an Isearch,
> >> >   and repeatedly do M-s C-e, until the highlighted match continues on to
> >> >   the next window.  Continue doing M-s C-e until the string in the
> >> >   minibuffer expands by a line.  At this point the top of the RH window
> >> >   gets spuriously scrolled into the middle of the window, leaving the FM
> >> >   windows unsynchronised.

> >> I see the same behavior even without Isearch.

> > The buggy behaviour I've described is explicitly and essentially an
> > Isearch scenario.  What do you mean by "the same behavior" and what
> > sequence of commands generates it?

> > The solution to problem 4 involves making sure point is at the right
> > place at the right time when invoking isearch-message.  I have made
> > changes to fix it.

> I meant such things as 'M-: (message "x\ny\nz") RET' causes jumping too.

It causes some scrolling of windows to keep them properly aligned when
the echo area expands to three lines.  This is expected and proper.
However, when the echo area goes back to one line, the windows don't
scroll back again, leaving them unaligned.  I raised a bug for this this
morning.  It seems it is a known problem which is tricky to solve.

> >> Honestly, I see here nothing that requires a new multi-window framework.

> > I want these bugs to be solved.  What, then, are the alternatives to
> > this framework (or something like it)?  Isearch needs information about
> > the Follow Mode windows, or it can't work properly.

[ .... ]

> > So, where do we go from here?  I would like these bugs fixed for 25.1.

> The problem is that you are trying to design a new framework,
> but not demonstrated yet how it would be useful generally in cases
> other than a specific combination of isearch/follow-mode.

Any elisp software which manipulates windows will be able to work better
by considering entire FM groups.  I did some grepping: here are some
examples which the new framework could improve:

`kill-ring-save' should blink the cursor on the matching other-end, if
that is visible.  With FM active, and other end being in a different
window, currently it wrongly outputs a message instead.

`window-screen-lines' could be enhanced (optionally) to return the number
of screen lines in the entire group.

`blink-matching-open' could be enhanced to handle FM windows properly.

`ns-scroll-bar-move' (probably) should `set-window-start' for the entire
window group.  This (might) avoid scrolling such that the RH follow
window is displaying empty space.


One point about this new framework is that it is "harmless" in the sense
that anything which worked (or failed to work) prior to it will continue
unchanged unless specifically amended.  It's disadvantage is that it adds
a little to the bulk of things people need to know or to look up in
manuals.

I think Follow Mode should become easier to use, both for users and for
hackers, especially given that wide screens (>200 characters) are now the
norm rather than the exception.  At the moment Follow Mode is
ridiculously difficult to start (manually splitting windows, doing M-x
follow-mode) and even more difficult to hack for - just what interfaces
are currently available for making a package work with FM?  Answer: none
at all.  This new framework is intended to go some way to closing the
gap.

The alternative to a framework is a bugfix which is specific to Isearch.
Even this would require some sort of interface definition and
abstraction.  At a minimum, for "part 3" of the bug, Isearch would need
somehow to obtain the bounds of the group of windows.

The last alternative is a quick and dirty fix where Isearch would just
call Follow Mode functions.  I don't think anybody really wants this.

Would it help if I actually made the source available?  If so, where?  I
don't really think it would be appropriate to dump a patch of this size
on emacs-devel, and the time to commit the changes to master has clearly
not yet arrived.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Juri Linkov]

>> I tried to not use isearch-string-out-of-window/isearch-back-into-window
>> at all, but I can't get a useful behavior in such situation of scrolling
>> out of the window with the current search hit.  Could you show how you see
>> it should work in this case in follow-mode?
>
> To start with, set
>
>   (global-set-key [next] 'follow-scroll-up)
>   (global-set-key [prior] 'follow-scroll-down)
>   (setq isearch-allow-scroll t)
>
> .  Then start an Isearch not too close to the start of a buffer with
> Follow Mode enabled with at least two windows.  Type something to get a
> search match highlighted.  Now <PageUp> and <PageDown> should scroll that
> match between Follow Mode windows, the boundaries of that scrolling being
> the top of the LH window and the bottom of the RH window.
>
> To make this work properly, the four variables in
> isearch-string-out-of-window, w-start, w-end, w-L1, w-L-1, are set to the
> positions in the entire group of windows, by setting the proposed
> &optional argument GROUP to t in the calls to certain window functions,
> e.g.
>
>      (let ((w-start (window-start nil t))
>                                       ^

Could you provide the shortest patch to test the behavior that you describe?
For now I tried the following, is this what you want to generalise with
a new framework?

diff --git a/lisp/isearch.el b/lisp/isearch.el
index b762884..3b61505 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -2237,10 +2237,19 @@ (defun isearch-string-out-of-window (isearch-point)
 together with as much of the search string as will fit; the symbol
 `above' if we need to scroll the text downwards; the symbol `below',
 if upwards."
-  (let ((w-start (window-start))
-        (w-end (window-end nil t))
-        (w-L1 (save-excursion (move-to-window-line 1) (point)))
-        (w-L-1 (save-excursion (move-to-window-line -1) (point)))
+  (let ((w-start (window-start (and (fboundp 'follow-all-followers)
+                                    (car (follow-all-followers)))))
+        (w-end (window-end (and (fboundp 'follow-all-followers)
+                                (car (last (follow-all-followers))))
+                           t))
+        (w-L1 (save-excursion
+                (when (fboundp 'follow-all-followers)
+                  (select-window (car (follow-all-followers))))
+                (move-to-window-line 1) (point)))
+        (w-L-1 (save-excursion
+                 (when (fboundp 'follow-all-followers)
+                   (select-window (car (last (follow-all-followers)))))
+                 (move-to-window-line -1) (point)))
         start end)                  ; start and end of search string in buffer
     (if isearch-forward
         (setq end isearch-point  start (or isearch-other-end isearch-point))

>> The problem is that you are trying to design a new framework,
>> but not demonstrated yet how it would be useful generally in cases
>> other than a specific combination of isearch/follow-mode.
>
> Any elisp software which manipulates windows will be able to work better
> by considering entire FM groups.  I did some grepping: here are some
> examples which the new framework could improve:
>
> `kill-ring-save' should blink the cursor on the matching other-end, if
> that is visible.  With FM active, and other end being in a different
> window, currently it wrongly outputs a message instead.
>
> `window-screen-lines' could be enhanced (optionally) to return the number
> of screen lines in the entire group.
>
> `blink-matching-open' could be enhanced to handle FM windows properly.
>
> `ns-scroll-bar-move' (probably) should `set-window-start' for the entire
> window group.  This (might) avoid scrolling such that the RH follow
> window is displaying empty space.
>
>
> One point about this new framework is that it is "harmless" in the sense
> that anything which worked (or failed to work) prior to it will continue
> unchanged unless specifically amended.  It's disadvantage is that it adds
> a little to the bulk of things people need to know or to look up in
> manuals.
>
> I think Follow Mode should become easier to use, both for users and for
> hackers, especially given that wide screens (>200 characters) are now the
> norm rather than the exception.  At the moment Follow Mode is
> ridiculously difficult to start (manually splitting windows, doing M-x
> follow-mode) and even more difficult to hack for - just what interfaces
> are currently available for making a package work with FM?  Answer: none
> at all.  This new framework is intended to go some way to closing the
> gap.
>
> The alternative to a framework is a bugfix which is specific to Isearch.
> Even this would require some sort of interface definition and
> abstraction.  At a minimum, for "part 3" of the bug, Isearch would need
> somehow to obtain the bounds of the group of windows.
>
> The last alternative is a quick and dirty fix where Isearch would just
> call Follow Mode functions.  I don't think anybody really wants this.
>
> Would it help if I actually made the source available?  If so, where?  I
> don't really think it would be appropriate to dump a patch of this size
> on emacs-devel, and the time to commit the changes to master has clearly
> not yet arrived.

You are trying to do everything at once.  To successfully achieve your goals
it would be much more clear for us to see the progress step by step, i.e.
if at first you demonstrated how to fix the co-operation between Isearch and
Follow Mode by a quick and dirty fix like in the patch above then we could see
how well your fixes work, and also what places need generalisation,
and how your new framework would be useful here and for other packages
that might benefit from it.  By such inductive method we could quickly
arrive to a conclusion without much friction.

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Juri.

On Tue, Nov 10, 2015 at 02:51:41AM +0200, Juri Linkov wrote:
> >> I tried to not use isearch-string-out-of-window/isearch-back-into-window
> >> at all, but I can't get a useful behavior in such situation of scrolling
> >> out of the window with the current search hit.  Could you show how you see
> >> it should work in this case in follow-mode?

> > To start with, set

> >   (global-set-key [next] 'follow-scroll-up)
> >   (global-set-key [prior] 'follow-scroll-down)
> >   (setq isearch-allow-scroll t)

> > .  Then start an Isearch not too close to the start of a buffer with
> > Follow Mode enabled with at least two windows.  Type something to get a
> > search match highlighted.  Now <PageUp> and <PageDown> should scroll that
> > match between Follow Mode windows, the boundaries of that scrolling being
> > the top of the LH window and the bottom of the RH window.

> > To make this work properly, the four variables in
> > isearch-string-out-of-window, w-start, w-end, w-L1, w-L-1, are set to the
> > positions in the entire group of windows, by setting the proposed
> > &optional argument GROUP to t in the calls to certain window functions,
> > e.g.

> >      (let ((w-start (window-start nil t))
> >                                       ^

> Could you provide the shortest patch to test the behavior that you describe?

Can I ask you here to look at the initial patch in the archive for bug
#17453 (see below)?

> For now I tried the following, is this what you want to generalise with
> a new framework?

More or less, yes.

> diff --git a/lisp/isearch.el b/lisp/isearch.el
> index b762884..3b61505 100644
> --- a/lisp/isearch.el
> +++ b/lisp/isearch.el
> @@ -2237,10 +2237,19 @@ (defun isearch-string-out-of-window (isearch-point)
>  together with as much of the search string as will fit; the symbol
>  `above' if we need to scroll the text downwards; the symbol `below',
>  if upwards."
> -  (let ((w-start (window-start))
> -        (w-end (window-end nil t))
> -        (w-L1 (save-excursion (move-to-window-line 1) (point)))
> -        (w-L-1 (save-excursion (move-to-window-line -1) (point)))
> +  (let ((w-start (window-start (and (fboundp 'follow-all-followers)
> +                                    (car (follow-all-followers)))))
> +        (w-end (window-end (and (fboundp 'follow-all-followers)
> +                                (car (last (follow-all-followers))))
> +                           t))
> +        (w-L1 (save-excursion
> +                (when (fboundp 'follow-all-followers)
> +                  (select-window (car (follow-all-followers))))
> +                (move-to-window-line 1) (point)))
> +        (w-L-1 (save-excursion
> +                 (when (fboundp 'follow-all-followers)
> +                   (select-window (car (last (follow-all-followers)))))
> +                 (move-to-window-line -1) (point)))
>          start end)                  ; start and end of search string in buffer
>      (if isearch-forward
>          (setq end isearch-point  start (or isearch-other-end isearch-point))

As a small point, I think you'd want a `save-selected-window' around the
forms which bind w-L1 and w-L-1.

[ .... ]

> > The last alternative is a quick and dirty fix where Isearch would just
> > call Follow Mode functions.  I don't think anybody really wants this.

> > Would it help if I actually made the source available?  If so, where?  I
> > don't really think it would be appropriate to dump a patch of this size
> > on emacs-devel, and the time to commit the changes to master has clearly
> > not yet arrived.

> You are trying to do everything at once.  To successfully achieve your goals
> it would be much more clear for us to see the progress step by step, i.e.
> if at first you demonstrated how to fix the co-operation between Isearch and
> Follow Mode by a quick and dirty fix like in the patch above then we could see
> how well your fixes work, and also what places need generalisation,
> and how your new framework would be useful here and for other packages
> that might benefit from it.

I posted the quick and dirty fix on 2014-05-09 in the opening post for
bug #17453 (the bug which is still current and has around 55 posts).
This post is, naturally, still available on http://debbugs.gnu.org.  I
was encouraged by Stefan instead to formulate the change as a more
general framework, removing the direct access to follow-mode.el from
isearch.el.  I first posted a description of the framework on 2015-10-29,
in the bug #17453 thread.  This was criticised by Eli, and I amended it
substantially in response.

> By such inductive method we could quickly arrive to a conclusion
> without much friction.

I posted the essence of the framework, as it now is, in a (cut down)
patch at the beginning of this thread.  I have complete patches for both
the framework and isearch.el available.  Together, they are really too
big to post on emacs-devel.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Juri Linkov]

> Can I ask you here to look at the initial patch in the archive for bug
> #17453 (see below)?

Thanks, I see.

> I posted the essence of the framework, as it now is, in a (cut down)
> patch at the beginning of this thread.  I have complete patches for both
> the framework and isearch.el available.  Together, they are really too
> big to post on emacs-devel.

Could you put them on a branch then?

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Juri.

On Wed, Nov 11, 2015 at 02:12:17AM +0200, Juri Linkov wrote:
> > Can I ask you here to look at the initial patch in the archive for bug
> > #17453 (see below)?

> Thanks, I see.

> > I posted the essence of the framework, as it now is, in a (cut down)
> > patch at the beginning of this thread.  I have complete patches for both
> > the framework and isearch.el available.  Together, they are really too
> > big to post on emacs-devel.

> Could you put them on a branch then?

I've created the branch scratch/follow in savannah, with both the new
framework and the proposed changes to isearch.el.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Juri Linkov]

>> > I posted the essence of the framework, as it now is, in a (cut down)
>> > patch at the beginning of this thread.  I have complete patches for both
>> > the framework and isearch.el available.  Together, they are really too
>> > big to post on emacs-devel.
>
>> Could you put them on a branch then?
>
> I've created the branch scratch/follow in savannah, with both the new
> framework and the proposed changes to isearch.el.

Thanks, I'm looking at your changes, and also Cc'ing Martin in the hope
that Martin could review your window-related changes as well.

PS: one immediate question - why do you need a new function isearch-call-message
when at the top of this thread Stefan asked you to replace this code with
(funcall (or isearch-message-function #'isearch-message) ,cqh ,ellip)
and even to change isearch-message-function to default to isearch-message
rather than to nil.

bug#17453: Framework extending window functions for Follow Mode (etc.).

[martin rudalics]

 > Thanks, I'm looking at your changes, and also Cc'ing Martin in the hope
 > that Martin could review your window-related changes as well.

Glimpsing on the window-related changes I think I have already said
everything in this thread.  Usurpating the term "group" in the sense
that all windows belonging to a group have to show the same buffer seems
overly restrictive but I don't want to continue the earlier discussion.

martin

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Juri Linkov]

>> Thanks, I'm looking at your changes, and also Cc'ing Martin in the hope
>> that Martin could review your window-related changes as well.
>
> Glimpsing on the window-related changes I think I have already said
> everything in this thread.  Usurpating the term "group" in the sense
> that all windows belonging to a group have to show the same buffer seems
> overly restrictive but I don't want to continue the earlier discussion.

I agree that the cleanest way to add a new feature is not to change window
primitives, but to add it on the top of existing window primitives
by defining a new functional layer with a set of functions having a common
name prefix either 'window-group-' or 'windows-' (used by follow.el in some
places).  Here are existing primitives and two variants of new functions:

(window-start &optional WINDOW)
(windows-start &optional WINDOW)
(window-group-start &optional WINDOW)

(set-window-start WINDOW POS &optional NOFORCE)
(set-windows-start WINDOW POS &optional NOFORCE)
(set-window-group-start WINDOW POS &optional NOFORCE)

(move-to-window-line COUNT)
(move-to-windows-line COUNT)
(move-to-window-group-line COUNT)

(pos-visible-in-window-p &optional POS WINDOW PARTIALLY)
(pos-visible-in-windows-p &optional POS WINDOW PARTIALLY)
(pos-visible-in-window-group-p &optional POS WINDOW PARTIALLY)

These new functions could be defined in a new core package with a name like
window-group.el or windows.el, or maybe added to the end of window.el,
and have no follow-specific code.

Then there is no need in a set of functions like window-start-group-function,
because one function should be enough for a package like follow.el
to declare an active group of windows via follow-all-followers,
or packages other than follow.el using window parameters.
Then window-group/windows functions should take care about all
aspects of handling multiple windows.

So e.g. new function follow-window-start will be window-group-start
taking only follow-all-followers from follow.el, etc.

This means that while adapting other packages to multi-window configurations,
instead of adding the 't' arg to core primitives, we need to add 's' (window -> windows)
or '-group' to the names of the existing function calls.

The key point is not touching core primitives until we'll be able to
implement a proper display abstraction for window groups such as proposed
a while ago under the name "framelettes".  This comment in follow.el also
gives a hint in this direction:

;; Almost like the real thing, except when the cursor ends up outside
;; the top or bottom...  In our case however, we end up outside the
;; window and hence we are recentered.  Should we let `recenter' handle
;; the point position we would never leave the selected window.  To do
;; it ourselves we would need to do our own redisplay, which is easier
;; said than done.  (Why didn't I do a real display abstraction from
;; the beginning?)

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Juri.

On Thu, Nov 12, 2015 at 10:14:22PM +0200, Juri Linkov wrote:
> >> Thanks, I'm looking at your changes, and also Cc'ing Martin in the hope
> >> that Martin could review your window-related changes as well.

> > Glimpsing on the window-related changes I think I have already said
> > everything in this thread.  Usurpating the term "group" in the sense
> > that all windows belonging to a group have to show the same buffer seems
> > overly restrictive but I don't want to continue the earlier discussion.

> I agree that the cleanest way to add a new feature is not to change window
> primitives, but to add it on the top of existing window primitives
> by defining a new functional layer with a set of functions having a common
> name prefix either 'window-group-' or 'windows-' (used by follow.el in some
> places).

I am coming around to the idea that the new functions are a better way of
implementing this than adding an extra optional parameter to the
primitives.  The reason is, I have a vision that some time in the medium
future, the Follow Mode functionality will come to be implemented
directly in the redisplay engine.  If we add the extra parameter, it
will then be redundant and stick out like a sore thumb.  But with
functions like `window-group-start', these functions could quietly merge
with `window-start', etc., as soon as the display engine stuff is
working.

I have already implemented (though not committed) the extra functional
layer.  The names I used were `window*-start', ....,
`move-to-window*-line'.  Thinking about Anders's comment today that the
"*" could easily get lost, ....

> Here are existing primitives and two variants of new functions:

> (window-start &optional WINDOW)
> (windows-start &optional WINDOW)
> (window-group-start &optional WINDOW)

...., I now think the best name would be the last of these three,
window-group-start.  It is not too long, and the phrase "window_group"
fits in with all the primitives apart from `recenter'.  So we'd need some
different naming ideas, perhaps "recenter-group" or "group-recenter".

> These new functions could be defined in a new core package with a name like
> window-group.el or windows.el, or maybe added to the end of window.el,
> and have no follow-specific code.

I put them into window.el.  It seems like a good place.

> Then there is no need in a set of functions like window-start-group-function,
> because one function should be enough for a package like follow.el
> to declare an active group of windows via follow-all-followers,
> or packages other than follow.el using window parameters.
> Then window-group/windows functions should take care about all
> aspects of handling multiple windows.

We don't want Follow Mode to become too tightly coupled with "its
users".

> So e.g. new function follow-window-start will be window-group-start
> taking only follow-all-followers from follow.el, etc.

> This means that while adapting other packages to multi-window configurations,
> instead of adding the 't' arg to core primitives, we need to add 's' (window -> windows)
> or '-group' to the names of the existing function calls.

I say the '-group' variant.

> The key point is not touching core primitives until we'll be able to
> implement a proper display abstraction for window groups such as proposed
> a while ago under the name "framelettes".  This comment in follow.el also
> gives a hint in this direction:

> ;; Almost like the real thing, except when the cursor ends up outside
> ;; the top or bottom...  In our case however, we end up outside the
> ;; window and hence we are recentered.  Should we let `recenter' handle
> ;; the point position we would never leave the selected window.  To do
> ;; it ourselves we would need to do our own redisplay, which is easier
> ;; said than done.  (Why didn't I do a real display abstraction from
> ;; the beginning?)

Yes.

By the way, have you had the chance to look at the changes I made to
isearch.el in the scratch/follow branch?

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Juri Linkov]

> ...., I now think the best name would be the last of these three,
> window-group-start.  It is not too long, and the phrase "window_group"
> fits in with all the primitives apart from `recenter'.  So we'd need some
> different naming ideas, perhaps "recenter-group" or "group-recenter".

For consistency with other related names it could have the same prefix, e.g.
"window-group-recenter".

> By the way, have you had the chance to look at the changes I made to
> isearch.el in the scratch/follow branch?

One thing in the isearch-related part of your patch that I'm not excited about
is the macro isearch-call-message.  With Stefan's request there is no need in it
since it becomes one-liner: (funcall isearch-message-function).

Another thing that disturbs me is moving lazy-highlighting checks
to a new function isearch-lazy-highlight-maybe-new-loop.

What do you think about moving only window-checking into new function
i.e. only checks for window-start/window-end/etc that need (sit-for 0)
and leaving other checks in isearch-lazy-highlight-new-loop?
Would this help to avoid new problems such as un-highlighting
postponed until the timer fires?

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Juri.

On Wed, Nov 18, 2015 at 02:38:49AM +0200, Juri Linkov wrote:
> > ...., I now think the best name would be the last of these three,
> > window-group-start.  It is not too long, and the phrase "window_group"
> > fits in with all the primitives apart from `recenter'.  So we'd need some
> > different naming ideas, perhaps "recenter-group" or "group-recenter".

> For consistency with other related names it could have the same prefix, e.g.
> "window-group-recenter".

Or even "recenter-window-group", since "window-group" isn't strictly a
prefix in a lot of the other cases anyway.

> > By the way, have you had the chance to look at the changes I made to
> > isearch.el in the scratch/follow branch?

> One thing in the isearch-related part of your patch that I'm not excited about
> is the macro isearch-call-message.  With Stefan's request there is no need in it
> since it becomes one-liner: (funcall isearch-message-function).

A minor thing, which I will amend this evening.

> Another thing that disturbs me is moving lazy-highlighting checks
> to a new function isearch-lazy-highlight-maybe-new-loop.

What disturbs you about this change in particular?

> What do you think about moving only window-checking into new function
> i.e. only checks for window-start/window-end/etc that need (sit-for 0)
> and leaving other checks in isearch-lazy-highlight-new-loop?

Actually, I think that "(sit-for 0)" isn't needed - I left it in to deal
with isearch-lazy-highlight-initial-delay being set to 0, thinking that
Follow Mode's post-command-hook might "not have had time" to run.  But
surely post-command-hook will be called before checking timers.  I'll
need to look at the source code (probably keyboard.c) to be absolutely
sure of this.

I don't think splitting the checks between
isearch-lazy-highlight-new-loop (in the command loop) and a function
triggered by a timer is a good idea.  I tried it out, and it kind of
jars when the lazy highlighting sometimes stays 0.25s, sometimes goes
instantly.  How about maybe ...

How about maybe putting the test for new highlighting in Isearch's
post-command-hook (and using the APPEND argument of `add-hook' to make
sure Isearch's post-hook-function comes after Follow Modes's)?  That
way, all the lazy h. would be removed immediately after the command, as
at present.

> Would this help to avoid new problems such as un-highlighting
> postponed until the timer fires?

Is this actually a problem?  I played with Emacs a little with that
un-highlighting strategy, and didn't find it any more disturbing than
the 0.25s gap between old highlighting going and new highlighting
arriving.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello again, Juri.

On Wed, Nov 18, 2015 at 05:58:14PM +0000, Alan Mackenzie wrote:
> > One thing in the isearch-related part of your patch that I'm not excited about
> > is the macro isearch-call-message.  With Stefan's request there is no need in it
> > since it becomes one-liner: (funcall isearch-message-function).

> A minor thing, which I will amend this evening.

DONE.  I have committed the patch to the scratch/follow branch of the
repository.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Juri Linkov]

>> For consistency with other related names it could have the same prefix, e.g.
>> "window-group-recenter".
>
> Or even "recenter-window-group", since "window-group" isn't strictly a
> prefix in a lot of the other cases anyway.

Maybe "recenter-window-group" in the sense of "recenter a window in the
window group" as opposed to "window-group-recenter" in the sense of
"recenter the whole window group"?

>> Another thing that disturbs me is moving lazy-highlighting checks
>> to a new function isearch-lazy-highlight-maybe-new-loop.
>
> What disturbs you about this change in particular?

The same consequences you mentioned earlier: various time periods
of lazy-highlighting staying active.

>> What do you think about moving only window-checking into new function
>> i.e. only checks for window-start/window-end/etc that need (sit-for 0)
>> and leaving other checks in isearch-lazy-highlight-new-loop?
>
> Actually, I think that "(sit-for 0)" isn't needed - I left it in to deal
> with isearch-lazy-highlight-initial-delay being set to 0, thinking that
> Follow Mode's post-command-hook might "not have had time" to run.  But
> surely post-command-hook will be called before checking timers.  I'll
> need to look at the source code (probably keyboard.c) to be absolutely
> sure of this.
>
> I don't think splitting the checks between
> isearch-lazy-highlight-new-loop (in the command loop) and a function
> triggered by a timer is a good idea.  I tried it out, and it kind of
> jars when the lazy highlighting sometimes stays 0.25s, sometimes goes
> instantly.  How about maybe ...
>
> How about maybe putting the test for new highlighting in Isearch's
> post-command-hook (and using the APPEND argument of `add-hook' to make
> sure Isearch's post-hook-function comes after Follow Modes's)?  That
> way, all the lazy h. would be removed immediately after the command, as
> at present.

You could try to append a new follow+isearch specific hook
to post-command-hook in follow.el, if this helps.

>> Would this help to avoid new problems such as un-highlighting
>> postponed until the timer fires?
>
> Is this actually a problem?  I played with Emacs a little with that
> un-highlighting strategy, and didn't find it any more disturbing than
> the 0.25s gap between old highlighting going and new highlighting
> arriving.

We need to test the same in all modes that use lazy-highlighting too.

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Juri

It's been nearly a week since your last email.  In that time, I tried
one of the approaches (see below) that worked just fine.... except it
wouldn't work with replace.el.  I got discouraged by this, and moved
onto other things for a while.  Now I'm back.

On Thu, Nov 19, 2015 at 02:45:43AM +0200, Juri Linkov wrote:

> >> Another thing that disturbs me is moving lazy-highlighting checks
> >> to a new function isearch-lazy-highlight-maybe-new-loop.

> > What disturbs you about this change in particular?

> The same consequences you mentioned earlier: various time periods
> of lazy-highlighting staying active.

OK.

> >> What do you think about moving only window-checking into new function
> >> i.e. only checks for window-start/window-end/etc that need (sit-for 0)
> >> and leaving other checks in isearch-lazy-highlight-new-loop?

> > Actually, I think that "(sit-for 0)" isn't needed - I left it in to deal
> > with isearch-lazy-highlight-initial-delay being set to 0, thinking that
> > Follow Mode's post-command-hook might "not have had time" to run.  But
> > surely post-command-hook will be called before checking timers.  I'll
> > need to look at the source code (probably keyboard.c) to be absolutely
> > sure of this.

The "(sit-for 0)" is absolutely not needed: post-command-hook is indeed
always invoked before Emacs checks for timer expiry; I tried this out.

> > I don't think splitting the checks between
> > isearch-lazy-highlight-new-loop (in the command loop) and a function
> > triggered by a timer is a good idea.  I tried it out, and it kind of
> > jars when the lazy highlighting sometimes stays 0.25s, sometimes goes
> > instantly.  How about maybe ...

> > How about maybe putting the test for new highlighting in Isearch's
> > post-command-hook (and using the APPEND argument of `add-hook' to make
> > sure Isearch's post-hook-function comes after Follow Modes's)?  That
> > way, all the lazy h. would be removed immediately after the command, as
> > at present.

This is precisely what I tried.  It works very well in Isearch.
However, the problem is in replace.el (the replace functionality, not
`occur'): `query-replace', instead of using the Command Loop like
Isearch does, implements its own command loop.  This seems suboptimal:
it doesn't invoke post-command-hook (or pre-command-hook) until the
entire replace session is over.

This means the use of `query-replace' whilst Follow Mode is enabled is
not going to work properly, without some radical change in replace.el.

Probably the smallest change would be to invoke new hooks
`pre-replace-command-hook' and `post-replace-command-hook' from
`query-replace''s command loop.

A more satisfying change would be to get rid of `perform-replace' and
use Emacs's command loop the way Isearch does.  This would probably not
be all that difficult.  Do you know if there's any special reason
`query-replace' implements its own command loop?

What do you think?

[ .... ]

> We need to test the same in all modes that use lazy-highlighting too.

Yes.  That's a problem, right now.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Juri Linkov]

> This means the use of `query-replace' whilst Follow Mode is enabled is
> not going to work properly, without some radical change in replace.el.
>
> Probably the smallest change would be to invoke new hooks
> `pre-replace-command-hook' and `post-replace-command-hook' from
> `query-replace''s command loop.
>
> A more satisfying change would be to get rid of `perform-replace' and
> use Emacs's command loop the way Isearch does.  This would probably not
> be all that difficult.  Do you know if there's any special reason
> `query-replace' implements its own command loop?

The patch in bug#20430 awaits the possibility of helping to fix this
problem.  It adds a new hook replace-update-post-hook that is like
its isearch counterpart hook isearch-update-post-hook is the right way
to handle display updates like syncing follow windows, etc.

Together with changing the order of calling isearch-update-post-hook
and isearch-lazy-highlight-new-loop in isearch-update, adding
follow-post-command-hook to isearch-update-post-hook, and adding
follow-post-command-hook to replace-update-post-hook to handle
follow-mode in query-replace will comprise the least radical change
just before the next release.

Do you see a shortcoming of this course of action?

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Juri.

On Fri, Nov 27, 2015 at 01:03:43AM +0200, Juri Linkov wrote:
> > This means the use of `query-replace' whilst Follow Mode is enabled is
> > not going to work properly, without some radical change in replace.el.

> > Probably the smallest change would be to invoke new hooks
> > `pre-replace-command-hook' and `post-replace-command-hook' from
> > `query-replace''s command loop.

> > A more satisfying change would be to get rid of `perform-replace' and
> > use Emacs's command loop the way Isearch does.  This would probably not
> > be all that difficult.  Do you know if there's any special reason
> > `query-replace' implements its own command loop?

> The patch in bug#20430 awaits the possibility of helping to fix this
> problem.  It adds a new hook replace-update-post-hook that is like
> its isearch counterpart hook isearch-update-post-hook is the right way
> to handle display updates like syncing follow windows, etc.

Does this patch exist, yet?

It bothers me a little that we might be adding hook after hook into
Emacs, each one for a single special purpose.

Would it not perhaps be better to call `isearch-update-post-hook' also
from `perform-replace', since that would be more economical with hooks;
the meaning of the hook invocation would be "the same" in Isearch and
`perform-replace' - "hook called after having moved to the next match".

> Together with changing the order of calling isearch-update-post-hook
> and isearch-lazy-highlight-new-loop in isearch-update, adding
> follow-post-command-hook to isearch-update-post-hook, and adding
> follow-post-command-hook to replace-update-post-hook to handle
> follow-mode in query-replace will comprise the least radical change
> just before the next release.

This sounds like a good idea.  Though, again, I think calling
isearch-update-post-hook from `query-replace' would be better than
implementing a new hook.

> Do you see a shortcoming of this course of action?

Only that it is working around the problems in replace.el rather than
fixing them.  But to fix them properly would mean a radical redesign of
`perform-replace', or superseding it altogether, which is probably best
postponed until 25.2 or later.  I still think `post-command-hook' is the
best hook for us to use - but it isn't called from `query-replace'.

Would it still be possible to mark `isearch-update-post-hook' as "for
internal use only", so that we could get rid of it later?

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Juri Linkov]

>> The patch in bug#20430 awaits the possibility of helping to fix this
>> problem.  It adds a new hook replace-update-post-hook that is like
>> its isearch counterpart hook isearch-update-post-hook is the right way
>> to handle display updates like syncing follow windows, etc.
>
> Does this patch exist, yet?

Yes, you can find the patch in http://debbugs.gnu.org/20430#8

> It bothers me a little that we might be adding hook after hook into
> Emacs, each one for a single special purpose.

Fortunately, a new hook is not for a single special purpose -
it's a general type of hook, requested for different user needs.

> Would it not perhaps be better to call `isearch-update-post-hook' also
> from `perform-replace', since that would be more economical with hooks;
> the meaning of the hook invocation would be "the same" in Isearch and
> `perform-replace' - "hook called after having moved to the next match".

Logically, it makes sense to reuse isearch hooks in query-replace
since query-replace searches for matches like isearch does, but
practically users might have such customizations in .emacs that
would break query-replace in unpredictable ways.  This is why
a separate query-replace hook would be much safer.

I see no problem for follow-mode to add follow-post-command-hook
to both hooks.

>> Together with changing the order of calling isearch-update-post-hook
>> and isearch-lazy-highlight-new-loop in isearch-update, adding
>> follow-post-command-hook to isearch-update-post-hook, and adding
>> follow-post-command-hook to replace-update-post-hook to handle
>> follow-mode in query-replace will comprise the least radical change
>> just before the next release.
>
> This sounds like a good idea.  Though, again, I think calling
> isearch-update-post-hook from `query-replace' would be better than
> implementing a new hook.

Adding a new hook is just a one-liner, but we have to find the right place
in perform-replace to call it.  I think replace-update-pre-hook should be
called before (read-event), and replace-update-post-hook after (read-event).
I'm not yet sure which one is needed for follow-mode to sync windows?

> Would it still be possible to mark `isearch-update-post-hook' as "for
> internal use only", so that we could get rid of it later?

isearch-update-post-hook is a first-class hook added 5 years ago,
so there is no need to remove it.

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Juri.

On Tue, Dec 01, 2015 at 02:07:31AM +0200, Juri Linkov wrote:
> >> The patch in bug#20430 awaits the possibility of helping to fix this
> >> problem.  It adds a new hook replace-update-post-hook that is like
> >> its isearch counterpart hook isearch-update-post-hook is the right way
> >> to handle display updates like syncing follow windows, etc.

> > Does this patch exist, yet?

> Yes, you can find the patch in http://debbugs.gnu.org/20430#8

OK, I've got it.

> > It bothers me a little that we might be adding hook after hook into
> > Emacs, each one for a single special purpose.

> Fortunately, a new hook is not for a single special purpose -
> it's a general type of hook, requested for different user needs.

I'll stop arguing the philosophy right now (and take it up again a bit
lower down).  ;-)

> > Would it not perhaps be better to call `isearch-update-post-hook' also
> > from `perform-replace', since that would be more economical with hooks;
> > the meaning of the hook invocation would be "the same" in Isearch and
> > `perform-replace' - "hook called after having moved to the next match".

> Logically, it makes sense to reuse isearch hooks in query-replace
> since query-replace searches for matches like isearch does, but
> practically users might have such customizations in .emacs that
> would break query-replace in unpredictable ways.  This is why
> a separate query-replace hook would be much safer.

> I see no problem for follow-mode to add follow-post-command-hook
> to both hooks.

Because this ties Follow Mode to implementation details of isearch.el,
replace.el, and ispell.el.  It is plain ugly for Follow Mode to have to
add-hook an obscure hook for every package that attempts lazy
highlighting.

    (add-hook 'post-command-hook follow-post-command-hook nil t)

should be the only pertinent 'add-hook necessary in follow-mode (and it
would be if isearch.el were the only library to adpat for - I had it
working at one point).  It's not, because replace.el and ispell.el both
attempt to implement their own command loops rather than using Emacs's
standard one.  These design decisions were Bad.

> >> Together with changing the order of calling isearch-update-post-hook
> >> and isearch-lazy-highlight-new-loop in isearch-update, adding
> >> follow-post-command-hook to isearch-update-post-hook, and adding
> >> follow-post-command-hook to replace-update-post-hook to handle
> >> follow-mode in query-replace will comprise the least radical change
> >> just before the next release.

> > This sounds like a good idea.  Though, again, I think calling
> > isearch-update-post-hook from `query-replace' would be better than
> > implementing a new hook.

> Adding a new hook is just a one-liner, but we have to find the right place
> in perform-replace to call it.  I think replace-update-pre-hook should be
> called before (read-event), and replace-update-post-hook after (read-event).
> I'm not yet sure which one is needed for follow-mode to sync windows?

At the moment there is only replace-update-post-hook.  It is called after
point has been moved, but before i-l-highlight-new-loop is called.  I had
to move it slightly from where your patch had put it.

> > Would it still be possible to mark `isearch-update-post-hook' as "for
> > internal use only", so that we could get rid of it later?

> isearch-update-post-hook is a first-class hook added 5 years ago,
> so there is no need to remove it.

Sorry, I made a typo there.  I really meant replace-update-post-hook.
Can we somehow keep this "internal use only", so that we are not bound
somehow to keep supporting it should the `query-replace' command loop be
reformulated (as believe it should, ASAP)?  The same applies to
ispell-update-post-hook, which I've been forced to introduce into
ispell.el for the same reason.

#########################################################################

Anyhow, here's a status update with where I am on making isearch.el and
follow.el work together:

(i) Yesterday I rebased the scratch/follow branch on the emacs-25 branch.
(ii) I haven't yet replaced the GROUP parameter in the windows primitives
  with (e.g.) `window-group-start'.
(iii) isearch.el now appears to work properly.  For this, I had to swap
  the order of invocation of isearch-update-post-hook and i-l-h-new-loop,
  like you said.  I restored i-l-h-new-loop pretty much to the way it was
  prior to my experimentations.
(iv) replace.el now appears to work properly.
(v) ispell.el is more troublesome.  See bug #22097.  I have a problem
  with ispell putting its *Choices* window at the top of the left hand
  Follow Mode window.  Because of FM's sorting algorithm, this causes the
  two windows to be logically swapped, leading to confusing results.  I
  think the neatest solution would be to put *Choices* at the top of the
  rightmost window, preventing this.

Here's a diff of my current state, based off of the rebased
scratch/follow branch mentioned above in (i):



diff --git a/.gitignore b/.gitignore
index 34b0c02..5ef5a5c 100644
--- a/.gitignore
+++ b/.gitignore
@@ -261,6 +261,10 @@ ChangeLog
 [0-9]*.txt
 .dir-locals?.el
 /vc-dwim-log-*
+*.diff
+
+.gitattributes
+*.acm
 
 # Built by 'make install'.
 etc/emacs.tmpdesktop
diff --git a/lisp/follow.el b/lisp/follow.el
index 2cbf0f2..609b29f 100644
--- a/lisp/follow.el
+++ b/lisp/follow.el
@@ -423,6 +423,9 @@ follow-mode
 	(add-hook 'post-command-hook 'follow-post-command-hook t)
 	(add-hook 'window-size-change-functions 'follow-window-size-change t)
         (add-hook 'after-change-functions 'follow-after-change nil t)
+        (add-hook 'isearch-update-post-hook 'follow-post-command-hook nil t)
+        (add-hook 'replace-update-post-hook 'follow-post-command-hook nil t)
+        (add-hook 'ispell-update-post-hook 'follow-post-command-hook nil t)
 
         (setq window-start-group-function 'follow-window-start)
         (setq window-end-group-function 'follow-window-end)
@@ -431,8 +434,7 @@ follow-mode
         (setq pos-visible-in-window-p-group-function
               'follow-pos-visible-in-window-p)
         (setq selected-window-group-function 'follow-all-followers)
-        (setq move-to-window-line-group-function 'follow-move-to-window-line)
-        (setq sit*-for-function 'follow-sit-for))
+        (setq move-to-window-line-group-function 'follow-move-to-window-line))
 
     ;; Remove globally-installed hook functions only if there is no
     ;; other Follow mode buffer.
@@ -445,7 +447,6 @@ follow-mode
 	(remove-hook 'post-command-hook 'follow-post-command-hook)
 	(remove-hook 'window-size-change-functions 'follow-window-size-change)))
 
-    (kill-local-variable 'sit*-for-function)
     (kill-local-variable 'move-to-window-line-group-function)
     (kill-local-variable 'selected-window-group-function)
     (kill-local-variable 'pos-visible-in-window-p-group-function)
@@ -454,6 +455,9 @@ follow-mode
     (kill-local-variable 'window-end-group-function)
     (kill-local-variable 'window-start-group-function)
 
+    (remove-hook 'ispell-update-post-hook 'follow-post-command-hook t)
+    (remove-hook 'replace-update-post-hook 'follow-post-command-hook t)
+    (remove-hook 'isearch-update-post-hook 'follow-post-command-hook t)
     (remove-hook 'after-change-functions 'follow-after-change t)
     (remove-hook 'compilation-filter-hook 'follow-align-compilation-windows t)))
 
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 12ded02..e43d860 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -1018,12 +1018,12 @@ isearch-update
   (setq ;; quit-flag nil  not for isearch-mode
    isearch-adjusted nil
    isearch-yank-flag nil)
-  (when isearch-lazy-highlight
-    (isearch-lazy-highlight-new-loop))
   ;; We must prevent the point moving to the end of composition when a
   ;; part of the composition has just been searched.
   (setq disable-point-adjustment t)
-  (run-hooks 'isearch-update-post-hook))
+  (run-hooks 'isearch-update-post-hook)
+  (when isearch-lazy-highlight
+    (isearch-lazy-highlight-new-loop)))
 
 (defun isearch-done (&optional nopush edit)
   "Exit Isearch mode.
@@ -3068,21 +3068,7 @@ 'isearch-lazy-highlight-cleanup
                                 "22.1")
 
 (defun isearch-lazy-highlight-new-loop (&optional beg end)
-  "Set an idle timer, which will trigger a new `lazy-highlight' loop.
-BEG and END specify the bounds within which highlighting should
-occur.  This is called when `isearch-update' is invoked (which
-can cause the search string to change or the window(s) to
-scroll).  It is also used by other Emacs features.  Do not start
-the loop when we are executing a keyboard macro."
-  (setq isearch-lazy-highlight-start-limit beg
-        isearch-lazy-highlight-end-limit end)
-  (when (null executing-kbd-macro)
-    (setq isearch-lazy-highlight-timer
-          (run-with-idle-timer lazy-highlight-initial-delay nil
-                               'isearch-lazy-highlight-maybe-new-loop))))
-
-(defun isearch-lazy-highlight-maybe-new-loop ()
-  "If needed cleanup any previous `lazy-highlight' loop and begin a new one.
+  "Cleanup any previous `lazy-highlight' loop and begin a new one.
 BEG and END specify the bounds within which highlighting should occur.
 This is called when `isearch-update' is invoked (which can cause the
 search string to change or the window to scroll).  It is also used
@@ -3118,6 +3104,8 @@ isearch-lazy-highlight-maybe-new-loop
     ;; It used to check for `(not isearch-error)' here, but actually
     ;; lazy-highlighting might find matches to highlight even when
     ;; `isearch-error' is non-nil.  (Bug#9918)
+    (setq isearch-lazy-highlight-start-limit beg
+	  isearch-lazy-highlight-end-limit end)
     (setq isearch-lazy-highlight-window       (selected-window)
           isearch-lazy-highlight-window-group (selected-window-group)
 	  isearch-lazy-highlight-window-start (window-start nil t)
@@ -3140,7 +3128,9 @@ isearch-lazy-highlight-maybe-new-loop
 	  isearch-lazy-highlight-regexp-function  isearch-regexp-function
 	  isearch-lazy-highlight-forward      isearch-forward)
     (unless (equal isearch-string "")
-      (isearch-lazy-highlight-update))))
+      (setq isearch-lazy-highlight-timer
+            (run-with-idle-timer lazy-highlight-initial-delay nil
+                                 'isearch-lazy-highlight-update)))))
 
 (defun isearch-lazy-highlight-search ()
   "Search ahead for the next or previous match, for lazy highlighting.
diff --git a/lisp/replace.el b/lisp/replace.el
index 54b3a71..d48f4f3 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -2011,6 +2011,9 @@ replace-match-maybe-edit
   (when backward (goto-char (nth 0 match-data)))
   noedit)
 
+(defvar replace-update-post-hook nil
+  "Function(s) to call after query-replace has found a match in the buffer.")
+
 (defvar replace-search-function nil
   "Function to use when searching for strings to replace.
 It is used by `query-replace' and `replace-string', and is called
@@ -2264,7 +2267,7 @@ perform-replace
 		(and nonempty-match
 		     (or (not regexp-flag)
 			 (and (if backward
-				  (looking-back search-string)
+				  (looking-back search-string nil)
 				(looking-at search-string))
 			      (let ((match (match-data)))
 				(and (/= (nth 0 match) (nth 1 match))
@@ -2318,7 +2321,8 @@ perform-replace
 		;; `real-match-data'.
 		(while (not done)
 		  (set-match-data real-match-data)
-		  (replace-highlight
+                  (run-hooks 'replace-update-post-hook) ; Before `replace-highlight'.
+                  (replace-highlight
 		   (match-beginning 0) (match-end 0)
 		   start end search-string
 		   regexp-flag delimited-flag case-fold-search backward)
diff --git a/lisp/textmodes/ispell.el b/lisp/textmodes/ispell.el
index fe27f0f..fae0549 100644
--- a/lisp/textmodes/ispell.el
+++ b/lisp/textmodes/ispell.el
@@ -2248,6 +2248,11 @@ ispell-pdict-save
   (setq ispell-pdict-modified-p nil))
 
 
+(defvar ispell-update-post-hook nil
+  "A normal hook invoked from the ispell command loop.
+It is called once per iteration, before displaying a prompt to
+the user.")
+
 (defun ispell-command-loop (miss guess word start end)
   "Display possible corrections from list MISS.
 GUESS lists possibly valid affix construction of WORD.
@@ -2315,8 +2320,10 @@ ispell-command-loop
 	      count (ispell-int-char (1+ count))))
       (setq count (ispell-int-char (- count ?0 skipped))))
 
+    (run-hooks 'ispell-update-post-hook)
+
     ;; ensure word is visible
-    (if (not (pos-visible-in-window-p end))
+    (if (not (pos-visible-in-window-p end nil nil t))
 	(sit-for 0))
 
     ;; Display choices for misspelled word.


-- 
Alan Mackenzie (Nuremberg, Germany).

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Juri Linkov]

>> I see no problem for follow-mode to add follow-post-command-hook
>> to both hooks.
>
> Because this ties Follow Mode to implementation details of isearch.el,
> replace.el, and ispell.el.  It is plain ugly for Follow Mode to have to
> add-hook an obscure hook for every package that attempts lazy
> highlighting.

A hook is needed not only for lazy highlighting (in its current state),
but also for every package that doesn't use the standard command loop.

>> Adding a new hook is just a one-liner, but we have to find the right place
>> in perform-replace to call it.  I think replace-update-pre-hook should be
>> called before (read-event), and replace-update-post-hook after (read-event).
>> I'm not yet sure which one is needed for follow-mode to sync windows?
>
> At the moment there is only replace-update-post-hook.  It is called after
> point has been moved, but before i-l-highlight-new-loop is called.  I had
> to move it slightly from where your patch had put it.

Yes, you've moved it to a better place.

> Sorry, I made a typo there.  I really meant replace-update-post-hook.
> Can we somehow keep this "internal use only", so that we are not bound
> somehow to keep supporting it should the `query-replace' command loop be
> reformulated (as believe it should, ASAP)?  The same applies to
> ispell-update-post-hook, which I've been forced to introduce into
> ispell.el for the same reason.

isearch-update-post-hook, replace-update-post-hook, ispell-update-post-hook
are not just a hack, they will stay as useful hooks even after we'll
rewrite query-replace/ispell to use the standard command loop.  These hooks
are for convenience, for the users to be able to set in ~/.emacs, e.g.:

  (add-hook 'isearch-update-post-hook 'recenter)
  (add-hook 'replace-update-post-hook 'recenter)

How would you do the same without these hooks, using only post-command-hook?

> #########################################################################
>
> Anyhow, here's a status update with where I am on making isearch.el and
> follow.el work together:
>
> (i) Yesterday I rebased the scratch/follow branch on the emacs-25 branch.
> (ii) I haven't yet replaced the GROUP parameter in the windows primitives
>   with (e.g.) `window-group-start'.
> (iii) isearch.el now appears to work properly.  For this, I had to swap
>   the order of invocation of isearch-update-post-hook and i-l-h-new-loop,
>   like you said.  I restored i-l-h-new-loop pretty much to the way it was
>   prior to my experimentations.
> (iv) replace.el now appears to work properly.
> (v) ispell.el is more troublesome.  See bug #22097.  I have a problem
>   with ispell putting its *Choices* window at the top of the left hand
>   Follow Mode window.  Because of FM's sorting algorithm, this causes the
>   two windows to be logically swapped, leading to confusing results.  I
>   think the neatest solution would be to put *Choices* at the top of the
>   rightmost window, preventing this.

I believe bug#22097 is easy to fix for lazy highlighting, but what you
described above is more troublesome, and might require adding support for
window groups to ispell.el (like you're adding it to isearch.el).

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Juri.

On Sun, Dec 06, 2015 at 01:06:05AM +0200, Juri Linkov wrote:
> >> I see no problem for follow-mode to add follow-post-command-hook
> >> to both hooks.

> > Because this ties Follow Mode to implementation details of isearch.el,
> > replace.el, and ispell.el.  It is plain ugly for Follow Mode to have to
> > add-hook an obscure hook for every package that attempts lazy
> > highlighting.

> A hook is needed not only for lazy highlighting (in its current state),
> but also for every package that doesn't use the standard command loop.

Yes, I think we're in violent agreement here.

[ .... ]

> > Sorry, I made a typo there.  I really meant replace-update-post-hook.
> > Can we somehow keep this "internal use only", so that we are not
> > bound somehow to keep supporting it should the `query-replace'
> > command loop be reformulated (as believe it should, ASAP)?  The same
> > applies to ispell-update-post-hook, which I've been forced to
> > introduce into ispell.el for the same reason.

> isearch-update-post-hook, replace-update-post-hook, ispell-update-post-hook
> are not just a hack, they will stay as useful hooks even after we'll
> rewrite query-replace/ispell to use the standard command loop.  These hooks
> are for convenience, for the users to be able to set in ~/.emacs, e.g.:

>   (add-hook 'isearch-update-post-hook 'recenter)
>   (add-hook 'replace-update-post-hook 'recenter)

First comment: this sort of thing will wreck Follow Mode, scrolling text
in the current window rather than point moving forward to the next
window.

> How would you do the same without these hooks, using only post-command-hook?

By putting #'recenter into `post-command-hook'?  This should work for
Isearch, and would work for Replace and Ispell if these two used the
Emacs command loop.

> > #########################################################################

> > Anyhow, here's a status update with where I am on making isearch.el and
> > follow.el work together:

> > (i) Yesterday I rebased the scratch/follow branch on the emacs-25 branch.
> > (ii) I haven't yet replaced the GROUP parameter in the windows primitives
> >   with (e.g.) `window-group-start'.
> > (iii) isearch.el now appears to work properly.  For this, I had to swap
> >   the order of invocation of isearch-update-post-hook and i-l-h-new-loop,
> >   like you said.  I restored i-l-h-new-loop pretty much to the way it was
> >   prior to my experimentations.
> > (iv) replace.el now appears to work properly.
> > (v) ispell.el is more troublesome.  See bug #22097.  I have a problem
> >   with ispell putting its *Choices* window at the top of the left hand
> >   Follow Mode window.  Because of FM's sorting algorithm, this causes the
> >   two windows to be logically swapped, leading to confusing results.  I
> >   think the neatest solution would be to put *Choices* at the top of the
> >   rightmost window, preventing this.

I have indeed put *Choices* at the top of the RH window, to preserve
sanity.

I committed those changes, including changes to Ispell this afternoon.  I
think they're close to working fully.

> I believe bug#22097 is easy to fix for lazy highlighting, but what you
> described above is more troublesome, and might require adding support for
> window groups to ispell.el (like you're adding it to isearch.el).

I've put the window group stuff into ispell.el - there wasn't a lot to
change.

This afternoon, lack of lazy highlighting in Ispell (bug #22097) made
itself noticeable again.  I'll see what I can do, here.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Juri Linkov]

>>   (add-hook 'isearch-update-post-hook 'recenter)
>>   (add-hook 'replace-update-post-hook 'recenter)
>
> First comment: this sort of thing will wreck Follow Mode, scrolling text
> in the current window rather than point moving forward to the next
> window.

I guess a lot of other customizations will wreck Follow Mode too.

>> How would you do the same without these hooks, using only post-command-hook?
>
> By putting #'recenter into `post-command-hook'?  This should work for
> Isearch, and would work for Replace and Ispell if these two used the
> Emacs command loop.

This will call #'recenter on every command.

> I have indeed put *Choices* at the top of the RH window, to preserve
> sanity.
>
> I committed those changes, including changes to Ispell this afternoon.  I
> think they're close to working fully.

Yes, pretty close.

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Juri.

On Thu, Nov 12, 2015 at 02:52:56AM +0200, Juri Linkov wrote:
> >> > I posted the essence of the framework, as it now is, in a (cut down)
> >> > patch at the beginning of this thread.  I have complete patches for both
> >> > the framework and isearch.el available.  Together, they are really too
> >> > big to post on emacs-devel.

> >> Could you put them on a branch then?

> > I've created the branch scratch/follow in savannah, with both the new
> > framework and the proposed changes to isearch.el.

> Thanks, I'm looking at your changes, and also Cc'ing Martin in the hope
> that Martin could review your window-related changes as well.

Thanks.

> PS: one immediate question - why do you need a new function isearch-call-message
> when at the top of this thread Stefan asked you to replace this code with
> (funcall (or isearch-message-function #'isearch-message) ,cqh ,ellip)
> and even to change isearch-message-function to default to isearch-message
> rather than to nil.

isearch-call-message is actually a macro.  It's exists because it's
invoked several times from the rest of the code, and is cleaner this way.

Stefan's request just slipped my memory.  It is a relatively minor
stylistic thing, of little consequence.  At the time he requested it, he
had lots of far more important criticisms to make.  But I could change
it now, no problem.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Alan Mackenzie]

Hello, Juri.

On Mon, Nov 09, 2015 at 02:50:04AM +0200, Juri Linkov wrote:
> >> This problem is already solved by enabling lazy-highlighting of the whole
> >> follow-mode buffer, but I postponed installing that patch to not create merge
> >> conflicts with your work in the same functions.

> > My solution is to lazily highlight just the part visible in windows (but
> > in all pertinent windows).  Isn't highlighting all of it going to be a
> > bit slow on a large buffer?

> It shouldn't be slow with a sufficiently low value of lazy-highlight-max-at-a-time.

OK, I see it: it would be slow if it had to go to completion, but any
key/mouse/other event will terminate it.  So the only place it might be a
problem is somewhere where CPU cycles are at a premium.

> > Part 3 of the bug is most assuredly NOT an intrinsic limitation of
> > isearch-allow-scroll.  It's caused by the variables w-L1 and w-L-1, the
> > bounds for the permissible scrolling range in
> > isearch-string-out-of-window, being set to the top and botom of the
> > _single_ window.  When these variables are set to the top of bottom of
> > the entire FM group of windows, the bug is solved.  This requires my new
> > framework, or something like it.

> I tried to not use isearch-string-out-of-window/isearch-back-into-window
> at all, but I can't get a useful behavior in such situation of scrolling
> out of the window with the current search hit.  Could you show how you see
> it should work in this case in follow-mode?

To start with, set

  (global-set-key [next] 'follow-scroll-up)
  (global-set-key [prior] 'follow-scroll-down)
  (setq isearch-allow-scroll t)

.  Then start an Isearch not too close to the start of a buffer with
Follow Mode enabled with at least two windows.  Type something to get a
search match highlighted.  Now <PageUp> and <PageDown> should scroll that
match between Follow Mode windows, the boundaries of that scrolling being
the top of the LH window and the bottom of the RH window.

To make this work properly, the four variables in
isearch-string-out-of-window, w-start, w-end, w-L1, w-L-1, are set to the
positions in the entire group of windows, by setting the proposed
&optional argument GROUP to t in the calls to certain window functions,
e.g.

     (let ((w-start (window-start nil t))
                                      ^

> >> > 4: With point near the bottom of a Follow Mode window, start an Isearch,
> >> >   and repeatedly do M-s C-e, until the highlighted match continues on to
> >> >   the next window.  Continue doing M-s C-e until the string in the
> >> >   minibuffer expands by a line.  At this point the top of the RH window
> >> >   gets spuriously scrolled into the middle of the window, leaving the FM
> >> >   windows unsynchronised.

> >> I see the same behavior even without Isearch.

> > The buggy behaviour I've described is explicitly and essentially an
> > Isearch scenario.  What do you mean by "the same behavior" and what
> > sequence of commands generates it?

> > The solution to problem 4 involves making sure point is at the right
> > place at the right time when invoking isearch-message.  I have made
> > changes to fix it.

> I meant such things as 'M-: (message "x\ny\nz") RET' causes jumping too.

It causes some scrolling of windows to keep them properly aligned when
the echo area expands to three lines.  This is expected and proper.
However, when the echo area goes back to one line, the windows don't
scroll back again, leaving them unaligned.  I raised a bug for this this
morning.  It seems it is a known problem which is tricky to solve.

> >> Honestly, I see here nothing that requires a new multi-window framework.

> > I want these bugs to be solved.  What, then, are the alternatives to
> > this framework (or something like it)?  Isearch needs information about
> > the Follow Mode windows, or it can't work properly.

[ .... ]

> > So, where do we go from here?  I would like these bugs fixed for 25.1.

> The problem is that you are trying to design a new framework,
> but not demonstrated yet how it would be useful generally in cases
> other than a specific combination of isearch/follow-mode.

Any elisp software which manipulates windows will be able to work better
by considering entire FM groups.  I did some grepping: here are some
examples which the new framework could improve:

`kill-ring-save' should blink the cursor on the matching other-end, if
that is visible.  With FM active, and other end being in a different
window, currently it wrongly outputs a message instead.

`window-screen-lines' could be enhanced (optionally) to return the number
of screen lines in the entire group.

`blink-matching-open' could be enhanced to handle FM windows properly.

`ns-scroll-bar-move' (probably) should `set-window-start' for the entire
window group.  This (might) avoid scrolling such that the RH follow
window is displaying empty space.


One point about this new framework is that it is "harmless" in the sense
that anything which worked (or failed to work) prior to it will continue
unchanged unless specifically amended.  It's disadvantage is that it adds
a little to the bulk of things people need to know or to look up in
manuals.

I think Follow Mode should become easier to use, both for users and for
hackers, especially given that wide screens (>200 characters) are now the
norm rather than the exception.  At the moment Follow Mode is
ridiculously difficult to start (manually splitting windows, doing M-x
follow-mode) and even more difficult to hack for - just what interfaces
are currently available for making a package work with FM?  Answer: none
at all.  This new framework is intended to go some way to closing the
gap.

The alternative to a framework is a bugfix which is specific to Isearch.
Even this would require some sort of interface definition and
abstraction.  At a minimum, for "part 3" of the bug, Isearch would need
somehow to obtain the bounds of the group of windows.

The last alternative is a quick and dirty fix where Isearch would just
call Follow Mode functions.  I don't think anybody really wants this.

Would it help if I actually made the source available?  If so, where?  I
don't really think it would be appropriate to dump a patch of this size
on emacs-devel, and the time to commit the changes to master has clearly
not yet arrived.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#17453: Framework extending window functions for Follow Mode (etc.).

[Juri Linkov]

>> >> I admit I am confused. I thought we had narrowed down three possible
>> >> solutions to this issue (which you just listed over at the bug thread), all
>> >> of which are simpler than this code.
>
>> > Those solutions are to merely one part of the bug, namely C-s wrongly
>> > scrolling a window instead of moving onto the next one.
>
>> > The other parts of #17453 are:
>> > 2: lazy highlighting is confined to one Follow Mode window (I'm a bit
>> >   confused as to the status of this, though);
>
>> This problem is already solved by enabling lazy-highlighting of the whole
>> follow-mode buffer, but I postponed installing that patch to not create merge
>> conflicts with your work in the same functions.
>
> My solution is to lazily highlight just the part visible in windows (but
> in all pertinent windows).  Isn't highlighting all of it going to be a
> bit slow on a large buffer?

It shouldn't be slow with a sufficiently low value of lazy-highlight-max-at-a-time.

>> > 3: With isearch-allow-scroll enabled, it is not possible to scroll point
>> >   to the next or previous Follow Mode window;
>> > , in addition to which I have a fix for ...
>
>> That's because isearch-post-command-hook uses isearch-pre-scroll-point
>> to move it back, so it's possible to nullify isearch-pre-scroll-point
>> in follow-mode, but I see that it leaves the highlighted found string
>> at its old position because the isearch-allow-scroll feature doesn't support
>> finding the next search hit after scrolling, or something like this
>> that would make sense.  IOW, this is a limitation of isearch-allow-scroll.
>
> In my personal copy of Emacs, I've had the isearch scrolling working
> properly with Follow Mode for ~18 months.  It was me that wrote the
> isearch scrolling code in the first place, back in 2003.

Yes, I remember, and appreciate your efforts to develop it further.

> Part 3 of the bug is most assuredly NOT an intrinsic limitation of
> isearch-allow-scroll.  It's caused by the variables w-L1 and w-L-1, the
> bounds for the permissible scrolling range in
> isearch-string-out-of-window, being set to the top and botom of the
> _single_ window.  When these variables are set to the top of bottom of
> the entire FM group of windows, the bug is solved.  This requires my new
> framework, or something like it.

I tried to not use isearch-string-out-of-window/isearch-back-into-window
at all, but I can't get a useful behavior in such situation of scrolling
out of the window with the current search hit.  Could you show how you see
it should work in this case in follow-mode?

>> > 4: With point near the bottom of a Follow Mode window, start an Isearch,
>> >   and repeatedly do M-s C-e, until the highlighted match continues on to
>> >   the next window.  Continue doing M-s C-e until the string in the
>> >   minibuffer expands by a line.  At this point the top of the RH window
>> >   gets spuriously scrolled into the middle of the window, leaving the FM
>> >   windows unsynchronised.
>
>> I see the same behavior even without Isearch.
>
> The buggy behaviour I've described is explicitly and essentially an
> Isearch scenario.  What do you mean by "the same behavior" and what
> sequence of commands generates it?
>
> The solution to problem 4 involves making sure point is at the right
> place at the right time when invoking isearch-message.  I have made
> changes to fix it.

I meant such things as 'M-: (message "x\ny\nz") RET' causes jumping too.

>> Honestly, I see here nothing that requires a new multi-window framework.
>
> I want these bugs to be solved.  What, then, are the alternatives to
> this framework (or something like it)?  Isearch needs information about
> the Follow Mode windows, or it can't work properly.
>
> So far, I've written three solutions for these bugs, as I outlined at
> length in an email to Martin R. today.  The first of these solutions was
> (justifiably) rejected by Stefan because it was a quick and dirty fix,
> and he explicitly requested the new framework that I have now built.
> Eli didn't like the second attempt and explicitly suggested the way for
> my third attempt.  You and Martin dislike this most recent third
> attempt.
>
> It seems to me I've spent more time discussing this bug on the bug list
> and emacs-devel, and reformulating the fix, than actually tracking down
> and fixing the bugs in the first place.  At the moment I feel like I'm
> trying to hack down a wall of constant negativity.  I don't recall
> anybody else saying positively they want this bug fixed, and I certainly
> don't feel I've had much encouragement wrt this bug, in the last few
> days and weeks.
>
> I see Follow Mode as being a critically important component of Emacs,
> the more so since very wide (240 characters and more) screens displaced
> the fairly narrow CRT monitors.  I would like every Emacs user to be
> able to use FM as easily as I can.  Right at the moment there is no
> suitable interface to FM for libraries that require to do their own
> window manipulation.  Such an interface is what Stefan wanted, and it's
> what I want, too.  As of yet, there's been practically no discussion of
> this interface I've written, beyond Eli suggesting the current version
> and John suggesting a name change for some hooks.
>
> So, where do we go from here?  I would like these bugs fixed for 25.1.

The problem is that you are trying to design a new framework,
but not demonstrated yet how it would be useful generally in cases
other than a specific combination of isearch/follow-mode.