unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#23347: 25.0.92; Follow mode scrolling broken -- scrolls only one page
@ 2016-04-23 17:25 Anders Lindgren
       [not found] ` <mailman.843.1461432366.7477.bug-gnu-emacs@gnu.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Anders Lindgren @ 2016-04-23 17:25 UTC (permalink / raw)
  To: 23347

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

Hi!

In Emacs 25.0.92, the command `follow-scroll-up' and `follow-scroll-down'
only scroll one page. The intention is that they should scroll as many
pages as there are windows in the window group (i.e. the number of windows
showing the same buffer in the current frame, when follow-mode is enabled),
as was done in Emacs 24.

Steps to repeat:

    emacs -Q
    C-h t
    M-x follow-delete-other-windows-and-split RET
      ;; Page 1 and 2 are visible.
    C-c . C-v
      ;; Now, page 2 and 3 are visible.

I expect page 3 and 4 to be visible.

The idea behind `follow-scroll-up' and `follow-scroll-down' is to scroll
away the entire window group at once. For example, if you have six side by
side windows, `follow-scroll-up' should go from displaying page 1 .. 6 to 7
.. 12. Currently Emacs 25 go from 1 .. 6 to 2 .. 7.

    -- Anders

Ps. I'm the original author of Follow mode, but I haven't been involved in
recent development.


In GNU Emacs 25.0.92.6 (x86_64-apple-darwin14.5.0, NS appkit-1348.17
Version 10.10.5 (Build 14F1713))
 of 2016-04-10 built on macpro.lan
Repository revision: 0e7bcec103073eceecc2621c19e4d290b2d97e8d
Windowing system distributor 'Apple', version 10.3.1348
Configured using:
 'configure --with-ns --without-dbus'

Configured features:
NOTIFY ACL ZLIB TOOLKIT_SCROLL_BARS NS

Important settings:
  value of $LC_CTYPE: UTF-8
  value of $LANG: en_SE.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Fundamental

Minor modes in effect:
  follow-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Preparing tutorial ...
Resume your last saved tutorial? (y or n) n
You can run the command ‘follow-scroll-up’ with C-c . C-v

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message dired format-spec rfc822 mml
mml-sec password-cache epg epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns mail-prsvr mail-utils follow
tutorial help-mode easymenu cl-loaddefs pcase cl-lib time-date mule-util
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel ns-win ucs-normalize term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list newcomment elisp-mode lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cl-generic cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese charscript case-table epa-hook
jka-cmpr-hook help simple abbrev minibuffer cl-preloaded nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote kqueue cocoa ns multi-tty
make-network-process emacs)

Memory information:
((conses 16 196924 8312)
 (symbols 48 19718 0)
 (miscs 40 48 196)
 (strings 32 15395 5251)
 (string-bytes 1 449890)
 (vectors 16 33084)
 (vector-slots 8 650906 6707)
 (floats 8 158 31)
 (intervals 56 196 0)
 (buffers 976 12))

[-- Attachment #2: Type: text/html, Size: 4664 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#23347: 25.0.92; Follow mode scrolling broken -- scrolls only one page
       [not found] ` <mailman.843.1461432366.7477.bug-gnu-emacs@gnu.org>
@ 2016-04-24  9:00   ` Alan Mackenzie
  2016-04-24 12:36     ` Anders Lindgren
  0 siblings, 1 reply; 8+ messages in thread
From: Alan Mackenzie @ 2016-04-24  9:00 UTC (permalink / raw)
  To: Anders Lindgren; +Cc: 23347

Hello, Anders.

In article <mailman.843.1461432366.7477.bug-gnu-emacs@gnu.org> you wrote:
> [-- text/plain, encoding quoted-printable, charset: UTF-8, 106 lines --]

> Hi!

> In Emacs 25.0.92, the command `follow-scroll-up' and `follow-scroll-down'
> only scroll one page. The intention is that they should scroll as many
> pages as there are windows in the window group (i.e. the number of windows
> showing the same buffer in the current frame, when follow-mode is enabled),
> as was done in Emacs 24.

