Re: The unwarranted scrolling assumption

[Lennart Borgman <lennart.borgman@gmail.com>]

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

On Thu, Jun 17, 2010 at 5:59 AM, Lennart Borgman
<lennart.borgman@gmail.com> wrote:
> On Thu, Jun 17, 2010 at 5:10 AM, Lennart Borgman
> <lennart.borgman@gmail.com> wrote:
>> On Thu, Jun 17, 2010 at 5:05 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>>>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>>>> Cc: Lennart Borgman <lennart.borgman@gmail.com>,  storm@cua.dk,  emacs-devel@gnu.org,  david@harpegolden.net
>>>> Date: Wed, 16 Jun 2010 20:54:52 -0400
>>>>
>>>> Indeed what he suggests is probably impossible.  But the end result he's
>>>> after is definitely possible: move the window-start as little as possible
>>>> (tho still, line by line) to keep point displayed.
>>>
>>> xdisp.c already tries to do that, we just need to find out why it
>>> fails in this particular case.
>>
>> After trying to restore the clip_changed value when save-restriction,
>> widen etc unnecessarily changed I suddenly wonder why they changed it.
>> Is not this something that only redisplay should handle? redisplay_can
>> easily save the clipping when it sets clip_changed to 0 and compare
>> with that when it enters redisplay_window and set clip_changed to 1 if
>> the saved clipping does not match.
>>
>> Does that sound reasonable? (I do not know if I am missing something.)
>
> At least it works. I just tested. First time I am using Emacs without
> "jumping scrolling"! :-)
>
> If you think this is the right way to fix it I will send patches later
> when I cleaned up a bit.
>
> There is one more similar problem which I do not know how to handle,
> in fileio.c. In the function Finsert_file_contents clip_changed is set
> to 1. I am not quite sure of the impact of this but it looks wrong to
> me to do this. I think the display engine should be told, but it must
> decide what to do with redisplay, not fileio.

Here is the patch (without the fileio.c part which I leave until I got
some comments).

[-- Attachment #2: buffer-h-jumpscroll-0.diff --]
[-- Type: application/octet-stream, Size: 514 bytes --]

=== modified file 'src/buffer.h'
--- trunk/src/buffer.h	2010-04-20 17:01:39 +0000
+++ patched/src/buffer.h	2010-06-17 04:48:01 +0000
@@ -541,6 +541,9 @@
 
   /* Set nonzero whenever the narrowing is changed in this buffer.  */
   int clip_changed;
+  /* Set to the narrowing in effect when clip_changed is set to 0. */
+  EMACS_INT clip_begv;
+  EMACS_INT clip_zv;
 
   /* If the long line scan cache is enabled (i.e. the buffer-local
      variable cache-long-line-scans is non-nil), newline_cache


[-- Attachment #3: editfns-jumpscroll-0.diff --]
[-- Type: application/octet-stream, Size: 1508 bytes --]

=== modified file 'src/editfns.c'
--- trunk/src/editfns.c	2010-06-10 05:31:36 +0000
+++ patched/src/editfns.c	2010-06-17 04:07:10 +0000
@@ -3215,12 +3221,13 @@
      ()
 {
   if (BEG != BEGV || Z != ZV)
-    current_buffer->clip_changed = 1;
+    {
   BEGV = BEG;
   BEGV_BYTE = BEG_BYTE;
   SET_BUF_ZV_BOTH (current_buffer, Z, Z_BYTE);
   /* Changing the buffer bounds invalidates any recorded current column.  */
   invalidate_current_column ();
+    }
   return Qnil;
 }
 
@@ -3249,8 +3256,7 @@
     args_out_of_range (start, end);
 
   if (BEGV != XFASTINT (start) || ZV != XFASTINT (end))
-    current_buffer->clip_changed = 1;
-
+    {
   SET_BUF_BEGV (current_buffer, XFASTINT (start));
   SET_BUF_ZV (current_buffer, XFASTINT (end));
   if (PT < XFASTINT (start))
@@ -3259,6 +3265,7 @@
     SET_PT (XFASTINT (end));
   /* Changing the buffer bounds invalidates any recorded current column.  */
   invalidate_current_column ();
+    }
   return Qnil;
 }
 
@@ -3327,8 +3334,6 @@
 			     clip_to_bounds (beg->charpos, pt, end->charpos),
 			     clip_to_bounds (beg->bytepos, BUF_PT_BYTE (buf),
 					     end->bytepos));
-
-	  buf->clip_changed = 1; /* Remember that the narrowing changed. */
 	}
     }
   else
@@ -3340,8 +3345,6 @@
 	{
 	  SET_BUF_BEGV_BOTH (buf, BUF_BEG (buf), BUF_BEG_BYTE (buf));
 	  SET_BUF_ZV_BOTH (buf, BUF_Z (buf), BUF_Z_BYTE (buf));
-
-	  buf->clip_changed = 1; /* Remember that the narrowing changed. */
 	}
     }
 


[-- Attachment #4: xdisp-jumpscroll-0.diff --]
[-- Type: application/octet-stream, Size: 822 bytes --]

=== modified file 'src/xdisp.c'
--- trunk/src/xdisp.c	2010-06-01 02:34:49 +0000
+++ patched/src/xdisp.c	2010-06-17 04:14:21 +0000
@@ -11716,7 +11716,11 @@
 	   && w->current_matrix->buffer == b
 	   && w->current_matrix->zv == BUF_ZV (b)
 	   && w->current_matrix->begv == BUF_BEGV (b))
+    {
     b->clip_changed = 0;
+      b->clip_begv = b->begv;
+      b->clip_zv = b->zv;
+    }
 
   /* If display wasn't paused, and W is not a tool bar window, see if
      point has been moved into or out of a composition.  In that case,
@@ -14055,6 +14059,9 @@
   int last_line_misfit = 0;
   int beg_unchanged, end_unchanged;
 
+  if ((buffer->clip_begv != buffer->begv) || (buffer->clip_zv != buffer->zv))
+    buffer->clip_changed = 1;
+
   SET_TEXT_POS (lpoint, PT, PT_BYTE);
   opoint = lpoint;