"Pager" page-up and -down, why not merge?

"Pager" page-up and -down, why not merge?

[Adrian Robert]

Hello,

The "pager" commands at:

http://user.it.uu.se/~mic/pager.el

seem to be an improvement over the existing definitions of scroll-down  
and scroll-up in emacs.  In particular, hitting sequences like [next]  
or Ctrl-v followed by [previous] or M-v leaves point in the same  
place, which is very calming.  ;-)  Is there any reason these could  
not be incorporated into emacs to replace scroll-up and scroll-down in  
these bindings?

It was discussed before with no conclusion:

http://article.gmane.org/gmane.emacs.devel/40324/match=pager
http://article.gmane.org/gmane.emacs.devel/69511/match=pager+el

I'm including below an improved version of this lisp that is used in  
Emacs.app.

----------------------

;;; page-down,page-up etc. leaves cursor in same place
;;; modified (ABR) from pager.el --- windows-scroll commands
;;; Version 2.0 - 97-10-06
;;; Copyright (C) 1992-1997 Mikael Sjodin (mic@docs.uu.se)
;;; http://user.it.uu.se/~mic/emacs.shtml

(defvar pager-temporary-goal-column 0
   "Similar to temporary-goal-column but used by the pager.el  
functions")
;(make-variable-buffer-local 'pager-temporary-goal-column)

(defconst pager-keep-column-commands
   '(pager-row-down pager-row-up row-dn row-up
		   pager-page-down pager-page-up pg-dn pg-up)
   "Commands which when called without any other intervening command  
should
keep the `pager-temporary-goal-column'")

(defun pager-page-down ()
   "Like scroll-up, but moves a fixed amount of lines (fixed relative  
the
`window-text-height') so that pager-page-up moves back to the same  
line."
   (interactive)
   (if (not (pos-visible-in-window-p (point-max)))
       (pager-scroll-screen (- (window-text-height)
			   next-screen-context-lines))))

(defun pager-page-up ()
   "Like scroll-down, but moves a fixed amount of lines (fixed  
relative the
`window-text-height') so that pager-page-down moves back to the same  
line."
   (interactive)
   (if (not (pos-visible-in-window-p (point-min)))
       (pager-scroll-screen (- next-screen-context-lines
			      (window-text-height)))))

(defun pager-scroll-screen (lines)
   "Scroll screen LINES, but keep the cursors position on screen."
   (if (not (memq last-command pager-keep-column-commands))
       (setq pager-temporary-goal-column (current-column)))
   (save-excursion
     (goto-char (window-start))
     (vertical-motion lines)
     (set-window-start (selected-window) (point)))
   (vertical-motion lines)
   (move-to-column pager-temporary-goal-column))

(global-set-key "\C-v"     'pager-page-down)
(global-set-key [next]     'pager-page-down)
(global-set-key "\ev"      'pager-page-up)
(global-set-key [prior]    'pager-page-up)

Re: "Pager" page-up and -down, why not merge?

[Richard M Stallman]

    seem to be an improvement over the existing definitions of scroll-down  
    and scroll-up in emacs.  In particular, hitting sequences like [next]  
    or Ctrl-v followed by [previous] or M-v leaves point in the same  
    place, which is very calming.

That in itself is a good thing, but ISTR that the simple
implementations of this had bad effects in other cases.
To do the job right requires something much more complex
which people usually don't bother trying to do.

Re: "Pager" page-up and -down, why not merge?

[Adrian Robert]

On Jun 4, 2008, at 7:13 AM, Richard M Stallman wrote:

>    seem to be an improvement over the existing definitions of scroll- 
> down
>    and scroll-up in emacs.  In particular, hitting sequences like  
> [next]
>    or Ctrl-v followed by [previous] or M-v leaves point in the same
>    place, which is very calming.
>
> That in itself is a good thing, but ISTR that the simple
> implementations of this had bad effects in other cases.
> To do the job right requires something much more complex
> which people usually don't bother trying to do.

If anyone who uses Emacs.app or could try out the elisp I posted could  
provide specifics on such misbehavior, that would be great.  I did  
improve the implementations in a couple of respects after feedback  
from the Emacs.app user base, and complaints have died off.  ;-)

Re: "Pager" page-up and -down, why not merge?

[Juanma Barranquero]

On Wed, Jun 4, 2008 at 1:13 PM, Richard M Stallman <rms@gnu.org> wrote:

> That in itself is a good thing, but ISTR that the simple
> implementations of this had bad effects in other cases.

I'haven't noticed any bad effect (though that means that it is related
to things I don't use, not that the effects do not exist, of course).

Care to elaborate which those bad effects are?

> To do the job right requires something much more complex
> which people usually don't bother trying to do.

Making the Emacs scroll movement commands more pager-like is quite
worthwhile IMHO.

   Juanma

Re: "Pager" page-up and -down, why not merge?

[Stefan Monnier]

> scroll-up in emacs.  In particular, hitting sequences like [next]  or Ctrl-v
> followed by [previous] or M-v leaves point in the same  place, which is very
> calming.  ;-)

In what way is it different from setting scroll-preserve-screen-position?

> Is there any reason these could not be incorporated into
> emacs to replace scroll-up and scroll-down in these bindings?

The devil is in the details, see comment below.

> (defvar pager-temporary-goal-column 0
>   "Similar to temporary-goal-column but used by the pager.el functions")
> ;(make-variable-buffer-local 'pager-temporary-goal-column)

I suggest we reuse temporary-goal-column instead.

> (defconst pager-keep-column-commands
>   '(pager-row-down pager-row-up row-dn row-up
> 		   pager-page-down pager-page-up pg-dn pg-up)
>   "Commands which when called without any other intervening command should
> keep the `pager-temporary-goal-column'")

What are those extra commands?

> (defun pager-scroll-screen (lines)
>   "Scroll screen LINES, but keep the cursors position on screen."
>   (if (not (memq last-command pager-keep-column-commands))
>       (setq pager-temporary-goal-column (current-column)))
>   (save-excursion
>     (goto-char (window-start))
>     (vertical-motion lines)
>     (set-window-start (selected-window) (point)))
>   (vertical-motion lines)
>   (move-to-column pager-temporary-goal-column))

Since this doesn't doesn't use scroll-up or scroll-down, it's very much
unclear whether it's going to behave sufficiently well in all cases.

I'm especially uncomfortable with the use of vertical-motion, which
doesn't actually use the redisplay's code, but some approximation of its
behavior, (so it doesn't pay attention to images and other things like
that, for example).

I'd suggest a different approach: use scroll-up/scroll-down as before, but
wrap them within code that uses compute-motion first to determine the
original screen position, and then vertical-motion afterwards to set
the cursor back to its position (but double-checking that this new
computed position is indeed visible on screen, to avoid pathological
behavior when compute-motion of vertical-motion get it wrong).

This way, you can guarantee that the scrolling behavior is 100%
preserved, and the new code only moves point around within the displayed
part of the buffer presumably to the position it had before.

Instead of compute-motion/vertical-motion, you could try to use
posn-at-point and posn-at-x-y.


        Stefan


PS: Here's a patch to window.c which makes
scroll-preserve-screen-position preserve the column position as well as
the line position, tho it only applies to GUI frames, not to tty frames
(which use another piece of code which I haven't bothered to adjust
accordingly).


=== modified file 'src/window.c'
--- src/window.c	2008-06-03 06:20:49 +0000
+++ src/window.c	2008-06-04 16:10:40 +0000
@@ -229,6 +229,7 @@
 /* Used by the function window_scroll_pixel_based */
 
 static int window_scroll_pixel_based_preserve_y;
+static int window_scroll_pixel_based_preserve_x;
 
 #if 0 /* This isn't used anywhere.  */
 /* Nonzero means we can split a frame even if it is "unsplittable".  */
@@ -5221,10 +5222,12 @@
 	  start_display (&it, w, start);
 	  move_it_to (&it, PT, -1, -1, -1, MOVE_TO_POS);
 	  window_scroll_pixel_based_preserve_y = it.current_y;
+	  window_scroll_pixel_based_preserve_x = it.current_x;
 	}
     }
   else