It was me who constructed `follow-scroll-up/down' as they currently are.

It seemed "obvious" to me that scrolling one page at a time was the Right
Thing to do - this enables the user to move, for example, text on the
right hand screen to the middle or left, so that she can see it together
with either the text above or the text below.  Also, three C-v's are
easier to type than a third of a C-v.

I admit it didn't occur to me that there might be use cases other than my
own.  What I think we need here is a customisable option.  How about
`follow-scroll-single-page-flag'?  The cleanest default for it would
probably be nil, since that better observes the abstraction that the
collection of all visible windows should be regarded as a single page.

What do you say?

> Steps to repeat:

>     emacs -Q
>     C-h t
>     M-x follow-delete-other-windows-and-split RET
>       ;; Page 1 and 2 are visible.
>     C-c . C-v
>       ;; Now, page 2 and 3 are visible.

> I expect page 3 and 4 to be visible.

> The idea behind `follow-scroll-up' and `follow-scroll-down' is to scroll
> away the entire window group at once. For example, if you have six side by
> side windows, `follow-scroll-up' should go from displaying page 1 .. 6 to 7
> .. 12. Currently Emacs 25 go from 1 .. 6 to 2 .. 7.

>     -- Anders

> Ps. I'm the original author of Follow mode, but I haven't been involved in
> recent development.



> In GNU Emacs 25.0.92.6 (x86_64-apple-darwin14.5.0, NS appkit-1348.17
> Version 10.10.5 (Build 14F1713))
>  of 2016-04-10 built on macpro.lan
> Repository revision: 0e7bcec103073eceecc2621c19e4d290b2d97e8d
> Windowing system distributor 'Apple', version 10.3.1348
> Configured using:
>  'configure --with-ns --without-dbus'

-- 
Alan Mackenzie (Nuremberg, Germany).






^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#23347: 25.0.92; Follow mode scrolling broken -- scrolls only one page
  2016-04-24  9:00   ` Alan Mackenzie
@ 2016-04-24 12:36     ` Anders Lindgren
  2016-04-24 14:23       ` Alan Mackenzie
  2016-04-29 15:02       ` Alan Mackenzie
  0 siblings, 2 replies; 8+ messages in thread
From: Anders Lindgren @ 2016-04-24 12:36 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 23347

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

Hi!

The normal `scroll-up-command' and `scroll-down-command' (typically bound
to C-v and M-v) can scroll a page at a time, there is no need for a Follow
mode-specific command for that, as the normal Follow mode machinery can
reposition the other windows. (The only difference is that when using C-v
the original window is still selected, whereas with the new
follow-scroll-up command the windows where the original point ends up is
selected.)


I admit it didn't occur to me that there might be use cases other than my
> own.  What I think we need here is a customisable option.  How about
> `follow-scroll-single-page-flag'?  The cleanest default for it would
> probably be nil, since that better observes the abstraction that the
> collection of all visible windows should be regarded as a single page.
>

A cleaner solution would be to define two sets of functions, e.g.
follow-scroll-up-one-page. Many users might want to use both kind of
scrolling (I use both C-v and C-c . C-v several time every day, depending
on how I would like to scroll the display). With two sets of functions it's
easier to do this. (Also, it's easier to write documentation for a function
designed to do one thing.)

    -- Anders

[-- Attachment #2: Type: text/html, Size: 1568 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#23347: 25.0.92; Follow mode scrolling broken -- scrolls only one page
  2016-04-24 12:36     ` Anders Lindgren
@ 2016-04-24 14:23       ` Alan Mackenzie
  2016-04-24 15:37         ` Anders Lindgren
  2016-04-29 15:02       ` Alan Mackenzie
  1 sibling, 1 reply; 8+ messages in thread
From: Alan Mackenzie @ 2016-04-24 14:23 UTC (permalink / raw)
  To: Anders Lindgren; +Cc: 23347

Hello, Anders.

On Sun, Apr 24, 2016 at 02:36:27PM +0200, Anders Lindgren wrote:
> Hi!

> The normal `scroll-up-command' and `scroll-down-command' (typically bound
> to C-v and M-v) can scroll a page at a time, there is no need for a Follow
> mode-specific command for that, as the normal Follow mode machinery can
> reposition the other windows. (The only difference is that when using C-v
> the original window is still selected, whereas with the new
> follow-scroll-up command the windows where the original point ends up is
> selected.)

Indeed.  This is a difference important enough to have a command in
follow.el which keeps point at the same buffer position it started with.

