[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: "Pager" page-up and -down, why not merge?
From: |
Stefan Monnier |
Subject: |
Re: "Pager" page-up and -down, why not merge? |
Date: |
Wed, 04 Jun 2008 12:14:24 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (gnu/linux) |
> 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.