Re: Scrolling and follow-mode

[Alan Mackenzie]

Hi, Anders.

On Tue, Apr 22, 2014 at 10:20:46PM +0200, Anders Lindgren wrote:
> When it comes to the patch, it looks well written. I have only two comments:

> * The documentation says that `follow-fixed-window` is the window that must
> not be scrolled. However, in the code, it's only read once, as the
> condition to a `when`, so the actual value doesn't matter as long as it's
> non-nil. Wouldn't it better do document is a boolean?

Yes, I think so too.  I've changed it to a boolean.

> * The code at (your) lines --- 1201,1222 ---- could be cleaner. It contains
> a `let*` that sets two variables, `dest` and `windows` and set a bunch
> others to a default nil value. The others are later set using a `setq`. I
> would suggest placing your code before the first assignment of `dest` and
> simply move the point, that way `dest` will get the value of the new point.
> Then use a new `let*` to set the others. Like the following (untested) code:

> (let ((windows (follow-all-followers win)))
>   (when follow-fixed-window
>     (follow-debug-message "fixed")
>     (follow-redisplay windows win)
>     (goto-char (follow-get-scrolled-point (point) windows))
>     (setq follow-fixed-window nil))
>   (let* ((dest (point))
>          (win-start-end (progn
>                           (follow-update-window-start (car windows))
>                           (follow-windows-start-end windows)))
>          (aligned (follow-windows-aligned-p win-start-end))
>          (visible (follow-pos-visible dest win win-start-end))
>          selected-window-up-to-date)
>     (unless (and aligned visible)
>       (setq follow-windows-start-end-cache nil))

I've incorporated this change.  (In my own personal copy, I mean).  It is
indeed neater.

However, things aren't as simple as I thought.  In follow-scroll-up, if
point is at EOB, and at the top of the last window, (follow-scroll-up 1)
should scroll EOB into the previous window.  Instead it throws an
'end-of-buffer exception.

I've fixed this by using follow-previous-window and trying scroll-up
again.  However, the fix contains nested `condition-case's in a while
loop, and I'm not totally happy with it.  It's too "clever", really.  (I
also need to put this fix into follow-scroll-down for negative ARGs.)

Here is what the change to follow-scroll-up currently looks like:

@@ -467,9 +519,23 @@
   (interactive "P")
   (cond ((not follow-mode)
         (scroll-up arg))
+       ((eq arg '-)
+        (follow-scroll-down))
        (arg
-        (save-excursion (scroll-up arg))
-        (setq follow-internal-force-redisplay t))
+        (let ((opoint (point))  (owin (selected-window)))
+          (while
+              ;; If we are too near EOB, try scrolling the previous window.
+              (condition-case nil (progn (scroll-up arg) nil)
+                (end-of-buffer 
+                 (condition-case nil (progn (follow-previous-window) t)
+                   (error
+                    (select-window owin)
+                    (goto-char opoint)
+                    (signal 'end-of-buffer nil))))))
+          (unless (and scroll-preserve-screen-position
+                       (get this-command 'scroll-command))
+            (goto-char opoint))
+          (setq follow-fixed-window t)))
        (t
         (let* ((windows (follow-all-followers))
                (end (window-end (car (reverse windows)))))

#########################################################################

As a matter of interest, I mainly use a Linux tty for Emacs, and here
there is never a partially visible line at the bottom of the window.  I
think in a GUI environment, there almost always is.  While solving the
above issue, I stumbled over something in follow-calc-win-end: the flag
END-OF-BUFFER becomes t when EOB is at (window-end win), which is one
line beyond the end of the window (in a tty).  I think the flag is meant
to mean "EOB is fully visible in WIN"; it is commented thus somewhere
else in the file.  I've changed follow-calc-win-end as follows:

@@ -766,15 +842,16 @@
 Return (END-POS END-OF-BUFFER).
 
 Actually, the position returned is the start of the line after
-the last fully-visible line in WIN.  If WIN is nil, the selected
-window is used."
+the last fully-visible line in WIN.  END-OF-BUFFER is t when EOB
+is fully-visible in WIN.  If WIN is nil, the selected window is
+used."
   (let* ((win (or win (selected-window)))
         (edges (window-inside-pixel-edges win))
         (ht (- (nth 3 edges) (nth 1 edges)))
         (last-line-pos (posn-point (posn-at-x-y 0 (1- ht) win))))
     (if (pos-visible-in-window-p last-line-pos win)
        (let ((end (window-end win t)))
-         (list end (= end (point-max))))
+         (list end (pos-visible-in-window-p (point-max) win)))
       (list last-line-pos nil))))
 
 (defun follow-calc-win-start (windows pos win)

.

#########################################################################

With 3 follow windows, and the last few lines of text filling the middle
window, with just EOB in the right hand window, I encountered this
problem: put point at the end of the middle window (i.e. one character
before EOB) and do C-f.  Point moves into the RH window, but the middle
and left windows scroll up 2 lines.

I tracked this down to follow-estimate-first-window-start (called from
follow-redisplay) which decrements the height of the current window
whilst moving vertically over it.  In the above case, this left point two
lines too low, causing the unwanted scrolling.

So I patched the -1 out of the function, and it works, at least on the
tty.  In the GUI, it is still scrolling 1 line up.  I'll try and track
down what's causing this.

Here is the change I've made to follow-estimate-first-window-start:

@@ -1008,7 +1085,7 @@
       (goto-char start)
       (vertical-motion 0 win)
       (dolist (w windows-before)
-       (vertical-motion (- 1 (window-text-height w)) w))
+       (vertical-motion (- (window-text-height w)) w))
       (point))))



> Again, thanks!

> Sincerely,
>     Anders Lindgren

-- 
Alan Mackenzie (Nuremberg, Germany).