> > I admit it didn't occur to me that there might be use cases other
> > than my own.  What I think we need here is a customisable option.
> > How about `follow-scroll-single-page-flag'?  The cleanest default
> > for it would probably be nil, since that better observes the
> > abstraction that the collection of all visible windows should be
> > regarded as a single page.


> A cleaner solution would be to define two sets of functions, e.g.
> follow-scroll-up-one-page.

OK.  I'm happy enought with that.

I'm not sure that's a good name, though - the "page" bit could easily
get confused with the region in a buffer between two ^L characters.

Maybe follow-scroll-up-window would be better.  What do you think?

> Many users might want to use both kind of scrolling (I use both C-v
> and C-c . C-v several time every day, depending on how I would like to
> scroll the display). With two sets of functions it's easier to do
> this. (Also, it's easier to write documentation for a function
> designed to do one thing.)

For me, C-c . C-v is too tedious to type, so I've just bound <PageUp>
and <PageDown> to `follow-scroll-down/up'.  I use them many times per
day.

Another possibility would be to use a C-u prefix argument to indicate N
windows as opposed to 1.  No, I don't think that's a good idea, either.

Anyway, here is what I think we can agree on:
1. `follow-scroll-up/down' should scroll by N windows.
2. There need to be commands in follow.el that scroll by 1 window as is
  done by the current `follow-scroll-up/down'.  You have suggested the
  name `follow-scroll-up/down-one-page'.  I have countered with
  `follow-scroll-up/down-window'.

If we can agree, I'm willing to amend follow.el.  I think it's probably
too late for this fix to make it into Emacs 25.1, though.

>     -- Anders

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#23347: 25.0.92; Follow mode scrolling broken -- scrolls only one page
  2016-04-24 14:23       ` Alan Mackenzie
@ 2016-04-24 15:37         ` Anders Lindgren
  2016-04-26 11:51           ` Alan Mackenzie
  0 siblings, 1 reply; 8+ messages in thread
From: Anders Lindgren @ 2016-04-24 15:37 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 23347

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

Hi!


> > The normal `scroll-up-command' and `scroll-down-command' (typically bound
> > to C-v and M-v) can scroll a page at a time, there is no need for a
> Follow
> > mode-specific command for that, as the normal Follow mode machinery can
> > reposition the other windows. (The only difference is that when using C-v
> > the original window is still selected, whereas with the new
> > follow-scroll-up command the windows where the original point ends up is
> > selected.)
>
> Indeed.  This is a difference important enough to have a command in
> follow.el which keeps point at the same buffer position it started with.
>

Agreed. All three scroll commands (non-follow-mode scroll, scroll one page,
and scroll N pages) are useful.

> A cleaner solution would be to define two sets of functions, e.g.
> > follow-scroll-up-one-page.
>
> OK.  I'm happy enought with that.
>
> I'm not sure that's a good name, though - the "page" bit could easily
> get confused with the region in a buffer between two ^L characters.
>
> Maybe follow-scroll-up-window would be better.  What do you think?
>

Yes, I agree, `page' isn't a good choice.

What about `follow-scroll-up-keep-point'? That way it would be clear what
the difference is between it and the normal scroll command.

If I would have designed Follow mode today, I probably would have used your
name, `follow-scroll-up-window', and named the N-page scroll
`follow-scroll-up-window-group'.



> > Many users might want to use both kind of scrolling (I use both C-v
> > and C-c . C-v several time every day, depending on how I would like to
> > scroll the display). With two sets of functions it's easier to do
> > this. (Also, it's easier to write documentation for a function
> > designed to do one thing.)
>
> For me, C-c . C-v is too tedious to type, so I've just bound <PageUp>
> and <PageDown> to `follow-scroll-down/up'.  I use them many times per
> day.
>

I've remapped them to C-c C-v and C-c v, that way I can scroll a window
down a single page using C-v and the entire display using C-c C-v. Of
course, this isn't in a part of a keymap available to minor modes, so it's
nothing we can make official.


Another possibility would be to use a C-u prefix argument to indicate N
> windows as opposed to 1.  No, I don't think that's a good idea, either.
>

No, too cumbersome.


Anyway, here is what I think we can agree on:
> 1. `follow-scroll-up/down' should scroll by N windows.
> 2. There need to be commands in follow.el that scroll by 1 window as is
>   done by the current `follow-scroll-up/down'.  You have suggested the
>   name `follow-scroll-up/down-one-page'.  I have countered with
>   `follow-scroll-up/down-window'.
>
> If we can agree, I'm willing to amend follow.el.  I think it's probably
> too late for this fix to make it into Emacs 25.1, though.
>

Well, give it a try. Feel free to pick a suitable name. If we come up with
a clean solution quickly John might accept it.

    -- Anders

[-- Attachment #2: Type: text/html, Size: 4290 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#23347: 25.0.92; Follow mode scrolling broken -- scrolls only one page
  2016-04-24 15:37         ` Anders Lindgren
