bug#3361: 23.0.92; View-scroll-page-forward/backward does wrong in text mode after text-scale-increase/decrease

bug#3361: 23.0.92; View-scroll-page-forward/backward does wrong in text mode after text-scale-increase/decrease

[端瑞]

Open a normal text file with the text mode, do
(view-mode)
(text-scale-increase) or (text-scale-decrease)
(View-scroll-page-forward) or (View-scroll-page-backward)
The View-scroll-page-* command does wrong.  It cannot scroll "page
size" as expected like it does when the text-scale minor mode is off.


In GNU Emacs 23.0.92.1 (i386-mingw-nt5.1.2600)
 of 2009-03-31 on SOFT-MJASON
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (3.4)'

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: CHS
  value of $XMODIFIERS: nil
  locale-coding-system: cp936
  default-enable-multibyte-characters: t

Major mode: Text

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

Recent input:
<help-echo> C-x C-f d : / c o d e / b r e w / c t m
a i l / r e f / p r o t o c <tab> r f c 1 9 3 9 <tab>
<return> M-x v i e w SPC m o d e <return> SPC SPC SPC
SPC SPC SPC SPC SPC SPC SPC <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <S-down-mouse-1>
SPC SPC SPC SPC SPC M-x l i n SPC u m SPC m o e <backspace>
d e <return> SPC SPC SPC SPC SPC SPC SPC SPC <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> <backspace> <backspace>
<backspace> <backspace> <backspace> SPC <backspace>
SPC SPC SPC <backspace> SPC <backspace> SPC <S-down-mouse-1>
<S-down-mouse-1> <S-down-mouse-1> SPC SPC SPC <backspace>
SPC <backspace> SPC SPC SPC <S-down-mouse-1> M-x M-p
<return> C-x C-f d : / e h o m e / t e <tab> <return>
C-@ C-n C-n C-n C-n C-n C-n C-n C-n C-n C-n C-w C-x
d <return> n n n n n n n n n n n n n n n n n p p d
x y e s <return> C-x b <return> C-x k <return> y e
s <return> M-x r e p o r t <return> C-g M-x r e p o
r t <tab> <return> V i e w - C-g C-x b <return> C-h
k SPC C-x b <return> C-x b <return> M-x r e p o r t
<return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Making completion list...
Linum mode enabled
Linum mode disabled
Mark set
1 of 1 deletions
1 deletion done
Quit [2 times]
Type C-x 1 to delete the help window.

bug#3361: 23.0.92; View-scroll-page-forward/backward does wrong in text mode after text-scale-increase/decrease

[Chong Yidong]

> Open a normal text file with the text mode, do
> (view-mode)
> (text-scale-increase) or (text-scale-decrease)
> (View-scroll-page-forward) or (View-scroll-page-backward)
> The View-scroll-page-* command does wrong.  It cannot scroll "page
> size" as expected like it does when the text-scale minor mode is off.

View-mode, written in the dawn of time, assumes that all lines have the
default height.  The following patch changes it so that
scroll-up/scroll-down are passed nil arguments where possible; then
Emacs will automatically determine how to scroll by one page, taking
variable-height lines and text-scaling into account.

It's not good to make this kind of change at this stage in the release,
but this bug would be pretty annoying if you happen to come across it,
and that's much more likely now we've introduced the text-scaling
commands.  Could someone on emacs-devel help review the patch?


