[Emacs-diffs] /srv/bzr/emacs/trunk r103792: Fix bug #6671 with recentering and other scrolling problems.

[Eli Zaretskii]

------------------------------------------------------------
revno: 103792 [merge]
committer: Eli Zaretskii <address@hidden>
branch nick: trunk
timestamp: Thu 2011-03-31 21:12:30 +0200
message:
  Fix bug #6671 with recentering and other scrolling problems.
  
   src/xdisp.c (SCROLL_LIMIT): New macro.
   (try_scrolling): Use it when setting scroll_limit.  Limit
   scrolling to 100 screen lines.
   (redisplay_window): Even when falling back on "recentering",
   position point in the window according to scroll-conservatively,
   scroll-margin, and scroll-*-aggressively variables.
   (try_scrolling): When point is above the window, allow searching
   as far as scroll_max, or one screenful, to compute vertical
   distance from PT to the scroll margin position.  This prevents
   try_scrolling from unnecessarily failing when
   scroll-conservatively is set to a value slightly larger than the
   window height.  Clean up the case of PT below the margin at bottom
   of window: scroll_max can no longer be INT_MAX.  When aggressive
   scrolling is in use, don't let point enter the opposite scroll
   margin as result of the scroll.
   (syms_of_xdisp) <scroll-conservatively>: Document the
   threshold of 100 lines for never-recentering scrolling.
  
   doc/emacs/display.texi (Auto Scrolling): Document the limit of 100 lines
   for never-recentering scrolling with `scroll-conservatively'.
modified:
  doc/emacs/ChangeLog
  doc/emacs/display.texi
  src/ChangeLog
  src/xdisp.c

=== modified file 'doc/emacs/ChangeLog'
--- a/doc/emacs/ChangeLog       2011-03-12 19:19:47 +0000
+++ b/doc/emacs/ChangeLog       2011-03-30 20:59:42 +0000
@@ -1,3 +1,9 @@
+2011-03-30  Eli Zaretskii  <address@hidden>
+
+       * display.texi (Auto Scrolling): Document the limit of 100 lines
+       for never-recentering scrolling with `scroll-conservatively'.
+       (Bug#6671)
+
 2011-03-12  Eli Zaretskii  <address@hidden>
 
        * msdog.texi (Windows HOME): Fix the wording to clarify how Emacs sets

=== modified file 'doc/emacs/display.texi'
--- a/doc/emacs/display.texi    2011-03-08 04:31:57 +0000
+++ b/doc/emacs/display.texi    2011-03-30 20:59:42 +0000
@@ -183,7 +183,20 @@
 if you set @code{scroll-conservatively} to a small number @var{n},
 then if you move point just a little off the screen (less than @var{n}
 lines), Emacs scrolls the text just far enough to bring point back on
-screen.  By default, @code{scroll-conservatively} address@hidden
+screen.  By default, @code{scroll-conservatively} address@hidden  If you
+set @code{scroll-conservatively} to a large number (larger than 100),
+Emacs will never center point as result of scrolling, even if point
+moves far away from the text previously displayed in the window.  With
+such a large value, Emacs will always scroll text just enough for
+bringing point into view, so point will end up at the top or bottom of
+the window, depending on the scroll direction.
+
address@hidden scroll-step
+  The variable @code{scroll-step} determines how many lines to scroll
+the window when point moves off the screen.  If moving by that number
+of lines fails to bring point back into view, point is centered
+instead.  The default value is zero, which causes point to always be
+centered after scrolling.
 
 @cindex aggressive scrolling
 @vindex scroll-up-aggressively
@@ -204,6 +217,9 @@
 bottom of the window; thus, as with @code{scroll-up-aggressively}, a
 larger value is more aggressive.
 
+  These two variables are ignored if either @code{scroll-step} or
address@hidden are set to a non-zero value.
+
 @vindex scroll-margin
   The variable @code{scroll-margin} restricts how close point can come
 to the top or bottom of a window.  Its value is a number of screen

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2011-03-31 01:37:51 +0000
+++ b/src/ChangeLog     2011-03-31 19:12:30 +0000
@@ -1,3 +1,29 @@
+2011-03-31  Eli Zaretskii  <address@hidden>
+
+       * xdisp.c (SCROLL_LIMIT): New macro.
+       (try_scrolling): Use it when setting scroll_limit.  Limit
+       scrolling to 100 screen lines.
+       (redisplay_window): Even when falling back on "recentering",
+       position point in the window according to scroll-conservatively,
+       scroll-margin, and scroll-*-aggressively variables.  (Bug#6671)
+
+       (try_scrolling): When point is above the window, allow searching
+       as far as scroll_max, or one screenful, to compute vertical
+       distance from PT to the scroll margin position.  This prevents
+       try_scrolling from unnecessarily failing when
+       scroll-conservatively is set to a value slightly larger than the
+       window height.  Clean up the case of PT below the margin at bottom
+       of window: scroll_max can no longer be INT_MAX.  When aggressive
+       scrolling is in use, don't let point enter the opposite scroll
+       margin as result of the scroll.
+       (syms_of_xdisp) <scroll-conservatively>: Document the
+       threshold of 100 lines for never-recentering scrolling.
+
+2011-03-26  Eli Zaretskii  <address@hidden>
+
+       * xdisp.c (redisplay_window): Don't check buffer's clip_changed
+       flag as a prerequisite for invoking try_scrolling.  (Bug#6671)
+
 2011-03-31  Juanma Barranquero  <address@hidden>
 
        * dispextern.h (move_it_by_lines):
@@ -184,11 +210,6 @@
        * s/usg5-4-common.h (SIGTYPE): Remove definition.
        * s/template.h (SIGTYPE): Remove commented out definition.
 
-2011-03-26  Eli Zaretskii  <address@hidden>
-
-       * xdisp.c (redisplay_window): Don't check buffer's clip_changed
-       flag as a prerequisite for invoking try_scrolling.  (Bug#6671)
-
 2011-03-26  Juanma Barranquero  <address@hidden>
 
        * w32.c (read_unc_volume): Use parameter `henum', instead of

=== modified file 'src/xdisp.c'
--- a/src/xdisp.c       2011-03-31 01:37:51 +0000
+++ b/src/xdisp.c       2011-03-31 19:12:30 +0000
@@ -12992,6 +12992,12 @@
   SCROLLING_NEED_LARGER_MATRICES
 };
 
+/* If scroll-conservatively is more than this, never recenter.
+
+   If you change this, don't forget to update the doc string of
+   `scroll-conservatively' and the Emacs manual.  */
+#define SCROLL_LIMIT 100
+
 static int
 try_scrolling (Lisp_Object window, int just_this_one_p,
               EMACS_INT arg_scroll_conservatively, EMACS_INT scroll_step,
@@ -13005,7 +13011,8 @@
   int dy = 0, amount_to_scroll = 0, scroll_down_p = 0;
   int extra_scroll_margin_lines = last_line_misfit ? 1 : 0;
   Lisp_Object aggressive;
-  int scroll_limit = INT_MAX / FRAME_LINE_HEIGHT (f);
+  /* We will never try scrolling more than this number of lines.  */
+  int scroll_limit = SCROLL_LIMIT;
 
 #if GLYPH_DEBUG
   debug_method_add (w, "try_scrolling");
@@ -13021,14 +13028,14 @@
   else
     this_scroll_margin = 0;
 
-  /* Force arg_scroll_conservatively to have a reasonable value, to avoid
-     overflow while computing how much to scroll.  Note that the user
-     can supply scroll-conservatively equal to `most-positive-fixnum',
-     which can be larger than INT_MAX.  */
+  /* Force arg_scroll_conservatively to have a reasonable value, to
+     avoid scrolling too far away with slow move_it_* functions.  Note
+     that the user can supply scroll-conservatively equal to
+     `most-positive-fixnum', which can be larger than INT_MAX.  */
   if (arg_scroll_conservatively > scroll_limit)
     {
-      arg_scroll_conservatively = scroll_limit;
-      scroll_max = INT_MAX;
+      arg_scroll_conservatively = scroll_limit + 1;
+      scroll_max = scroll_limit * FRAME_LINE_HEIGHT (f);
     }
   else if (scroll_step || arg_scroll_conservatively || temp_scroll_step)
     /* Compute how much we should try to scroll maximally to bring
@@ -13065,13 +13072,10 @@
          /* Compute how many pixels below window bottom to stop searching
             for PT.  This avoids costly search for PT that is far away if
             the user limited scrolling by a small number of lines, but
-            always finds PT if arg_scroll_conservatively is set to a large
+            always finds PT if scroll_conservatively is set to a large
             number, such as most-positive-fixnum.  */
          int slack = max (scroll_max, 10 * FRAME_LINE_HEIGHT (f));
-         int y_to_move =
-           slack >= INT_MAX - it.last_visible_y
-           ? INT_MAX
-           : it.last_visible_y + slack;
+         int y_to_move = it.last_visible_y + slack;
 
          /* Compute the distance from the scroll margin to PT or to
             the scroll limit, whichever comes first.  This should
@@ -13110,6 +13114,10 @@
              amount_to_scroll = float_amount;
              if (amount_to_scroll == 0 && float_amount > 0)
                amount_to_scroll = 1;
+             /* Don't let point enter the scroll margin near top of
+                the window.  */
+             if (amount_to_scroll > height - 2*this_scroll_margin + dy)
+               amount_to_scroll = height - 2*this_scroll_margin + dy;
            }
        }
 
@@ -13117,12 +13125,12 @@
        return SCROLLING_FAILED;
 
       start_display (&it, w, startp);
-      if (scroll_max < INT_MAX)
+      if (arg_scroll_conservatively <= scroll_limit)
        move_it_vertically (&it, amount_to_scroll);
       else
        {
          /* Extra precision for users who set scroll-conservatively
-            to most-positive-fixnum: make sure the amount we scroll
+            to a large number: make sure the amount we scroll
             the window start is never less than amount_to_scroll,
             which was computed as distance from window bottom to
             point.  This matters when lines at window top and lines
@@ -13160,16 +13168,19 @@
        {
          /* Point is in the scroll margin at the top of the window or
             above what is displayed in the window.  */
-         int y0;
+         int y0, y_to_move;
 
          /* Compute the vertical distance from PT to the scroll
-            margin position.  Give up if distance is greater than
-            scroll_max.  */
+            margin position.  Move as far as scroll_max allows, or
+            one screenful, or 10 screen lines, whichever is largest.
+            Give up if distance is greater than scroll_max.  */
          SET_TEXT_POS (pos, PT, PT_BYTE);
          start_display (&it, w, pos);
          y0 = it.current_y;
+         y_to_move = max (it.last_visible_y,
+                          max (scroll_max, 10 * FRAME_LINE_HEIGHT (f)));
          move_it_to (&it, CHARPOS (scroll_margin_pos), 0,
-                     it.last_visible_y, -1,
+                     y_to_move, -1,
                      MOVE_TO_POS | MOVE_TO_X | MOVE_TO_Y);
          dy = it.current_y - y0;
          if (dy > scroll_max)
@@ -13179,8 +13190,8 @@
          start_display (&it, w, startp);
 
          if (arg_scroll_conservatively)
-           amount_to_scroll
-             = max (dy, FRAME_LINE_HEIGHT (f) * max (scroll_step, 
temp_scroll_step));
+           amount_to_scroll = max (dy, FRAME_LINE_HEIGHT (f) *
+                                   max (scroll_step, temp_scroll_step));
          else if (scroll_step || temp_scroll_step)
            amount_to_scroll = scroll_max;
          else
@@ -13193,6 +13204,12 @@
                  amount_to_scroll = float_amount;
                  if (amount_to_scroll == 0 && float_amount > 0)
                    amount_to_scroll = 1;
+                 amount_to_scroll -=
+                   this_scroll_margin - dy - FRAME_LINE_HEIGHT (f);
+                 /* Don't let point enter the scroll margin near
+                    bottom of the window.  */
+                 if (amount_to_scroll > height - 2*this_scroll_margin + dy)
+                   amount_to_scroll = height - 2*this_scroll_margin + dy;
                }
            }
 
@@ -14190,11 +14207,10 @@
        }
     }
 