@ 2016-04-26 11:51           ` Alan Mackenzie
  2016-04-26 13:08             ` Anders Lindgren
  0 siblings, 1 reply; 8+ messages in thread
From: Alan Mackenzie @ 2016-04-26 11:51 UTC (permalink / raw)
  To: Anders Lindgren; +Cc: 23347

Hello, Anders.

On Sun, Apr 24, 2016 at 05:37:57PM +0200, Anders Lindgren wrote:
> Hi!

> Agreed. All three scroll commands (non-follow-mode scroll, scroll one page,
> and scroll N pages) are useful.

[ .... ]

> Yes, I agree, `page' isn't a good choice.

> What about `follow-scroll-up-keep-point'? That way it would be clear what
> the difference is between it and the normal scroll command.

> If I would have designed Follow mode today, I probably would have used your
> name, `follow-scroll-up-window', and named the N-page scroll
> `follow-scroll-up-window-group'.

I've just used `follow-scroll-up-window' and
`follow-scroll-down-window'.  At least, for now.

[ .... ]

> Anyway, here is what I think we can agree on:
> > 1. `follow-scroll-up/down' should scroll by N windows.
> > 2. There need to be commands in follow.el that scroll by 1 window as is
> >   done by the current `follow-scroll-up/down'.  You have suggested the
> >   name `follow-scroll-up/down-one-page'.  I have countered with
> >   `follow-scroll-up/down-window'.

> > If we can agree, I'm willing to amend follow.el.  I think it's probably
> > too late for this fix to make it into Emacs 25.1, though.
> >

> Well, give it a try. Feel free to pick a suitable name. If we come up with
> a clean solution quickly John might accept it.

OK.  I had one or two minor problems with follow-scroll-up/down with a
non-nil ARG.  In the end, I just extracted functionality from
follow-scroll-up/down-window into separate functions, and called those
from all four scrolling functions.

Anyway, here is the patch I've come up with.  Could you review it and
test it, please.  Thanks!


diff --git a/lisp/follow.el b/lisp/follow.el
index 5801f79..3e23247 100644
--- a/lisp/follow.el
+++ b/lisp/follow.el
@@ -532,6 +532,80 @@ follow-get-scrolled-point
 ;; position...  (This would also be corrected if we would have had a
 ;; good redisplay abstraction.)
 
+(defun follow-scroll-up-arg (arg)
+  "Scroll the text in a follow mode window chain up by ARG lines.
+If ARG is nil, scroll the size of the current window.
+
+This is an internal function for `follow-scroll-up' and
+`follow-scroll-up-window'."
+  (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)))
+
+(defun follow-scroll-down-arg (arg)
+  "Scroll the text in a follow mode window chain down by ARG lines.
+If ARG is nil, scroll the size of the current window.
+
+This is an internal function for `follow-scroll-down' and
+`follow-scroll-down-window'."
+  (let ((opoint (point)))
+    (scroll-down arg)
+    (unless (and scroll-preserve-screen-position
+                 (get this-command 'scroll-command))
+      (goto-char opoint))
+    (setq follow-fixed-window t)))
+
+;;;###autoload
+(defun follow-scroll-up-window (&optional arg)
+  "Scroll text in a Follow mode window up by that window's size.
+The other windows in the window chain will scroll synchronously.
+
+If called with no ARG, the `next-screen-context-lines' last lines of
+the window will be visible after the scroll.
+
+If called with an argument, scroll ARG lines up.
+Negative ARG means scroll downward.
+
+Works like `scroll-up' when not in Follow mode."
+  (interactive "P")
+  (cond ((not follow-mode)
+	 (scroll-up arg))
+	((eq arg '-)
+	 (follow-scroll-down-window))
+	(t (follow-scroll-up-arg arg))))
+(put 'follow-scroll-up-window 'scroll-command t)
+
+;;;###autoload
+(defun follow-scroll-down-window (&optional arg)
+  "Scroll text in a Follow mode window down by that window's size.
+The other windows in the window chain will scroll synchronously.
+
+If called with no ARG, the `next-screen-context-lines' top lines of
+the window in the chain will be visible after the scroll.
+
+If called with an argument, scroll ARG lines down.
+Negative ARG means scroll upward.
+
+Works like `scroll-down' when not in Follow mode."
+  (interactive "P")
+  (cond ((not follow-mode)
+	 (scroll-down arg))
+	((eq arg '-)
+	 (follow-scroll-up-window))
+	(t (follow-scroll-down-arg arg))))
+(put 'follow-scroll-down-window 'scroll-command t)
+
 ;;;###autoload
 (defun follow-scroll-up (&optional arg)
   "Scroll text in a Follow mode window chain up.
@@ -546,23 +620,18 @@ follow-scroll-up
   (interactive "P")
   (cond ((not follow-mode)
 	 (scroll-up arg))
-	((eq arg '-)
-	 (follow-scroll-down))
-	(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)))))
+	(arg (follow-scroll-up-arg arg))
+        (t
+	 (let* ((windows (follow-all-followers))
+		(end (window-end (car (reverse windows)))))
+	   (if (eq end (point-max))
+	       (signal 'end-of-buffer nil)
+	     (select-window (car windows))
+	     ;; `window-end' might return nil.
+	     (if end
+		 (goto-char end))
+	     (vertical-motion (- next-screen-context-lines))
+	     (set-window-start (car windows) (point)))))))
 (put 'follow-scroll-up 'scroll-command t)
 
 ;;;###autoload