*** trunk/lisp/view.el.~1.102.~	2009-05-26 10:34:04.000000000 -0400
--- trunk/lisp/view.el	2009-05-26 10:39:03.000000000 -0400
***************
*** 740,747 ****
  ;;; Some help routines.
  
  (defun view-window-size ()
!   ;; Window height excluding mode line.
!   (1- (window-height)))
  
  ;; (defun view-last-command (&optional who what)
  ;;  (setq view-last-command-entry this-command)
--- 740,754 ----
  ;;; Some help routines.
  
  (defun view-window-size ()
!   ;; Return the number of lines in the current window, excluding the
!   ;; mode line.  Using `window-line-height' accounts for
!   ;; variable-height fonts.
!   (let ((h (window-line-height -1)))
!     (if h
! 	(1+ (nth 1 h))
!       ;; This should not happen, but if `window-line-height' returns
!       ;; nil, fall back on `window-height'.
!       (1- (window-height)))))
  
  ;; (defun view-last-command (&optional who what)
  ;;  (setq view-last-command-entry this-command)
***************
*** 761,771 ****
    (recenter '(1)))
  
  (defun view-page-size-default (lines)
!   ;; Get page size.
!   (let ((default (- (view-window-size) next-screen-context-lines)))
!     (if (or (null lines) (zerop (setq lines (prefix-numeric-value lines))))
! 	default
!       (min (abs lines) default))))
  
  (defun view-set-half-page-size-default (lines)
    ;; Get and maybe set half page size.
--- 768,780 ----
    (recenter '(1)))
  
  (defun view-page-size-default (lines)
!   ;; Return nil if LINES is nil, 0, or larger than
!   ;; `view-window-size'. Otherwise, return LINES.
!   (and lines
!        (not (zerop (setq lines (prefix-numeric-value lines))))
!        (<= (abs lines)
! 	   (abs (- (view-window-size) next-screen-context-lines)))
!        lines))
  
  (defun view-set-half-page-size-default (lines)
    ;; Get and maybe set half page size.
***************
*** 827,854 ****
    ;; This function does the job for all the scrolling commands.
    ;; Scroll forward LINES lines.  If BACKWARD is true scroll backwards.
    ;; If LINES is negative scroll in the other direction.  If LINES is 0 or nil,
!   ;; scroll DEFAULT lines.  If MAXDEFAULT is true then scroll no more than a
!   ;; window full.
    (if (or (null lines) (zerop (setq lines (prefix-numeric-value lines))))
        (setq lines default))
!   (when (< lines 0)
!     (setq backward (not backward)) (setq lines (- lines)))
!   (setq default (view-page-size-default nil)) ; Max scrolled at a time.
!   (if maxdefault (setq lines (min lines default)))
!   (cond
!    (backward (scroll-down lines))
!    ((view-really-at-end)
!     (if view-scroll-auto-exit (View-quit)
!       (ding)
!       (view-end-message)))
!    (t (while (> lines default)
! 	(scroll-up default)
! 	(setq lines (- lines default))
! 	(if (view-really-at-end) (setq lines 0)))
!       (scroll-up lines)
!       (if (view-really-at-end) (view-end-message))
!       (move-to-window-line -1)
!       (beginning-of-line))))
  
  (defun view-really-at-end ()
    ;; Return true if buffer end visible.  Maybe revert buffer and test.
--- 836,857 ----
    ;; This function does the job for all the scrolling commands.
    ;; Scroll forward LINES lines.  If BACKWARD is true scroll backwards.
    ;; If LINES is negative scroll in the other direction.  If LINES is 0 or nil,
!   ;; scroll DEFAULT lines (if DEFAULT is nil, scroll by one page).  If
!   ;; MAXDEFAULT is true then scroll no more than a window full.
    (if (or (null lines) (zerop (setq lines (prefix-numeric-value lines))))
        (setq lines default))
!   (when (and lines (< lines 0))
!     (setq backward (not backward) lines (- lines)))
!   (when (and maxdefault lines (> lines (view-window-size)))
!     (setq lines nil))
!   (cond (backward (scroll-down lines))
! 	((view-really-at-end)
! 	 (if view-scroll-auto-exit
! 	     (View-quit)
! 	   (ding)
! 	   (view-end-message)))
! 	(t (scroll-up lines)
! 	   (if (view-really-at-end) (view-end-message)))))
  
  (defun view-really-at-end ()
    ;; Return true if buffer end visible.  Maybe revert buffer and test.

bug#3361: 23.0.92; View-scroll-page-forward/backward does wrong in text mode after text-scale-increase/decrease

[Stefan Monnier]

> It's not good to make this kind of change at this stage in the release,
> but this bug would be pretty annoying if you happen to come across it,
> and that's much more likely now we've introduced the text-scaling
> commands.  Could someone on emacs-devel help review the patch?

It looks fine to me.
Tho, you could change the doccomments to docstrings while you're there.


        Stefan