-    window_scroll_pixel_based_preserve_y = -1;
+    window_scroll_pixel_based_preserve_y
+      = window_scroll_pixel_based_preserve_x = -1;
 
   /* Move iterator it from start the specified distance forward or
      backward.  The result is the new window start.  */
@@ -5360,10 +5363,11 @@
 	{
 	  /* If we have a header line, take account of it.
 	     This is necessary because we set it.current_y to 0, above.  */
-	  move_it_to (&it, -1, -1,
+	  move_it_to (&it, -1,
+		      window_scroll_pixel_based_preserve_x,
 		      window_scroll_pixel_based_preserve_y
 		      - (WINDOW_WANTS_HEADER_LINE_P (w) ? 1 : 0 ),
-		      -1, MOVE_TO_Y);
+		      -1, MOVE_TO_Y | MOVE_TO_X);
 	  SET_PT_BOTH (IT_CHARPOS (it), IT_BYTEPOS (it));
 	}
       else
@@ -5421,8 +5425,9 @@
 	  /* It would be wrong to subtract CURRENT_HEADER_LINE_HEIGHT
 	     here because we called start_display again and did not
 	     alter it.current_y this time.  */
-	  move_it_to (&it, -1, -1, window_scroll_pixel_based_preserve_y, -1,
-		      MOVE_TO_Y);
+	  move_it_to (&it, -1, window_scroll_pixel_based_preserve_x,
+		      window_scroll_pixel_based_preserve_y, -1,
+		      MOVE_TO_Y | MOVE_TO_X);
 	  SET_PT_BOTH (IT_CHARPOS (it), IT_BYTEPOS (it));
 	}
       else