@@ -579,15 +648,22 @@ follow-scroll-down
   (interactive "P")
   (cond ((not follow-mode)
 	 (scroll-down arg))
-	((eq arg '-)
-	 (follow-scroll-up))
-	(t
-	 (let ((opoint (point)))
-	   (scroll-down arg)
-	   (unless (and scroll-preserve-screen-position
-			(get this-command 'scroll-command))
-	     (goto-char opoint))
-	   (setq follow-fixed-window t)))))
+	(arg (follow-scroll-down-arg arg))
+        (t
+	 (let* ((windows (follow-all-followers))
+		(win (car (reverse windows)))
+		(start (window-start (car windows))))
+	   (if (eq start (point-min))
+	       (signal 'beginning-of-buffer nil)
+	     (select-window win)
+	     (goto-char start)
+	     (vertical-motion (- (- (window-height win)
+				    (if header-line-format 2 1)
+				    next-screen-context-lines)))
+	     (set-window-start win (point))
+	     (goto-char start)
+	     (vertical-motion (- next-screen-context-lines 1))
+	     (setq follow-internal-force-redisplay t))))))
 (put 'follow-scroll-down 'scroll-command t)
 
 (declare-function comint-adjust-point "comint" (window))


>     -- Anders

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#23347: 25.0.92; Follow mode scrolling broken -- scrolls only one page
  2016-04-26 11:51           ` Alan Mackenzie
@ 2016-04-26 13:08             ` Anders Lindgren
  0 siblings, 0 replies; 8+ messages in thread
From: Anders Lindgren @ 2016-04-26 13:08 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 23347

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

Hi!

I just tested this and I can confirm that it works, as far as I can tell!


I tested the "old" scroll functions `follow-scroll-up' and `-down' and now
they scroll the entire display, hence the bug I initially reported is fixed.

I tested with positive and negative numerical arguments and the
corresponding number of lines were scrolled, in the correct direction.

I tested the new "window" scroll functions and they work as well -- with
and without arguments.


I noticed some area where this could be improved upon, however they are
minor and I think we should address them after the emacs-25 release:

+ The new "window" scroll functions scroll almost a window. The effect is
that normally the point is moved from one window to the next. However, if
the cursor is at the bottom of a window the user do a
`follow-scroll-up-window', the point ens up at the top of the same window,
which could be a bit confusing. (In other words, skip the
`next-screen-context-lines' part. When using Follow mode, there is enough
context anyway.)

