bug#18545: 24.4.50: Bug - forward-line inside with-selected-window

[Eli Zaretskii]

> Date: Mon, 29 Sep 2014 09:16:22 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 18545@debbugs.gnu.org
> 
> > Date: Mon, 29 Sep 2014 02:31:04 +0200 (CEST)
> > From: lompik@voila.fr
> > Cc: 18545@debbugs.gnu.org
> > 
> > C-x C-f -> in a folder with lots of files
> > [tab]x2 -> open completion buffer
> > C-! -> modify completion buffer
> > C-h k a -> Show help for letter "a" (Important ! skipping this results in 
> > no bug ) 
> > C-` until bottom of *Completions* buffer -> no scrolling,
> > 
> > 
> > I have applied the 3 patches that you supplied on emacs bzr revno 117517. I 
> > have tested this on 24.3.93 as well.
> 
> Thanks.  This is indeed very similar to what Martin presented,
> i.e. forward-line moves point outside of the window, and then
> redisplay returns point back into the view.  But since the change that
> fixed Martin's case doesn't fix this one, I guess some other place in
> the code is setting the force_start flag of the window.
> 
> I will look into this.

It's a god-awful mess.  In this use case, we enter that code not
because of the force_start flag, but because of the frame's
frozen_window_starts flag.  That flag is set when we resize the
minibuffer window, and it currently acts almost like the force_start
flag.  (There's a function window_frozen_p, which attempts to exempt
some special windows from this "frozen-start" state, but it cannot
handle this complicated case, because by the time redisplay kicks in,
the fact that *Completions* was at some point displayed by the
selected window is long forgotten, and its window is now just like any
other one.)

The best solution I can propose is in the patch below, which is a
cumulative patch against the emacs-24 branch, and is supposed to fix
all of these problems.  It fixes this last problem by treating the
frozen_window_starts flag as advisory, i.e. like we treat the
optional_new_start flag of a window.

Please test it.  I will commit it later today (to get it into the next
pretest).

=== modified file 'src/window.c'
--- src/window.c        2014-09-11 08:47:34 +0000
+++ src/window.c        2014-09-29 06:03:36 +0000
@@ -5897,6 +5897,8 @@ and redisplay normally--don't erase and 
   w->start_at_line_beg = (bytepos == BEGV_BYTE ||
                          FETCH_BYTE (bytepos - 1) == '\n');
 
+  wset_redisplay (w);
+
   set_buffer_internal (obuf);
   return Qnil;
 }

=== modified file 'src/xdisp.c'
--- src/xdisp.c 2014-09-18 15:10:33 +0000
+++ src/xdisp.c 2014-09-29 13:13:42 +0000
@@ -15027,6 +15027,10 @@ run_window_scroll_functions (Lisp_Object
    If FORCE_P is non-zero, return 0 even if partial visible cursor row
    is higher than window.
 
+   If CURRENT_MATRIX_P is non-zero, use the information from the
+   window's current glyph matrix; otherwise uze the desired glyph
+   matrix.
+
    A value of 0 means the caller should do scrolling
    as if point had gone off the screen.  */
 
@@ -16136,26 +16140,47 @@ redisplay_window (Lisp_Object window, bo
 
   /* If someone specified a new starting point but did not insist,
      check whether it can be used.  */
-  if (w->optional_new_start
+  if ((w->optional_new_start || window_frozen_p (w))
       && CHARPOS (startp) >= BEGV
       && CHARPOS (startp) <= ZV)
     {
+      ptrdiff_t it_charpos;
+
       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;
+      /* Record IT's position now, since line_bottom_y might change
+        that.  */
+      it_charpos = IT_CHARPOS (it);
+      /* Make sure we set the force_start flag only if the cursor row
+        will be fully visible.  Otherwise, the code under force_start
+        label below will try to move point back into view, which is
+        not what the code which sets optional_new_start wants.  */
+      if (line_bottom_y (&it) < it.last_visible_y && !w->force_start)
+       {
+         if (it_charpos == PT)
+           w->force_start = 1;
+         /* IT may overshoot PT if text at PT is invisible.  */
+         else if (it_charpos > PT && CHARPOS (startp) <= PT)
+           w->force_start = 1;
+#ifdef GLYPH_DEBUG
+         if (w->force_start)
+           {
+             if (window_frozen_p (w))
+               debug_method_add (w, "set force_start from frozen window 
start");
+             else
+               debug_method_add (w, "set force_start from optional_new_start");
+           }
+#endif
+       }
     }
 
  force_start:
 
   /* Handle case where place to start displaying has been specified,
      unless the specified location is outside the accessible range.  */
-  if (w->force_start || window_frozen_p (w))
+  if (w->force_start)
     {
       /* We set this later on if we have to adjust point.  */
       int new_vpos = -1;
@@ -16200,7 +16225,7 @@ redisplay_window (Lisp_Object window, bo
          goto need_larger_matrices;
        }
 
-      if (w->cursor.vpos < 0 && !window_frozen_p (w))
+      if (w->cursor.vpos < 0)
        {
          /* If point does not appear, try to move point so it does
             appear. The desired matrix has been built above, so we
@@ -16293,6 +16318,11 @@ redisplay_window (Lisp_Object window, bo
            }
          */
        }
+      if (w->cursor.vpos < 0 || !cursor_row_fully_visible_p (w, 0, 0))
+       {
+         clear_glyph_matrix (w->desired_matrix);
+         goto try_to_scroll;
+       }
 
 #ifdef GLYPH_DEBUG
       debug_method_add (w, "forced window start");
@@ -16357,7 +16387,8 @@ redisplay_window (Lisp_Object window, bo
               || CHARPOS (startp) == BEGV
               || !window_outdated (w)))
     {
-      int d1, d2, d3, d4, d5, d6;
+      int d1, d2, d5, d6;
+      int rtop, rbot;
 
       /* If first window line is a continuation line, and window start
         is inside the modified region, but the first change is before
@@ -16386,10 +16417,14 @@ redisplay_window (Lisp_Object window, bo
             doing so will move point from its correct position
             instead of scrolling the window to bring point into view.
             See bug#9324.  */
-         && pos_visible_p (w, PT, &d1, &d2, &d3, &d4, &d5, &d6))
+         && pos_visible_p (w, PT, &d1, &d2, &rtop, &rbot, &d5, &d6)
+         && rtop == 0 && rbot == 0)
        {
          w->force_start = 1;
          SET_TEXT_POS_FROM_MARKER (startp, w->start);
+#ifdef GLYPH_DEBUG
+         debug_method_add (w, "recomputed window start in continuation line");
+#endif
          goto force_start;
        }