@@ -7445,6 +7450,7 @@
   staticpro (&minibuf_selected_window);
 
   window_scroll_pixel_based_preserve_y = -1;
+  window_scroll_pixel_based_preserve_x = -1;
 
   DEFVAR_LISP ("temp-buffer-show-function", &Vtemp_buffer_show_function,
 	       doc: /* Non-nil means call as function to display a help buffer.

Re: "Pager" page-up and -down, why not merge?

[Juanma Barranquero]

> In what way is it different from setting scroll-preserve-screen-position?

It preserves the column, too, which is the whole point of pager.el.

> I suggest we reuse temporary-goal-column instead.

>> (defconst pager-keep-column-commands
>>   '(pager-row-down pager-row-up row-dn row-up
>>                  pager-page-down pager-page-up pg-dn pg-up)
>>   "Commands which when called without any other intervening command should
>> keep the `pager-temporary-goal-column'")
>
> What are those extra commands?

pager-row-* are scrolling commands, not needed in Emacs (which has
scroll-lock-mode). pg-dn, pg-up, etc. are aliases for pager 1.0
compatibility.

> Since this doesn't doesn't use scroll-up or scroll-down, it's very much
> unclear whether it's going to behave sufficiently well in all cases.
>
> I'm especially uncomfortable with the use of vertical-motion, which
> doesn't actually use the redisplay's code, but some approximation of its
> behavior, (so it doesn't pay attention to images and other things like
> that, for example).

The original pager.el, which I'm using, does not use
`vertical-motion', but `forward-line', `scroll-down' and `scroll-up'.
Please take a look at http://user.it.uu.se/~mic/pager.el

> PS: Here's a patch to window.c which makes
> scroll-preserve-screen-position preserve the column position as well as
> the line position, tho it only applies to GUI frames, not to tty frames
> (which use another piece of code which I haven't bothered to adjust
> accordingly).

That is a great change, please install it!

 Juanma

Re: "Pager" page-up and -down, why not merge?

[Adrian Robert]

>> I'm especially uncomfortable with the use of vertical-motion, which
>> doesn't actually use the redisplay's code, but some approximation  
>> of its
>> behavior, (so it doesn't pay attention to images and other things  
>> like
>> that, for example).
>
> The original pager.el, which I'm using, does not use
> `vertical-motion', but `forward-line', `scroll-down' and `scroll-up'.
> Please take a look at http://user.it.uu.se/~mic/pager.el

Use of scroll-up and scroll-down had problems in buffers with wrapped  
lines; vertical-motion cured this.

Re: "Pager" page-up and -down, why not merge?

[Stefan Monnier]

> That is a great change, please install it!

Done,


        Stefan

Re: "Pager" page-up and -down, why not merge?

[Richard M Stallman]

    > That in itself is a good thing, but ISTR that the simple
    > implementations of this had bad effects in other cases.

    I'haven't noticed any bad effect (though that means that it is related
    to things I don't use, not that the effects do not exist, of course).

    Care to elaborate which those bad effects are?

I don't remember -- it was something like ten years ago that
I looked at the issue.  It may have involved cases where scrolling
hits the begining or end of the buffer and then you scroll in the
opposite direction.  Or it may have involved cases where you do some
editing then scroll back to where you were.

I think that all simple approaches will cause anomalies, but they will
be different depending on which simple approach.  You may not see them
in usual usage, but they will be there.  You need to think about the
cases where there may be anomalies -- that you do not encounter them
gives no evidence.

Re: "Pager" page-up and -down, why not merge?

[Juanma Barranquero]

On Thu, Jun 5, 2008 at 2:36 PM, Richard M Stallman <rms@gnu.org> wrote:

> I think that all simple approaches will cause anomalies, but they will
> be different depending on which simple approach.

The simple change done by Stefan to make
scroll-preserve-screen-position to preserve also the column already
does what I want. That's the functionality of pager.el I was
interested in.

>  You need to think about the
> cases where there may be anomalies -- that you do not encounter them
> gives no evidence.

Yes, I think I said as much in the message you're responding to.

 Juanma

Re: "Pager" page-up and -down, why not merge?

[Adrian Robert]

On Jun 4, 2008, at 12:14 PM, Stefan Monnier wrote:

> PS: Here's a patch to window.c which makes
> scroll-preserve-screen-position preserve the column position as well  
> as
> the line position, tho it only applies to GUI frames, not to tty  
> frames
> (which use another piece of code which I haven't bothered to adjust
> accordingly).

The new patch is great.  What about making scroll-preserve-screen- 
position t or non-nil/non-t by default?