branch: externals/topspace
commit 28f3792bf3b987bf901b6ab939bd42f4b0a852f0
Author: Trevor Pogue <pogu...@mcmaster.ca>
Commit: Trevor Pogue <pogu...@mcmaster.ca>
Internal refactoring
- Undo commit e046ab3 because the underlying issue it fixed was that
`window-end` needed to be updated when first centering new buffers.
- Update `window-end` now every time its used
- Add more comments for clarifications
- Remove some unnecessary uses of `float`
- Make sure `topspace-height` always returns a float
- Refactor `topspace--text` to be more clear
- Refactor `topspace--count-lines` to fix some corner cases
- Use `window-text-height` instead of `window-screen-lines`
- Update docstrings
---
README.md | 25 ++++---
test/topspace-test.el | 20 ++++--
topspace.el | 185 +++++++++++++++++++++++++++-----------------------
3 files changed, 131 insertions(+), 99 deletions(-)
diff --git a/README.md b/README.md
index a9b9d4706d..00adb0739d 100644
--- a/README.md
+++ b/README.md
@@ -96,11 +96,16 @@ then do auto-centering only when that function returns a
non-nil value."
(defcustom topspace-center-position 0.4
"Target position when centering buffers.
-Can be set to a float, integer, or function that returns a float or integer.
-If a float, it represents the position to center buffers as a ratio of
-frame height, and can be a value from 0.0 to 1.0 where lower values center
-buffers higher up in the screen.
+Used in `topspace-recenter-buffer' when called without an argument, or when
+opening/resizing buffers if `topspace-autocenter-buffers' returns non-nil.
+
+Can be set to a floating-point number, integer, or function that returns a
+floating-point number or integer.
+
+If a floating-point number, it represents the position to center buffers as a
+ratio of frame height, and can be a value from 0.0 to 1.0 where lower values
+center buffers higher up in the screen.
If a positive or negative integer value, buffers will be centered by putting
their center line at a distance of `topspace-center-position' lines away
@@ -109,10 +114,7 @@ of the selected window when negative.
The distance will be in units of lines with height `default-line-height',
and the value should be less than the height of the window.
-If a function, the same rules above apply to the functions' return value.
-
-Used in `topspace-recenter-buffer' when called without an argument, or when
-opening/resizing buffers if `topspace-autocenter-buffers' returns non-nil."
+If a function, the same rules above apply to the function's return value."
:group 'topspace
:type '(choice float integer
(function :tag "float or integer function")))
@@ -205,6 +207,9 @@ Valid top space line heights are:
(defun topspace-recenter-buffer (&optional position)
"Add enough top space to center small buffers according to POSITION.
POSITION defaults to `topspace-center-position'.
+Top space will not be added if the number of text lines in the buffer is larger
+than or close to the selected window's height, or if `window-start' is greater
+than 1.
If POSITION is a float, it represents the position to center buffer as a ratio
of frame height, and can be a value from 0.0 to 1.0 where lower values center
@@ -252,7 +257,9 @@ maps to in `fringe-indicator-alist'."
;;;###autoload
(defun topspace-buffer-was-scrolled-p ()
"Return t if current buffer has been scrolled in the selected window before.
-This is provided since it is used in `topspace-default-autocenter-buffers'."
+This is provided since it is used in `topspace-default-autocenter-buffers'.
+Scrolling is only recorded if topspace is active in the buffer at the time of
+scrolling."
...
```
diff --git a/test/topspace-test.el b/test/topspace-test.el
index c8fe4dc48a..853f34dac1 100644
--- a/test/topspace-test.el
+++ b/test/topspace-test.el
@@ -114,14 +114,14 @@ in case topspace-autocenter-buffers changed return value"
(expect topspace-mode :to-equal nil)
(scroll-up-line)
(topspace-set-height 1)
- (expect (topspace-height) :to-equal 0)
+ (expect (topspace-height) :to-equal 0.0)
(ignore-errors (scroll-down-line))
(topspace-mode 1)
(expect topspace-mode :to-equal t)
(switch-to-buffer "*scratch*")
(topspace-mode -1)
(topspace-recenter-buffer)
- (expect (topspace-height) :to-equal 0)
+ (expect (topspace-height) :to-equal 0.0)
(topspace-mode 1)))
(describe
@@ -200,6 +200,14 @@ by default"
;; :to-equal
;; (line-number-at-pos (point-max)))))
+ (it "can count lines end is larger `window-end'"
+ (set-window-start (selected-window) 300)
+ (expect (round (topspace--count-lines (window-start)
+ (1+ (topspace--window-end))))
+ :to-equal (count-screen-lines (window-start)
+ (topspace--window-end)))
+ (set-window-start (selected-window) 1))
+
(it "can count lines if start is larger than end"
(set-window-start (selected-window) 300)
(expect (round (topspace--count-lines 401 201))
@@ -217,7 +225,7 @@ by default"
"topspace--correct-height"
(it "fixes topspace height when larger than max valid value"
(let ((max-height
- (- (topspace--window-height) (topspace--context-lines))))
+ (- (window-text-height) topspace--context-lines)))
(expect (topspace--correct-height (1+ max-height))
:to-equal max-height))))
@@ -228,7 +236,7 @@ returns nil"
(let ((prev-autocenter-val topspace-autocenter-buffers))
(setq topspace--heights '())
(setq topspace-autocenter-buffers nil)
- (expect (topspace-height) :to-equal 0)
+ (expect (topspace-height) :to-equal 0.0)
(setq topspace-autocenter-buffers prev-autocenter-val))))
(describe
@@ -283,8 +291,8 @@ the bottom of the selected window.
(expect (topspace-height) :to-equal 4.0)
(setq topspace-center-position -4)
(topspace-recenter-buffer)
- (expect (topspace-height) :to-equal (- (topspace--window-height)
- (topspace--context-lines)))
+ (expect (topspace-height) :to-equal (float (- (window-text-height)
+ topspace--context-lines)))
(setq topspace-center-position topspace--prev-center-position))
)
)
diff --git a/topspace.el b/topspace.el
index 33da8e8815..b1a6329372 100644
--- a/topspace.el
+++ b/topspace.el
@@ -108,6 +108,11 @@ An error occurs in this mode any time `scroll-down' is
passed a
non-zero value, which halts tests and makes testing many topspace features
impossible. So this variable is set to zero when testing in this mode.")
+(defvar topspace--context-lines 1
+ "Determines how many lines away from `window-end' the cursor can get.
+This is relevant when scrolling in such a way that the cursor tries to
+move past `window-end'.")
+
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; Customization
@@ -159,11 +164,16 @@ then do auto-centering only when that function returns a
non-nil value."
(defcustom topspace-center-position 0.4
"Target position when centering buffers.
-Can be set to a float, integer, or function that returns a float or integer.
-If a float, it represents the position to center buffers as a ratio of
-frame height, and can be a value from 0.0 to 1.0 where lower values center
-buffers higher up in the screen.
+Used in `topspace-recenter-buffer' when called without an argument, or when
+opening/resizing buffers if `topspace-autocenter-buffers' returns non-nil.
+
+Can be set to a floating-point number, integer, or function that returns a
+floating-point number or integer.
+
+If a floating-point number, it represents the position to center buffers as a
+ratio of frame height, and can be a value from 0.0 to 1.0 where lower values
+center buffers higher up in the screen.
If a positive or negative integer value, buffers will be centered by putting
their center line at a distance of `topspace-center-position' lines away
@@ -172,13 +182,10 @@ of the selected window when negative.
The distance will be in units of lines with height `default-line-height',
and the value should be less than the height of the window.
-If a function, the same rules above apply to the functions' return value.
-
-Used in `topspace-recenter-buffer' when called without an argument, or when
-opening/resizing buffers if `topspace-autocenter-buffers' returns non-nil."
+If a function, the same rules above apply to the function's return value."
:group 'topspace
:type '(choice float integer
- (function :tag "float or integer function")))
+ (function :tag "floating-point number or integer function")))
(defcustom topspace-empty-line-indicator
#'topspace-default-empty-line-indicator
@@ -225,7 +232,7 @@ preferred bindings.")
(defun topspace-height ()
"Return the top space height in lines for current buffer in selected window.
The top space is the empty region in the buffer above the top text line.
-The return value is of type float, and is equivalent to
+The return value is a floating-point number, and is equivalent to
the top space pixel height / `default-line-height'.
If the height does not exist yet, zero will be returned if
@@ -238,14 +245,14 @@ Valid top space line heights are:
- never negative,
- only positive when `window-start' equals 1,
`topspace-active' returns non-nil, and `topspace-mode' is enabled,
-- not larger than `topspace--window-height' minus `topspace--context-lines'."
+- not larger than `window-text-height' minus `topspace--context-lines'."
(let ((height) (window (selected-window)))
;; First try returning previously stored top space height
(setq height (alist-get window topspace--heights))
(unless height
;; If it has never been created before then get the default value
(setq height (if (topspace--eval-choice topspace-autocenter-buffers)
- (topspace--height-to-recenter-buffer) 0)))
+ (topspace--height-to-recenter-buffer) 0.0)))
;; Correct, store, and return the new value
(topspace--set-height height)))
@@ -253,7 +260,7 @@ Valid top space line heights are:
(defun topspace-set-height (&optional total-lines)
"Set and redraw the top space overlay to have a target height of TOTAL-LINES.
This sets the top space height for the current buffer in the selected window.
-Integer or float values are accepted for TOTAL-LINES, and the value is
+Integer or floating-point numbers are accepted for TOTAL-LINES, and the value
is
considered to be in units of `default-line-height'.
If argument TOTAL-LINES is not provided, the top space height will be set to
@@ -267,7 +274,7 @@ Valid top space line heights are:
- never negative,
- only positive when `window-start' equals 1,
`topspace-active' returns non-nil, and `topspace-mode' is enabled,
-- not larger than `topspace--window-height' minus `topspace--context-lines'."
+- not larger than `window-text-height' minus `topspace--context-lines'."
(interactive "P")
(let ((old-height) (window (selected-window)))
;; Get the previous top space height
@@ -299,10 +306,13 @@ Valid top space line heights are:
(defun topspace-recenter-buffer (&optional position)
"Add enough top space to center small buffers according to POSITION.
POSITION defaults to `topspace-center-position'.
+Top space will not be added if the number of text lines in the buffer is larger
+than or close to the selected window's height, or if `window-start' is greater
+than 1.
-If POSITION is a float, it represents the position to center buffer as a ratio
-of frame height, and can be a value from 0.0 to 1.0 where lower values center
-the buffer higher up in the screen.
+If POSITION is a floating-point, it represents the position to center buffer as
+a ratio of frame height, and can be a value from 0.0 to 1.0 where lower values
+center the buffer higher up in the screen.
If POSITION is a positive or negative integer value, buffer will be centered
by putting its center line at a distance of `topspace-center-position' lines
@@ -320,7 +330,7 @@ Customize `topspace-autocenter-buffers' to run this command
automatically
after first opening buffers and after window sizes change."
(interactive)
(cond
- ((not (topspace--enabled)) (topspace-set-height 0))
+ ((not (topspace--enabled)) (topspace-set-height 0.0))
(t (topspace-set-height (topspace--height-to-recenter-buffer position)))))
;;;###autoload
@@ -361,7 +371,9 @@ maps to in `fringe-indicator-alist'."
;;;###autoload
(defun topspace-buffer-was-scrolled-p ()
"Return t if current buffer has been scrolled in the selected window before.
-This is provided since it is used in `topspace-default-autocenter-buffers'."
+This is provided since it is used in `topspace-default-autocenter-buffers'.
+Scrolling is only recorded if topspace is active in the buffer at the time of
+scrolling."
(alist-get (selected-window) topspace--buffer-was-scrolled))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
@@ -385,10 +397,10 @@ TOTAL-LINES is used in the same way as in `scroll-down'."
"Run before `scroll-down' for scrolling above the top line.
TOTAL-LINES is used in the same way as in `scroll-down'."
(cond
- ((not (topspace--enabled)) (topspace-set-height 0) total-lines)
+ ((not (topspace--enabled)) (topspace-set-height 0.0) total-lines)
(t
(setq total-lines (car total-lines))
- (setq total-lines (or total-lines (- (topspace--window-height)
+ (setq total-lines (or total-lines (- (window-text-height)
next-screen-context-lines)))
(setq topspace--total-lines-scrolling total-lines)
(list (* topspace--scroll-down-scale-factor
@@ -398,10 +410,10 @@ TOTAL-LINES is used in the same way as in `scroll-down'."
"Run before `scroll-up' for scrolling above the top line.
TOTAL-LINES is used in the same way as in `scroll-up'."
(cond
- ((not (topspace--enabled)) (topspace-set-height 0) total-lines)
+ ((not (topspace--enabled)) (topspace-set-height 0.0) total-lines)
(t
(setq total-lines (car total-lines))
- (setq total-lines (* (or total-lines (- (topspace--window-height)
+ (setq total-lines (* (or total-lines (- (window-text-height)
next-screen-context-lines)) -1))
(setq topspace--total-lines-scrolling total-lines)
(list (* (topspace--scroll total-lines) -1)))))
@@ -470,18 +482,18 @@ Valid top space line heights are:
- never negative,
- only positive when `window-start' equals 1,
`topspace-active' returns non-nil, and `topspace-mode' is enabled,
-- not larger than `topspace--window-height' minus `topspace--context-lines'."
- (let ((max-height (- (topspace--window-height) (topspace--context-lines))))
- (when (> (window-start) 1) (setq height 0))
- (when (< height 0) (setq height 0))
+- not larger than `window-text-height' minus `topspace--context-lines'."
+ (let ((max-height (- (window-text-height) topspace--context-lines)))
+ (setq height (float height))
+ (when (> (window-start) 1) (setq height 0.0))
+ (when (< height 0) (setq height 0.0))
(when (> height max-height) (setq height max-height))
- (unless (topspace--enabled) (setq height 0)))
+ (unless (topspace--enabled) (setq height 0.0)))
height)
-(defun topspace--context-lines ()
- "Return how many lines away from `window-end' the cursor can get.
-This is relevant when scrolling in such a way that the cursor tries to
-move past `window-end'." 1)
+(defun topspace--window-end ()
+ "Return the up-to-date `window-end'."
+ (or (window-end (selected-window) t)))
(defun topspace--total-lines-past-max (&optional topspace-height)
"Used when making sure top space height does not push cursor off-screen.
@@ -489,7 +501,7 @@ Return how many lines past the bottom of the window the
cursor would get pushed
if setting the top space to the target value TOPSPACE-HEIGHT.
Any value above 0 flags that the target TOPSPACE-HEIGHT is too large."
(- (topspace--current-line-plus-topspace topspace-height)
- (- (topspace--window-height) (topspace--context-lines))))
+ (- (window-text-height) topspace--context-lines)))
(defun topspace--current-line-plus-topspace (&optional topspace-height)
"Used when making sure top space height does not push cursor off-screen.
@@ -500,12 +512,12 @@ Return the current line plus the top space height
TOPSPACE-HEIGHT."
(defun topspace--calculate-recenter-line-offset (&optional line-offset)
"Convert LINE-OFFSET to a line offset from the top of the window.
It is interpreted in the same way as the first ARG in `recenter'."
- (unless line-offset (setq line-offset (/ (topspace--window-height) 2)))
+ (unless line-offset (setq line-offset (/ (float (window-text-height)) 2)))
(when (< line-offset 0)
;; subtracting 1 below made `recenter-top-bottom' act correctly
;; when it moves point to bottom and top space is added to get there
- (setq line-offset (- (- (topspace--window-height) line-offset)
- (topspace--context-lines)
+ (setq line-offset (- (- (window-text-height) line-offset)
+ topspace--context-lines
1)))
line-offset)
@@ -514,8 +526,9 @@ It is interpreted in the same way as the first ARG in
`recenter'."
Return how many lines away from the top of the selected window that the
buffer's center line will be moved to based on POSITION, which defaults to
`topspace-center-position'. Note that when POSITION
-is a float, the return value is only valid for windows starting at the top
-of the frame, which must be accounted for in the calling functions."
+is a floating-point number, the return value is only valid for windows
+starting at the top of the frame, which must be accounted for in the calling
+functions."
(setq position (or position (topspace--eval-choice
topspace-center-position)))
(if (floatp position)
(* (topspace--frame-height) position)
@@ -526,26 +539,24 @@ of the frame, which must be accounted for in the calling
functions."
Buffer will be centered according to POSITION, which defaults to
`topspace-center-position'."
(setq position (or position (topspace--eval-choice
topspace-center-position)))
- (let ((buffer-height (topspace--count-lines (window-start) (window-end)))
+ (let ((buffer-height (topspace--count-lines
+ (window-start)
+ (topspace--window-end)))
(result)
- (window-height (topspace--window-height)))
+ (window-height (window-text-height)))
(setq result (- (topspace--center-line position) (/ buffer-height 2)))
(when (floatp position) (setq result (- result (window-top-line))))
(when (> (+ result buffer-height)
- (- window-height (topspace--context-lines)))
+ (- window-height topspace--context-lines))
(setq result (- (- window-height buffer-height)
- (topspace--context-lines))))
+ topspace--context-lines)))
result))
(defun topspace--frame-height ()
"Return the number of lines in the selected frame's text area.
Subtract 3 from `frame-text-lines' to discount echo area and bottom
mode-line in centering."
- (float (- (frame-text-lines) 3)))
-
-(defun topspace--window-height ()
- "Return the number of screen lines in the selected window rounded down."
- (float (floor (window-screen-lines))))
+ (- (frame-text-lines) 3))
(defun topspace--count-pixel-height (start end)
"Return total pixels between points START and END as if they're both
visible."
@@ -573,52 +584,58 @@ return unexpected value when END is in column 0. This
fixes that issue.
This function also tries to first count the lines using a potentially faster
technique involving `window-absolute-pixel-position'.
If that doesn't work it uses `topspace--count-lines-slow'."
- (setq end (min end (point-max)))
- (setq start (max start (point-min)))
(let ((old-end) (old-start) (swap)
- (line-height (float (default-line-height)))
- (window-height (topspace--window-height)))
- (cond
- ((> (- (line-number-at-pos end) (line-number-at-pos start))
- (* 2 window-height))
- window-height)
- (t
- (when (> start end) (setq swap end) (setq end start) (setq start swap))
- (setq old-end end) (setq old-start start)
- (setq end (min end (- (window-end) 1)))
- (setq start (max start (window-start)))
- (let ((end-y (window-absolute-pixel-position end))
- (start-y (window-absolute-pixel-position start)))
- (+
- (if (> old-end end) (topspace--count-lines-slow end old-end) 0)
- (if (< old-start start) (topspace--count-lines-slow old-start start)
0)
- (condition-case nil
- ;; first try counting lines by getting the pixel difference
- ;; between end and start and dividing by `default-line-height'
- (/ (- (cdr end-y) (cdr start-y)) line-height)
- ;; if the pixel method above doesn't work do this slower method
- ;; (it won't work if either START or END are not visible in window)
- (error (topspace--count-lines-slow start end)))))))))
+ (line-height (float (default-line-height))))
+ (when (> start end) (setq swap end) (setq end start) (setq start swap))
+ (setq old-end end) (setq old-start start)
+ ;; use faster visual method for counting portion of lines in screen:
+ (when (< start (topspace--window-end))
+ (setq end (min end (topspace--window-end))))
+ (when (> end (window-start))
+ (setq start (max start (window-start))))
+ (let ((end-y (window-absolute-pixel-position end))
+ (start-y (window-absolute-pixel-position start)))
+ (+
+ (if (> old-end end) (topspace--count-lines-slow end old-end) 0.0)
+ (if (< old-start start) (topspace--count-lines-slow old-start start)
0.0)
+ (condition-case nil
+ ;; first try counting lines by getting the pixel difference
+ ;; between end and start and dividing by `default-line-height'
+ (/ (- (cdr end-y) (cdr start-y)) line-height)
+ ;; if the pixel method above doesn't work do this slower method
+ ;; (it won't work if either START or END are not visible in window)
+ (error (topspace--count-lines-slow start end)))))))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;; Overlay drawing
(defun topspace--text (height)
"Return the topspace text that appears in the top overlay with height
HEIGHT."
- (let ((text "")
- (indicator-line (topspace--eval-choice
- topspace-empty-line-indicator)))
- (setq indicator-line (cl-concatenate 'string indicator-line "\n"))
- (when (> height 0)
+ (cond
+ ((= (round height) 0) "")
+ ((= (round height) 1)
+ ;; comment a) It seems there is a bug in Emacs where you cannot set a
+ ;; string's line-height to a positive float less than 1. So in this
+ ;; condition, settle for rounding the top space height up to 1
+ ;; TODO: open issue with Emacs devel mailing list for this
+ "\n")
+ (t
+ ;; set the text to a series of newline characters with the last line
+ ;; having a line-height set to a float accounting for the potential
+ ;; fractional portion of the top space height
+ (let ((text "")
+ (indicator-line (topspace--eval-choice
+ topspace-empty-line-indicator)))
+ (setq indicator-line (cl-concatenate 'string indicator-line "\n"))
(dotimes (n (1- (floor height)))
n ;; remove flycheck warning
(setq text (cl-concatenate 'string text indicator-line)))
(setq indicator-line
+ ;; set that last line with a float line-height.
+ ;; The float will be set to >1 due to comment a) above
(propertize indicator-line 'line-height
- (round (* (+ 1.0 (- height (floor height)))
- (default-line-height)))))
- (setq text (cl-concatenate 'string text indicator-line))
- text)))
+ (- (1+ height) (floor height))))
+ (cl-concatenate 'string text indicator-line)))))
(defun topspace--increase-height (total-lines)
"Increase the top space line height by the target amount of TOTAL-LINES."
@@ -657,9 +674,9 @@ ARG defaults to 1."
(defun topspace--window-configuration-change ()
"Update top spaces when window buffers change or windows are resized."
(setq topspace--got-first-window-configuration-change t)
- (let ((current-height (topspace--window-height)) (window (selected-window)))
+ (let ((current-height (window-text-height)) (window (selected-window)))
(let ((previous-height (alist-get window topspace--previous-window-heights
- 0)))
+ 0.0)))
(if (and (topspace--eval-choice topspace-autocenter-buffers)
(not (= previous-height current-height)))
(topspace-recenter-buffer)
@@ -678,7 +695,7 @@ ARG defaults to 1."
(> (point) topspace--pre-command-point)
(< (- (line-number-at-pos (point))
(line-number-at-pos topspace--pre-command-point))
- (topspace--window-height)))
+ (window-text-height)))
(let ((topspace-height (topspace-height)) (total-lines-past-max))
(when (> topspace-height 0)
(setq total-lines-past-max (topspace--total-lines-past-max
