bug#5718: scroll-margin in buffer with small line count.

[npostavs]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: npostavs@users.sourceforge.net
>> Cc: 5718@debbugs.gnu.org,  ahyatt@gmail.com,  gavenkoa@gmail.com
>> Date: Tue, 13 Sep 2016 22:40:29 -0400
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> >> I have a patch set for fixing this and allowing the user to change the
>> >> maximum margin from 0.25.  The latter doesn't quite work perfectly, for
>> >> some reason when setting the maximum margin to 0.5 and scroll-margin to
>> >> 100, `scroll-down-command' doesn't keep point centered in the window,
>> >> even though other commands (e.g. `scroll-up-command') do.
[...]
>
> ... given the above, I now understand that your interpretation of 0.5
> is "half the window minus one line", which leaves the center line for
> point.  Is that so?

Yes, I hadn't quite thought it through, but that was my intention.  I've
updated the code to clarify this.

v2-0001-Refactor-uses-of-scroll_margin-to-a-function.patch.gz
Description: patch

v2-0002-Don-t-count-mode-line-for-scroll-margin-limit.patch
Description: patch

v2-0003-Make-limit-on-scroll-margin-variable.patch
Description: patch

The issues with `scroll-down-command' (and `next-line', below) remain.
I find the following change fixes the problem for `scroll-down-command',
though I'm not sure whether it's the right thing to do.

--- a/src/window.c
+++ b/src/window.c
@@ -5148,7 +5148,7 @@ window_scroll_pixel_based (Lisp_Object window, int n, 
bool whole, bool noerror)
         in the scroll margin at the bottom.  */
       move_it_to (&it, PT, -1,
                  (it.last_visible_y - WINDOW_HEADER_LINE_HEIGHT (w)
-                  - this_scroll_margin - 1),
+                  - this_scroll_margin - 1 - frame_line_height),
                  -1,
                  MOVE_TO_POS | MOVE_TO_Y);

>>>
>>> Also, did you test these changes with scroll-conservatively set to
>>> 101?  If not, please do, as that setting activates some code parts
>>> that no other option does.
>>
>> I hadn't; trying it out now, it seems to cause `next-line' to also have
>> the bad behaviour of `scroll-down-command' where point is one line too
>> far down.

For `next-line', the initial difference seems to be in `line-move'.
When `scroll-conservatively' is non-zero (I found no change at 101, in
particular), it calls `vertical-motion` (via `line-move-visual'),
otherwise `line-move-partial'.

    (defun line-move (arg &optional noerror _to-end try-vscroll)
    ...
        (unless (and auto-window-vscroll try-vscroll
                     ;; Only vscroll for single line moves
                     (= (abs arg) 1)
                     ;; Under scroll-conservatively, the display engine
                     ;; does this better.
                     (zerop scroll-conservatively)
                     ...
                     (line-move-partial arg noerror))
          ...
              (prog1 (line-move-visual arg noerror)

The problem involves partial lines.  In a window where
(window-screen-lines) returns 7.222, doing M-: (vertical-motion '(0.0
. 1)) does not scroll the window, which lets point end up 1 line away
from the center.  Doing M-: (vertical-motion '(0.0 . -1)) does scroll
the window, keeping the point in the center (as expected).

Adjusting the window so that (window-screen-lines) returns 7.0, there is
no discrepancy between up and down motion.

I guess there is some incorrect boundary condition in try_scrolling,
though I haven't worked out where.