+ Now both `follow-scroll-up' and `follow-scroll-up-window' scroll ARG
lines when presented with an argument. It would be nice if either one of
them could scroll ARG windows instead. (Where "one window" could be seen as
the number of lines of the current window.) I think it would make most
sense to apply this change to `follow-scroll-up'.

+ Documentation:

In `follow-scroll-up-window' -- we should mention the difference is
compared to `scroll-up' (that the point isn't moved, if it can be displayed
in another window).

In `follow-scroll-up' -- Make it clear what the difference between this and
`follow-scroll-up-window' is (that it scroll the entire display).


Finally -- Many, many thanks for all the work you have put into this!

    -- Anders


On Tue, Apr 26, 2016 at 1:51 PM, Alan Mackenzie <acm@muc.de> wrote:

> Hello, Anders.
>
> On Sun, Apr 24, 2016 at 05:37:57PM +0200, Anders Lindgren wrote:
> > Hi!
>
> > Agreed. All three scroll commands (non-follow-mode scroll, scroll one
> page,
> > and scroll N pages) are useful.
>
> [ .... ]
>
> > Yes, I agree, `page' isn't a good choice.
>
> > What about `follow-scroll-up-keep-point'? That way it would be clear what
> > the difference is between it and the normal scroll command.
>
> > If I would have designed Follow mode today, I probably would have used
> your
> > name, `follow-scroll-up-window', and named the N-page scroll
> > `follow-scroll-up-window-group'.
>
> I've just used `follow-scroll-up-window' and
> `follow-scroll-down-window'.  At least, for now.
>
> [ .... ]
>
> > Anyway, here is what I think we can agree on:
> > > 1. `follow-scroll-up/down' should scroll by N windows.
> > > 2. There need to be commands in follow.el that scroll by 1 window as is
> > >   done by the current `follow-scroll-up/down'.  You have suggested the
> > >   name `follow-scroll-up/down-one-page'.  I have countered with
> > >   `follow-scroll-up/down-window'.
>
> > > If we can agree, I'm willing to amend follow.el.  I think it's probably
> > > too late for this fix to make it into Emacs 25.1, though.
> > >
>
> > Well, give it a try. Feel free to pick a suitable name. If we come up
> with
> > a clean solution quickly John might accept it.
>
> OK.  I had one or two minor problems with follow-scroll-up/down with a
> non-nil ARG.  In the end, I just extracted functionality from
> follow-scroll-up/down-window into separate functions, and called those
> from all four scrolling functions.
>
> Anyway, here is the patch I've come up with.  Could you review it and
> test it, please.  Thanks!
>
>
> diff --git a/lisp/follow.el b/lisp/follow.el
> index 5801f79..3e23247 100644
> --- a/lisp/follow.el
> +++ b/lisp/follow.el
> @@ -532,6 +532,80 @@ follow-get-scrolled-point
>  ;; position...  (This would also be corrected if we would have had a
>  ;; good redisplay abstraction.)
>
> +(defun follow-scroll-up-arg (arg)
> +  "Scroll the text in a follow mode window chain up by ARG lines.
> +If ARG is nil, scroll the size of the current window.
> +
> +This is an internal function for `follow-scroll-up' and
> +`follow-scroll-up-window'."
> +  (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)))
> +
> +(defun follow-scroll-down-arg (arg)
> +  "Scroll the text in a follow mode window chain down by ARG lines.
> +If ARG is nil, scroll the size of the current window.
> +
> +This is an internal function for `follow-scroll-down' and
> +`follow-scroll-down-window'."
> +  (let ((opoint (point)))
> +    (scroll-down arg)
> +    (unless (and scroll-preserve-screen-position
> +                 (get this-command 'scroll-command))
> +      (goto-char opoint))
> +    (setq follow-fixed-window t)))
> +
> +;;;###autoload
> +(defun follow-scroll-up-window (&optional arg)
> +  "Scroll text in a Follow mode window up by that window's size.
> +The other windows in the window chain will scroll synchronously.
> +
> +If called with no ARG, the `next-screen-context-lines' last lines of
> +the window will be visible after the scroll.
> +
> +If called with an argument, scroll ARG lines up.
> +Negative ARG means scroll downward.
> +
> +Works like `scroll-up' when not in Follow mode."
> +  (interactive "P")
> +  (cond ((not follow-mode)
> +        (scroll-up arg))
> +       ((eq arg '-)
> +        (follow-scroll-down-window))
> +       (t (follow-scroll-up-arg arg))))
> +(put 'follow-scroll-up-window 'scroll-command t)
> +
> +;;;###autoload
> +(defun follow-scroll-down-window (&optional arg)
> +  "Scroll text in a Follow mode window down by that window's size.
> +The other windows in the window chain will scroll synchronously.
> +
> +If called with no ARG, the `next-screen-context-lines' top lines of
> +the window in the chain will be visible after the scroll.
> +
> +If called with an argument, scroll ARG lines down.
> +Negative ARG means scroll upward.
> +
> +Works like `scroll-down' when not in Follow mode."
> +  (interactive "P")
> +  (cond ((not follow-mode)
> +        (scroll-down arg))
> +       ((eq arg '-)
> +        (follow-scroll-up-window))
> +       (t (follow-scroll-down-arg arg))))
> +(put 'follow-scroll-down-window 'scroll-command t)
> +
>  ;;;###autoload
>  (defun follow-scroll-up (&optional arg)
>    "Scroll text in a Follow mode window chain up.
> @@ -546,23 +620,18 @@ follow-scroll-up
>    (interactive "P")
>    (cond ((not follow-mode)
>          (scroll-up arg))
> -       ((eq arg '-)
> -        (follow-scroll-down))
> -       (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)))))
> +       (arg (follow-scroll-up-arg arg))
> +        (t
> +        (let* ((windows (follow-all-followers))
> +               (end (window-end (car (reverse windows)))))
> +          (if (eq end (point-max))
> +              (signal 'end-of-buffer nil)
> +            (select-window (car windows))
> +            ;; `window-end' might return nil.
> +            (if end
> +                (goto-char end))
> +            (vertical-motion (- next-screen-context-lines))
> +            (set-window-start (car windows) (point)))))))
>  (put 'follow-scroll-up 'scroll-command t)
>
>  ;;;###autoload
> @@ -579,15 +648,22 @@ follow-scroll-down
>    (interactive "P")
>    (cond ((not follow-mode)
>          (scroll-down arg))
> -       ((eq arg '-)
> -        (follow-scroll-up))
> -       (t
> -        (let ((opoint (point)))
> -          (scroll-down arg)
> -          (unless (and scroll-preserve-screen-position
> -                       (get this-command 'scroll-command))
> -            (goto-char opoint))
> -          (setq follow-fixed-window t)))))
> +       (arg (follow-scroll-down-arg arg))
> +        (t
> +        (let* ((windows (follow-all-followers))
> +               (win (car (reverse windows)))
> +               (start (window-start (car windows))))
> +          (if (eq start (point-min))
> +              (signal 'beginning-of-buffer nil)
> +            (select-window win)
> +            (goto-char start)
> +            (vertical-motion (- (- (window-height win)
> +                                   (if header-line-format 2 1)
> +                                   next-screen-context-lines)))
> +            (set-window-start win (point))
> +            (goto-char start)
> +            (vertical-motion (- next-screen-context-lines 1))
> +            (setq follow-internal-force-redisplay t))))))
>  (put 'follow-scroll-down 'scroll-command t)
>
>  (declare-function comint-adjust-point "comint" (window))
>
>
> >     -- Anders
>
> --
> Alan Mackenzie (Nuremberg, Germany).
>

[-- Attachment #2: Type: text/html, Size: 12146 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#23347: 25.0.92; Follow mode scrolling broken -- scrolls only one page
  2016-04-24 12:36     ` Anders Lindgren
  2016-04-24 14:23       ` Alan Mackenzie
@ 2016-04-29 15:02       ` Alan Mackenzie
  1 sibling, 0 replies; 8+ messages in thread
From: Alan Mackenzie @ 2016-04-29 15:02 UTC (permalink / raw)
  To: 23347-done; +Cc: Anders Lindgren

Bug fixed in emacs-25 branch.

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2016-04-29 15:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-23 17:25 bug#23347: 25.0.92; Follow mode scrolling broken -- scrolls only one page Anders Lindgren
     [not found] ` <mailman.843.1461432366.7477.bug-gnu-emacs@gnu.org>
2016-04-24  9:00   ` Alan Mackenzie
2016-04-24 12:36     ` Anders Lindgren
2016-04-24 14:23       ` Alan Mackenzie
2016-04-24 15:37         ` Anders Lindgren
2016-04-26 11:51           ` Alan Mackenzie
2016-04-26 13:08             ` Anders Lindgren
2016-04-29 15:02       ` Alan Mackenzie

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).