-  /* Finally, just choose place to start which centers point */
+  /* Finally, just choose a place to start which positions point
+     according to user preferences.  */
 
  recenter:
-  if (centering_position < 0)
-    centering_position = window_box_height (w) / 2;
 
 #if GLYPH_DEBUG
   debug_method_add (w, "recenter");
@@ -14206,10 +14222,77 @@
   if (!buffer_unchanged_p)
     w->base_line_number = Qnil;
 
-  /* Move backward half the height of the window.  */
+  /* Determine the window start relative to point.  */
   init_iterator (&it, w, PT, PT_BYTE, NULL, DEFAULT_FACE_ID);
   it.current_y = it.last_visible_y;
+  if (centering_position < 0)
+    {
+      int margin =
+       scroll_margin > 0
+       ? min (scroll_margin, WINDOW_TOTAL_LINES (w) / 4)
+       : 0;
+      EMACS_INT margin_pos = CHARPOS (startp);
+      int scrolling_up;
+      Lisp_Object aggressive;
+
+      /* If there is a scroll margin at the top of the window, find
+        its character position.  */
+      if (margin)
+       {
+         struct it it1;
+
+         start_display (&it1, w, startp);
+         move_it_vertically (&it1, margin);
+         margin_pos = IT_CHARPOS (it1);
+       }
+      scrolling_up = PT > margin_pos;
+      aggressive =
+       scrolling_up
+       ? BVAR (current_buffer, scroll_up_aggressively)
+       : BVAR (current_buffer, scroll_down_aggressively);
+
+      if (!MINI_WINDOW_P (w)
+         && (scroll_conservatively > SCROLL_LIMIT || NUMBERP (aggressive)))
+       {
+         int pt_offset = 0;
+
+         /* Setting scroll-conservatively overrides
+            scroll-*-aggressively.  */
+         if (!scroll_conservatively && NUMBERP (aggressive))
+           {
+             double float_amount = XFLOATINT (aggressive);
+
+             pt_offset = float_amount * WINDOW_BOX_TEXT_HEIGHT (w);
+             if (pt_offset == 0 && float_amount > 0)
+               pt_offset = 1;
+             if (pt_offset)
+               margin -= 1;
+           }
+         /* Compute how much to move the window start backward from
+            point so that point will be displayed where the user
+            wants it.  */
+         if (scrolling_up)
+           {
+             centering_position = it.last_visible_y;
+             if (pt_offset)
+               centering_position -= pt_offset;
+             centering_position -=
+               FRAME_LINE_HEIGHT (f) * (1 + margin + (last_line_misfit != 0));
+             /* Don't let point enter the scroll margin near top of
+                the window.  */
+             if (centering_position < margin * FRAME_LINE_HEIGHT (f))
+               centering_position = margin * FRAME_LINE_HEIGHT (f);
+           }
+         else
+           centering_position = margin * FRAME_LINE_HEIGHT (f) + pt_offset;
+       }
+      else
+       /* Set the window start half the height of the window backward
+          from point.  */
+       centering_position = window_box_height (w) / 2;
+    }
   move_it_vertically_backward (&it, centering_position);
+
   xassert (IT_CHARPOS (it) >= BEGV);
 
   /* The function move_it_vertically_backward may move over more
@@ -14226,8 +14309,9 @@
 
   it.current_x = it.hpos = 0;
 
-  /* Set startp here explicitly in case that helps avoid an infinite loop
-     in case the window-scroll-functions functions get errors.  */
+  /* Set the window start position here explicitly, to avoid an
+     infinite loop in case the functions in window-scroll-functions
+     get errors.  */
   set_marker_both (w->start, Qnil, IT_CHARPOS (it), IT_BYTEPOS (it));
 
   /* Run scroll hooks.  */
@@ -26504,6 +26588,10 @@
 onto the screen again.  If that cannot be done, then redisplay
 recenters point as usual.
 
+If the value is greater than 100, redisplay will never recenter point,
+but will always scroll just enough text to bring point into view, even
+if you move far away.
+
 A value of zero means always recenter point if it moves off screen.  */);
   scroll_conservatively = 0;