bug#12170: save-excursion fails boundary case with recenter

[martin rudalics <rudalics@gmx.at>]

 > Your recipe also calls set-window-point, which moves point on its own,
 > and also does this:
 >
 >   /* We have to make sure that redisplay updates the window to show
 >      the new value of point.  */
 >   if (!EQ (window, selected_window))
 >     ++windows_or_buffers_changed;
 >
 > The windows_or_buffers_changed flag will force a thorough redisplay.
 >
 > Given this, do we still have something unexplained?

Yes, because `set-window-point' doesn't set windows_or_buffers_changed
in the case at hand since the window is the selected window.  Or am I
missing something?

 >> If you agree, then we'd have to explain why a subsequent invocation of
 >> `set-window-start' with NOFORCE t can override the setting of the window
 >> start position implied by the last invocation of one of the functions
 >> mentioned above.
 >
 > You didn't just call set-window-start, you also called
 > set-window-point.  If I remove that second call, the result of your
 > code is very different, and the window start position as the macro set
 > it is still in effect when control is returned.

But `set-window-point' should be equivalent to `goto-char' here because
the window is the selected window.

 >>  >> w->force_start 1 will cause redisplay to honor the start position set up
 >>  >> by `recenter'
 >>  >
 >>  > Only if point will be visible when window is displayed starting at
 >>  > startp.
 >>
 >> I completely miss you here.
 >
 > I meant this code:
 >
 >        w->optional_new_start = 0;
 >        start_display (&it, w, startp);
 >        move_it_to (&it, PT, 0, it.last_visible_y, -1,
 > 		  MOVE_TO_POS | MOVE_TO_X | MOVE_TO_Y);
 >        if (IT_CHARPOS (it) == PT)
 > 	w->force_start = 1;
 >        /* IT may overshoot PT if text at PT is invisible.  */
 >        else if (IT_CHARPOS (it) > PT && CHARPOS (startp) <= PT)
 > 	w->force_start = 1;
 >
 > It only sets the force_start flag if displaying the window starting at
 > startp will show point visible inside the window.  The call to
 > move_it_to moves in a simulated display lines, and stops either at
 > point or at the last pixel position visible in the window, whichever
 > happens first.  The subsequent test that IT_CHARPOS (it) == PT
 > verifies that it stopped at point and not because it reached the end
 > of the text displayed in the window.
 >
 > Ergo, sometimes setting the window start position will not be
 > honored.  That's what the comment above all this means when it says:
 >
 >   /* If someone specified a new starting point but did not insist,
 >      check whether it can be used.  */
 >
 > "Did not insist" means that whoever set w->optional_new_start did not
 > also set w->force_start.  The "check whether it can be used" part
 > describes what I just explained.

I believe you.  But it remains a mystery to me why `set-window-point'
should make a difference here.  As a matter of fact, if I do

(progn
   (defmacro save-this-window-excursion (&rest body)
     "..."
     (let ((start (make-symbol "start"))
       (point (make-symbol "point")))
       `(let ((,start (copy-marker (window-start)))
          (,point (copy-marker (window-point))))
      (save-selected-window
        (progn ,@body))
      (set-window-start (selected-window) ,start t)
      (with-current-buffer (window-buffer (selected-window))
        (goto-char ,point)))))

   (defun f (n)
      (save-this-window-excursion (forward-line (- n)) (recenter 0)))

    (let ((buffer (switch-to-buffer "foo"))
          (height (1- (window-height (get-buffer-window "foo")))))
      (insert-char 10 (* height 2))
      (let ((pt (point)))
        (f height)
        (redisplay)
        (message "height %s old %s new %s" height pt (point)))))

I get the same results as with `set-window-point'.  IMHO the
`set-window-start' call makes the difference but I don't understand why.

martin