Re: "... the window start at a meaningless point within a line."

[Eli Zaretskii]

> Date: Wed, 28 Oct 2015 13:15:05 +0000
> Cc: address@hidden
> From: Alan Mackenzie <address@hidden>
> 
> After applying the patch, to test things, in X (?or windows)
> o - Start emacs -Q.
> o - Load utilities.el.
> o - Visit fragment.el.
> o - Using the mouse, make the frame wide enough for two side by side
>   windows of width 79 and 80.
> o - C-x 3.
> o - Fiddle about with the mouse, M-{, M-}, until the windows are 79, and
>   80 wide (`window-body-width' is your friend, here).
> o - Scroll the buffer until L162 straddles window 1 and window 2, with
>   two display lines in each window.  (S-<up>/<down>, C-S-<up>/<down>
>   from utilies.el are handy, here).
> 
> First of all, note that in L162, all characters are displayed exactly
> once.  This is the purpose of this change.
> 
> With point in W2, note that C-<left> doesn't go to the beginning of
> the window, and C-<right> doesn't go to the end.  The actual end
> positions are a line out in both cases.  With point in the split line,
> note that vertical-motion (C-u <n> C-c m) doesn't go to the right
> places, even when the target line is below the split line.  Typing <up>
> and <down> also produce interesting effects.
> 
> Using M-{ or M-}, make W1 80 wide, and W2 79 wide.  Scroll the windows
> up and down, back to the same place, to ensure that Follow Mode has
> resynchronised its windows[*].  Now repeat the experiments of the
> previous paragraph.  If anything, the results now are even worse.

The patch below should go a long way towards solving these problems.
The only issue I left unsolved is exposed when typing C-p on the
second screen line of the right-side window: you will see that it
doesn't succeed in keeping the visual column.  Solving this is left as
an exercise ;-)

I hope the changes I made and the comments to those changes will speak
for themselves, and show how to solve such problems.

Please note that there's a very serious underlying problem here: the
move_it_* functions and all their callers assume that moving beyond
above or below the window keeps the same window dimensions, in
particularly its width.  Follow mode violates this assumption, because
moving from the left-side to the right-side window or vice versa can
change the width.  Accounting for this would need very deep changes in
the above mentioned functions.  For now, I avoided that, and the patch
below is just a band-aid.  If it works well enough in practice, maybe
we can avoid the surgery.  But in general, I must say that fixing what
you are trying to fix is an ambitious undertaking; perhaps it would be
better to make sure the two windows are always the same width, using
pixel-wise resizing where necessary.

diff --git a/src/indent.c b/src/indent.c
index 32597bb..993ef5f 100644
--- a/src/indent.c
+++ b/src/indent.c
@@ -2034,6 +2034,7 @@ whether or not it is currently displayed in some window.  
*/)
   else
     {
       ptrdiff_t it_start, it_overshoot_count = 0;
+      ptrdiff_t w_start = marker_position (w->start);
       int first_x;
       bool overshoot_handled = 0;
       bool disp_string_at_start_p = 0;
@@ -2043,6 +2044,8 @@ whether or not it is currently displayed in some window.  
*/)
       int start_x IF_LINT (= 0);
       int to_x = -1;
       struct text_pos xdisp_ws;
+      bool on_window_border = false;
+      ptrdiff_t goal_pt = PT;
 
       bool start_x_given = !NILP (cur_col);
 
@@ -2102,27 +2105,40 @@ whether or not it is currently displayed in some 
window.  */)
             do this, we start moving with IT->current_x == 0, while PT is
             really at some x > 0.  */
          reseat_at_previous_visible_line_start (&it);
+         /* If we are in a right-side window under follow-mode, and
+            previous visible line start is before that window's
+            start, reseat to the window's start point instead, where
+            we know the horizontal coordinates are zero.  This avoids
+            problems in iterator metrics when the 2 windows under
+            follow-mode have different widths.  */
+         if (w->exact_start
+             && IT_CHARPOS (it) < w_start)
+           {
+             on_window_border = true;
+             reseat_at_window_start (&it);
+           }
          it.current_x = it.hpos = 0;
        }
       if (IT_CHARPOS (it) != PT)
-       /* We used to temporarily disable selective display here; the
-          comment said this is "so we don't move too far" (2005-01-19
-          checkin by kfs).  But this does nothing useful that I can
-          tell, and it causes Bug#2694 .  -- cyd */
-       /* When the position we started from is covered by a display
-          string, move_it_to will overshoot it, while vertical-motion
-          wants to put the cursor _before_ the display string.  So in
-          that case, we move to buffer position before the display
-          string, and avoid overshooting.  But if the position before
-          the display string is a newline, we don't do this, because
-          otherwise we will end up in a screen line that is one too
-          far back.  */
-       move_it_to (&it,
-                   (!disp_string_at_start_p
-                    || FETCH_BYTE (IT_BYTEPOS (it)) == '\n')
-                   ? PT
-                   : PT - 1,
-                   -1, -1, -1, MOVE_TO_POS);
+       {
+         /* We used to temporarily disable selective display here; the
+            comment said this is "so we don't move too far" (2005-01-19
+            checkin by kfs).  But this does nothing useful that I can
+            tell, and it causes Bug#2694 .  -- cyd */
+         /* When the position we started from is covered by a display
+            string, move_it_to will overshoot it, while vertical-motion
+            wants to put the cursor _before_ the display string.  So in
+            that case, we move to buffer position before the display
+            string, and avoid overshooting.  But if the position before
+            the display string is a newline, we don't do this, because
+            otherwise we will end up in a screen line that is one too
+            far back.  */
+         goal_pt =
+           (!disp_string_at_start_p || FETCH_BYTE (IT_BYTEPOS (it)) == '\n')
+           ? PT
+           : PT - 1;
+         move_it_to (&it, goal_pt, -1, -1, -1, MOVE_TO_POS);
+       }
 
       /* IT may move too far if truncate-lines is on and PT lies
         beyond the right margin.  IT may also move too far if the
@@ -2176,7 +2192,9 @@ whether or not it is currently displayed in some window.  
*/)
          /* Do this even if LINES is 0, so that we move back to the
             beginning of the current line as we ought.  */
          if ((nlines < 0 && IT_CHARPOS (it) > 0)
-             || (nlines == 0 && !(start_x_given && start_x <= to_x)))
+             || (nlines == 0
+                 && !(start_x_given && start_x <= to_x)
+                 && !on_window_border))
            move_it_by_lines (&it, max (PTRDIFF_MIN, nlines));
        }
       else if (overshoot_handled)
@@ -2212,6 +2230,35 @@ whether or not it is currently displayed in some window. 
 */)
            }
        }
 
+      /* If we moved up in the right-side window under follow-mode,
+        and found ourselves beyond the window's start, we must move
+        back from window start to PT, see how many lines is that,
+        then switch to the left-side window, and move back the rest
+        of NLINES starting from that window's last line.  That's
+        because move_it_* functions use the window dimensions, which
+        might be different in the other window.  */
+      if (w->exact_start && IT_CHARPOS (it) < w_start)
+       {
+         int vpos = it.vpos, nlines_prev_window;
+         Lisp_Object prev_window = w->prev;
+         struct window *pw;
+         ptrdiff_t prev_window_endpos;
+         struct text_pos pw_top;
+
+         eassert (WINDOW_LIVE_P (prev_window));
+         reseat_at_window_start (&it);
+         it.current_x = it.hpos = it.vpos = 0;
+         move_it_to (&it, goal_pt, -1, -1, -1, MOVE_TO_POS);
+         nlines_prev_window = nlines + it.vpos + 1;
+         pw = XWINDOW (prev_window);
+         SET_TEXT_POS_FROM_MARKER (pw_top, pw->start);
+         start_display (&it, pw, pw_top);
+         move_it_to (&it, -1, -1, it.last_visible_y - 1, -1, MOVE_TO_Y);
+         move_it_by_lines (&it, nlines_prev_window);
+         /* Pretend the original move worked as intended.  */
+         it.vpos = vpos;
+       }
+
       /* Move to the goal column, if one was specified.  If the window
         was originally hscrolled, the goal column is interpreted as
         an addition to the hscroll amount.  */
diff --git a/src/window.c b/src/window.c
index b922511..38aadd6 100644
--- a/src/window.c
+++ b/src/window.c
@@ -7149,7 +7149,7 @@ and scrolling positions.  */)
 
 DEFUN ("window-test-dump", Fwindow_test_dump, Swindow_test_dump, 0, 0, "",
        doc: /* Dump some critical components of the selected window to 
`message'.*/)
-  ()
+  (void)
 {
   Lisp_Object window = Fselected_window ();
   struct window *w = decode_live_window (window);