procyberian/emacs
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix scrolling with partial line corner case (Bug#25792)

Browse source 
Also fix up the scrolling tests so that they don't make so many
assumptions about the current window configuration.

* src/xdisp.c (try_window): Take partial line height into account when
comparing cursor position against scroll margin.

* test/manual/scroll-tests.el (scroll-tests-with-buffer-window): Add
HEIGHT argument, to allow setting up window with exact height and
partial line.
(scroll-tests-display-buffer-with-height): New display-buffer action
function.
(scroll-tests-scroll-margin-over-max):
(scroll-tests--scroll-margin-whole-window): Pass HEIGHT to
`scroll-tests--scroll-margin-whole-window'.
(scroll-tests-conservative-show-trailing-whitespace): New test.
(scroll-tests-scroll-margin-negative): Fix line counting.
(scroll-tests--point-in-middle-of-window-p): Set window height
properly.
This commit is contained in:
Noam Postavsky 2017-02-20 13:34:39 -05:00
parent e52287ca3e
commit f0e7f39e0b
2 changed files with 80 additions and 37 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
src/xdisp.c
View file

@ -17380,21 +17380,27 @@ try_window (Lisp_Object window, struct text_pos pos, int flags)
return 0;
}
/* Save the character position of 'it' before we call
'start_display' again. */
ptrdiff_t it_charpos = IT_CHARPOS (it);
/* Don't let the cursor end in the scroll margins. */
if ((flags & TRY_WINDOW_CHECK_MARGINS)
&& !MINI_WINDOW_P (w))
{
int this_scroll_margin = window_scroll_margin (w, MARGIN_IN_PIXELS);
start_display (&it, w, pos);
if ((w->cursor.y >= 0 /* not vscrolled */
&& w->cursor.y < this_scroll_margin
&& CHARPOS (pos) > BEGV
&& IT_CHARPOS (it) < ZV)
&& it_charpos < ZV)
/* rms: considering make_cursor_line_fully_visible_p here
seems to give wrong results. We don't want to recenter
when the last line is partly visible, we want to allow
that case to be handled in the usual way. */
|| w->cursor.y > it.last_visible_y - this_scroll_margin - 1)
|| w->cursor.y > (it.last_visible_y - partial_line_height (&it)
- this_scroll_margin - 1))
{
w->cursor.vpos = -1;
clear_glyph_matrix (w->desired_matrix);
@ -17403,7 +17409,7 @@ try_window (Lisp_Object window, struct text_pos pos, int flags)
}
/* If bottom moved off end of frame, change mode line percentage. */
if (w->window_end_pos <= 0 && Z != IT_CHARPOS (it))
if (w->window_end_pos <= 0 && Z != it_charpos)
w->update_mode_line = true;
/* Set window_end_pos to the offset of the last character displayed

105
test/manual/scroll-tests.el
View file

@ -53,41 +53,79 @@
(sit-for 0)
(should (= 1 (window-start)))))
(defmacro scroll-tests-with-buffer-window (&rest body)
(declare (debug t))
(defun scroll-tests-display-buffer-with-height (buffer alist)
(let ((height (alist-get 'window-height alist)))
(when height
(let* ((window (or (get-buffer-window buffer) (selected-window)))
(lines (floor height))
(partial (round (* (- height lines) (default-line-height)))))
(setq window (cond ((window-in-direction 'above window nil +1))
((or (window-in-direction 'below window nil -1)
(split-window-below lines))
window)))
(set-window-buffer window buffer)
(set-window-text-height window lines)
(adjust-window-trailing-edge window partial nil t)
window))))
(defmacro scroll-tests-with-buffer-window (&optional height &rest body)
(declare (debug t) (indent defun))
`(with-temp-buffer
(with-selected-window (display-buffer (current-buffer))
(with-selected-window (display-buffer (current-buffer)
'(scroll-tests-display-buffer-with-height
. ,(if (numberp height)
`((window-height . ,height))
(push height body)
nil)))
,@body)))
(ert-deftest scroll-tests-scroll-margin-0 ()
(skip-unless (not noninteractive))
(scroll-tests-with-buffer-window
(scroll-tests-up-and-down 0)))
(scroll-tests-up-and-down 0)))
(ert-deftest scroll-tests-scroll-margin-negative ()
"A negative `scroll-margin' should be the same as 0."
(skip-unless (not noninteractive))
(scroll-tests-with-buffer-window
(scroll-tests-up-and-down -10 0)))
(scroll-tests-up-and-down -10 0)))
(ert-deftest scroll-tests-scroll-margin-max ()
(skip-unless (not noninteractive))
(scroll-tests-with-buffer-window
(let ((max-margin (/ (window-text-height) 4)))
(scroll-tests-up-and-down max-margin))))
(let ((max-margin (/ (window-text-height) 4)))
(scroll-tests-up-and-down max-margin))))
(ert-deftest scroll-tests-scroll-margin-over-max ()
"A `scroll-margin' more than max should be the same as max."
(skip-unless (not noninteractive))
(scroll-tests-with-buffer-window
(set-window-text-height nil 7)
(let ((max-margin (/ (window-text-height) 4)))
(scroll-tests-up-and-down (+ max-margin 1) max-margin)
(scroll-tests-up-and-down (+ max-margin 2) max-margin))))
(scroll-tests-with-buffer-window 7
(let ((max-margin (/ (window-text-height) 4)))
(scroll-tests-up-and-down (+ max-margin 1) max-margin)
(scroll-tests-up-and-down (+ max-margin 2) max-margin))))
(ert-deftest scroll-tests-conservative-show-trailing-whitespace ()
"Test for Bug#25792."
;; Note: requires partial line to trigger problem.
(scroll-tests-with-buffer-window 20.5
(let ((show-trailing-whitespace t)
(scroll-conservatively 101)
(scroll-margin 5))
(insert (mapconcat #'number-to-string
(number-sequence 1 200) "\n"))
(goto-char 1)
(forward-line 15)
(sit-for 0)
(let ((window-line (count-lines (window-start) (window-point))))
(dotimes (_ 10)
(call-interactively 'next-line)
(sit-for 0)
(should (= window-line (count-lines (window-start)
(window-point)))))))))
(defun scroll-tests--point-in-middle-of-window-p ()
(= (count-lines (window-start) (window-point))
(/ (1- (window-text-height)) 2)))
(/ (1- (floor (window-screen-lines))) 2)))
(cl-defun scroll-tests--scroll-margin-whole-window (&key with-line-spacing)
"Test `maximum-scroll-margin' at 0.5.
@ -95,27 +133,26 @@ With a high `scroll-margin', this should keep cursor in the
middle of the window."
(let ((maximum-scroll-margin 0.5)
(scroll-margin 100))
(scroll-tests-with-buffer-window
(setq-local line-spacing with-line-spacing)
;; Choose an odd number, so there is one line in the middle.
(set-window-text-height nil 7)
;; `set-window-text-height' doesn't count `line-spacing'.
(when with-line-spacing
(window-resize nil (* line-spacing 7) nil nil 'pixels))
(erase-buffer)
(insert (mapconcat #'number-to-string
(number-sequence 1 200) "\n"))
(goto-char 1)
(sit-for 0)
(call-interactively 'scroll-up-command)
(sit-for 0)
(should (scroll-tests--point-in-middle-of-window-p))
(call-interactively 'scroll-up-command)
(sit-for 0)
(should (scroll-tests--point-in-middle-of-window-p))
(call-interactively 'scroll-down-command)
(sit-for 0)
(should (scroll-tests--point-in-middle-of-window-p)))))
;; Choose an odd number of lines, so there is a middle line.
(scroll-tests-with-buffer-window 7
(setq-local line-spacing with-line-spacing)
;; `set-window-text-height' doesn't count `line-spacing'.
(when with-line-spacing
(window-resize nil (* line-spacing 8) nil nil 'pixels))
(erase-buffer)
(insert (mapconcat #'number-to-string
(number-sequence 1 200) "\n"))
(goto-char 1)
(sit-for 0)
(call-interactively 'scroll-up-command)
(sit-for 0)
(should (scroll-tests--point-in-middle-of-window-p))
(call-interactively 'scroll-up-command)
(sit-for 0)
(should (scroll-tests--point-in-middle-of-window-p))
(call-interactively 'scroll-down-command)
(sit-for 0)
(should (scroll-tests--point-in-middle-of-window-p)))))
(ert-deftest scroll-tests-scroll-margin-whole-window ()
(skip-unless (not noninteractive))