bug#13055: 24.3.50; `scroll-margin' not always honored in Info buffers

[Eli Zaretskii]

> Date: Mon, 3 Dec 2012 08:34:25 +0100
> From: Dani Moncayo <dmoncayo@gmail.com>
> Cc: 13055@debbugs.gnu.org
> 
> IMO, `scroll-margin' clearly makes sense whenever the displayed text
> changes, regardless of the relation between the old displayed text and
> the new one.

You are reading into the variable a meaning it never had.

> >> As I see it, this variable guarantees the users to _always_ see some
> >> context lines around point, which is an important feature to me.
> >
> > No, it doesn't.
> 
> It should.  In fact it does

No, it does not.  I could show you more examples, but I don't think it
will matter.  You won't change your mind.

> Please Eli, reconsider this.  The meaning of `scroll-margin' makes
> perfectly sense here.

It is against my best technical judgment to make changes that spread
the effect of scroll-related options beyond scrolling.  It would be
one more step towards making redisplay_window, which is the main
workhorse of the display engine, an unmaintainable heap of twisty
little passages all alike.  We are already half way there, just take
the hint from the fact that we need to explain in the user manual the
order of priority between 3 options that control scrolling in
contradicting ways.  Or just read the code, if you dare, and then try
writing a coherent description of what it does, and why.  I'm sure we
got there by following exactly this kind of path to creeping
featurism, accompanied all the way by requests like "please,
pretty-please, give me just this one more little feature."

Anyway, I'm tired of having my arguments heard and discarded.  The
change that will give you what you asked for is below.  I will commit
it -- under protest -- provided that Stefan and/or Chong give their
blessing -- not to the changes, but to the idea of applying
scroll-margin to this unrelated use case, and a marginal one at that.

=== modified file 'src/xdisp.c'
--- src/xdisp.c 2012-11-30 09:23:15 +0000
+++ src/xdisp.c 2012-12-03 17:24:48 +0000
@@ -15717,6 +15717,34 @@ redisplay_window (Lisp_Object window, in
             Move it back to a fully-visible line.  */
          new_vpos = window_box_height (w);
        }
+      else if (w->cursor.vpos >=0)
+       {
+         /* Some people insist on not letting point enter the scroll
+            margin, even though this part handles windows that didn't
+            scroll at all.  */
+         struct frame *f = XFRAME (w->frame);
+         int margin = min (scroll_margin, WINDOW_TOTAL_LINES (w) / 4);
+         int pixel_margin = margin * FRAME_LINE_HEIGHT (f);
+
+         if (w->cursor.vpos < margin)
+           new_vpos = pixel_margin;
+         else
+           {
+             int window_height = window_box_height (w);
+
+             if (w->cursor.y >= window_height - pixel_margin)
+               {
+                 /* Note the gotcha: window_box_height returns the
+                    pixel size of the window excluding the header
+                    line, but pixel coordinates, like new_vpos, are
+                    measured from the beginning of the window, and
+                    thus include the header line size.  */
+                 new_vpos = window_height - pixel_margin;
+                 if (WINDOW_WANTS_HEADER_LINE_P (w))
+                   new_vpos += CURRENT_HEADER_LINE_HEIGHT (w);
+               }
+           }
+       }
 
       /* If we need to move point for either of the above reasons,
         now actually do it.  */