From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Alan Mackenzie Newsgroups: gmane.emacs.devel Subject: Re: Scrolling and follow-mode Date: Sun, 27 Apr 2014 11:52:15 +0000 Message-ID: <20140427115215.GB3091@acm.acm> References: <20140421161751.GC4266@acm.acm> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1398599801 23896 80.91.229.3 (27 Apr 2014 11:56:41 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sun, 27 Apr 2014 11:56:41 +0000 (UTC) Cc: emacs-devel@gnu.org To: Anders Lindgren Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sun Apr 27 13:56:34 2014 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1WeNhC-0003xN-6v for ged-emacs-devel@m.gmane.org; Sun, 27 Apr 2014 13:56:34 +0200 Original-Received: from localhost ([::1]:38903 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WeNhB-00075A-SV for ged-emacs-devel@m.gmane.org; Sun, 27 Apr 2014 07:56:33 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:44301) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WeNh1-00074q-JZ for emacs-devel@gnu.org; Sun, 27 Apr 2014 07:56:31 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WeNgu-00007d-2k for emacs-devel@gnu.org; Sun, 27 Apr 2014 07:56:23 -0400 Original-Received: from colin.muc.de ([193.149.48.1]:59261 helo=mail.muc.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WeNgt-00006Z-Po for emacs-devel@gnu.org; Sun, 27 Apr 2014 07:56:16 -0400 Original-Received: (qmail 83899 invoked by uid 3782); 27 Apr 2014 11:56:13 -0000 Original-Received: from acm.muc.de (pD9518658.dip0.t-ipconnect.de [217.81.134.88]) by colin.muc.de (tmda-ofmipd) with ESMTP; Sun, 27 Apr 2014 13:56:12 +0200 Original-Received: (qmail 4174 invoked by uid 1000); 27 Apr 2014 11:52:15 -0000 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-Delivery-Agent: TMDA/1.1.12 (Macallan) X-Primary-Address: acm@muc.de X-detected-operating-system: by eggs.gnu.org: FreeBSD 8.x X-Received-From: 193.149.48.1 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:171636 Archived-At: 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).