Commit e44b56d1 authored by John Shahid's avatar John Shahid Committed by Stefan Monnier

Fix setting and resetting of scroll-with-delete

The start and end lines of the scroll region must to be in the range
[0,term-height).  There are few placees that incorrectly set the end
line of the scroll region to term-height which is outside the valid
range.  Combined with another off-by-one error in
term-set-scroll-region's clamping logic, this would cause
term-scroll-with-delete to be unnecessarily turned on.

* lisp/term.el (term-scroll-start,term-scroll-end): Use defvar-local
to define the variables and document the valid range of values that
the variables can take.
(term--last-line): New function to calculate the 0-based index of the
last line.
(term--reset-scroll-region): New function to reset the scroll region
to the full height of the terminal.
(term-mode,term-reset-size,term-reset-terminal): Call
term--reset-scroll-region to reset the scroll region.
(term-set-scroll-region): Fix the off-by-one error in the clamping
logic which allowed term-scroll-end to have values outside the valid
range [0,term-height).
parent 32cf0781
Pipeline #1601 failed with stage
in 52 minutes and 33 seconds
...@@ -390,8 +390,16 @@ This emulates (more or less) the behavior of xterm.") ...@@ -390,8 +390,16 @@ This emulates (more or less) the behavior of xterm.")
"A queue of strings whose echo we want suppressed.") "A queue of strings whose echo we want suppressed.")
(defvar term-terminal-undecoded-bytes nil) (defvar term-terminal-undecoded-bytes nil)
(defvar term-current-face 'term) (defvar term-current-face 'term)
(defvar term-scroll-start 0 "Top-most line (inclusive) of scrolling region.") (defvar-local term-scroll-start 0
(defvar term-scroll-end) ; Number of line (zero-based) after scrolling region. "Top-most line (inclusive) of the scrolling region.
`term-scroll-start' must be in the range [0,term-height). In addition, its
value has to be smaller than `term-scroll-end', i.e. one line scroll regions are
not allowed.")
(defvar-local term-scroll-end nil
"Bottom-most line (inclusive) of the scrolling region.
`term-scroll-end' must be in the range [0,term-height). In addition, its
value has to be greater than `term-scroll-start', i.e. one line scroll regions are
not allowed.")
(defvar term-pager-count nil (defvar term-pager-count nil
"Number of lines before we need to page; if nil, paging is disabled.") "Number of lines before we need to page; if nil, paging is disabled.")
(defvar term-saved-cursor nil) (defvar term-saved-cursor nil)
...@@ -1075,9 +1083,6 @@ Entry to this mode runs the hooks on `term-mode-hook'." ...@@ -1075,9 +1083,6 @@ Entry to this mode runs the hooks on `term-mode-hook'."
(make-local-variable 'term-current-column) (make-local-variable 'term-current-column)
(make-local-variable 'term-current-row) (make-local-variable 'term-current-row)
(make-local-variable 'term-log-buffer) (make-local-variable 'term-log-buffer)
(make-local-variable 'term-scroll-start)
(set (make-local-variable 'term-scroll-end) term-height)
(make-local-variable 'term-scroll-with-delete)
(make-local-variable 'term-pager-count) (make-local-variable 'term-pager-count)
(make-local-variable 'term-pager-old-local-map) (make-local-variable 'term-pager-old-local-map)
(make-local-variable 'term-old-mode-map) (make-local-variable 'term-old-mode-map)
...@@ -1117,6 +1122,8 @@ Entry to this mode runs the hooks on `term-mode-hook'." ...@@ -1117,6 +1122,8 @@ Entry to this mode runs the hooks on `term-mode-hook'."
(add-hook 'read-only-mode-hook #'term-line-mode-buffer-read-only-update nil t) (add-hook 'read-only-mode-hook #'term-line-mode-buffer-read-only-update nil t)
(term--reset-scroll-region)
(easy-menu-add term-terminal-menu) (easy-menu-add term-terminal-menu)
(easy-menu-add term-signals-menu) (easy-menu-add term-signals-menu)
(or term-input-ring (or term-input-ring
...@@ -1133,6 +1140,9 @@ Entry to this mode runs the hooks on `term-mode-hook'." ...@@ -1133,6 +1140,9 @@ Entry to this mode runs the hooks on `term-mode-hook'."
(let ((inhibit-read-only t)) (let ((inhibit-read-only t))
(delete-char 1))))) (delete-char 1)))))
(defun term--last-line ()
(1- term-height))
(defun term--filter-buffer-substring (content) (defun term--filter-buffer-substring (content)
(with-temp-buffer (with-temp-buffer
(insert content) (insert content)
...@@ -1174,7 +1184,7 @@ Entry to this mode runs the hooks on `term-mode-hook'." ...@@ -1174,7 +1184,7 @@ Entry to this mode runs the hooks on `term-mode-hook'."
(setq term-start-line-column nil) (setq term-start-line-column nil)
(setq term-current-row nil) (setq term-current-row nil)
(setq term-current-column nil) (setq term-current-column nil)
(term-set-scroll-region 0 height) (term--reset-scroll-region)
;; `term-set-scroll-region' causes these to be set, we have to ;; `term-set-scroll-region' causes these to be set, we have to
;; clear them again since we're changing point (Bug#30544). ;; clear them again since we're changing point (Bug#30544).
(setq term-start-line-column nil) (setq term-start-line-column nil)
...@@ -3205,7 +3215,7 @@ option is enabled. See `term-set-goto-process-mark'." ...@@ -3205,7 +3215,7 @@ option is enabled. See `term-set-goto-process-mark'."
(goto-char term-home-marker) (goto-char term-home-marker)
(term-vertical-motion (1+ count)) (term-vertical-motion (1+ count))
(set-marker term-home-marker (point)) (set-marker term-home-marker (point))
(setq term-current-row (1- term-height)))))) (setq term-current-row (term--last-line))))))
(defun term-reset-terminal () (defun term-reset-terminal ()
"Reset the terminal, delete all the content and set the face to the default one." "Reset the terminal, delete all the content and set the face to the default one."
...@@ -3213,8 +3223,7 @@ option is enabled. See `term-set-goto-process-mark'." ...@@ -3213,8 +3223,7 @@ option is enabled. See `term-set-goto-process-mark'."
(term-ansi-reset) (term-ansi-reset)
(setq term-current-row 0) (setq term-current-row 0)
(setq term-current-column 1) (setq term-current-column 1)
(setq term-scroll-start 0) (term--reset-scroll-region)
(setq term-scroll-end term-height)
(setq term-insert-mode nil) (setq term-insert-mode nil)
;; FIXME: No idea why this is here, it looks wrong. --Stef ;; FIXME: No idea why this is here, it looks wrong. --Stef
(setq term-ansi-face-already-done nil)) (setq term-ansi-face-already-done nil))
...@@ -3423,6 +3432,10 @@ option is enabled. See `term-set-goto-process-mark'." ...@@ -3423,6 +3432,10 @@ option is enabled. See `term-set-goto-process-mark'."
(1- (or (nth 1 params) 0)))) (1- (or (nth 1 params) 0))))
(t))) (t)))
(defun term--reset-scroll-region ()
"Sets the scroll region to the full height of the terminal."
(term-set-scroll-region 0 (term--last-line)))
(defun term-set-scroll-region (top bottom) (defun term-set-scroll-region (top bottom)
"Set scrolling region. "Set scrolling region.
TOP is the top-most line (inclusive) of the new scrolling region, TOP is the top-most line (inclusive) of the new scrolling region,
...@@ -3433,13 +3446,13 @@ The top-most line is line 0." ...@@ -3433,13 +3446,13 @@ The top-most line is line 0."
0 0
top)) top))
(setq term-scroll-end (setq term-scroll-end
(if (or (<= bottom term-scroll-start) (> bottom term-height)) (if (or (<= bottom term-scroll-start) (> bottom (term--last-line)))
term-height (term--last-line)
bottom)) bottom))
(setq term-scroll-with-delete (setq term-scroll-with-delete
(or (term-using-alternate-sub-buffer) (or (term-using-alternate-sub-buffer)
(not (and (= term-scroll-start 0) (not (and (= term-scroll-start 0)
(= term-scroll-end term-height))))) (= term-scroll-end (term--last-line))))))
(term-move-columns (- (term-current-column))) (term-move-columns (- (term-current-column)))
(term-goto 0 0)) (term-goto 0 0))
...@@ -3568,7 +3581,7 @@ The top-most line is line 0." ...@@ -3568,7 +3581,7 @@ The top-most line is line 0."
(when (> moved lines) (when (> moved lines)
(backward-char)) (backward-char))
(cond ((<= deficit 0) ;; OK, had enough in the buffer for request. (cond ((<= deficit 0) ;; OK, had enough in the buffer for request.
(recenter (1- term-height))) (recenter (term--last-line)))
((term-pager-continue deficit))))) ((term-pager-continue deficit)))))
(defun term-pager-page (arg) (defun term-pager-page (arg)
...@@ -3582,7 +3595,7 @@ The top-most line is line 0." ...@@ -3582,7 +3595,7 @@ The top-most line is line 0."
(goto-char (point-min)) (goto-char (point-min))
(when (= (vertical-motion term-height) term-height) (when (= (vertical-motion term-height) term-height)
(backward-char)) (backward-char))
(recenter (1- term-height))) (recenter (term--last-line)))
;; Pager mode command to go to end of buffer. ;; Pager mode command to go to end of buffer.
(defun term-pager-eob () (defun term-pager-eob ()
...@@ -3600,7 +3613,7 @@ The top-most line is line 0." ...@@ -3600,7 +3613,7 @@ The top-most line is line 0."
;; Move cursor to end of window. ;; Move cursor to end of window.
(vertical-motion term-height) (vertical-motion term-height)
(backward-char)) (backward-char))
(recenter (1- term-height))) (recenter (term--last-line)))
(defun term-pager-back-page (arg) (defun term-pager-back-page (arg)
(interactive "p") (interactive "p")
......
...@@ -119,7 +119,141 @@ line3\r ...@@ -119,7 +119,141 @@ line3\r
line4\r line4\r
line5\r line5\r
line6\r line6\r
")))) ")))
;; test reverse scrolling
(should (equal "line1
line7
line6
line2
line5"
(term-test-screen-from-input 40 5
'("\e[0;0H"
"\e[J"
"line1\r
line2\r
line3\r
line4\r
line5"
"\e[2;4r"
"\e[2;0H"
"\e[2;0H"
"\eMline6"
"\e[2;0H"
"\eMline7"))))
;; test scrolling down
(should (equal "line1
line3
line4
line7
line5"
(term-test-screen-from-input 40 5
'("\e[0;0H"
"\e[J"
"line1\r
line2\r
line3\r
line4\r
line5"
"\e[2;4r"
"\e[2;0H"
"\e[4;5H"
"\n\rline7"))))
;; setting the scroll region end beyond the max height should not
;; turn on term-scroll-with-delete
(should (equal "line1
line2
line3
line4
line5
line6
line7"
(term-test-screen-from-input 40 5
'("\e[1;10r"
"line1\r
line2\r
line3\r
line4\r
line5\r
line6\r
line7"))))
;; resetting the terminal should set the scroll region end to (1- term-height).
(should (equal "
line1
line2
line3
line4
"
(term-test-screen-from-input 40 5
'("\e[1;10r"
"\ec" ;reset
"line1\r
line2\r
line3\r
line4\r
line5"
"\e[1;1H"
"\e[L"))))
;; scroll region should be limited to the (1- term-height). Note,
;; this fixes an off by one error when comparing the scroll region
;; end with term-height.
(should (equal "
line1
line2
line3
line4
"
(term-test-screen-from-input 40 5
'("\e[1;6r"
"line1\r
line2\r
line3\r
line4\r
line5"
"\e[1;1H" ;go back to home
"\e[L" ;insert a new line at the top
))))
;; setting the scroll region to the entire height should not turn on
;; term-scroll-with-delete
(should (equal "line1
line2
line3
line4
line5
line6"
(term-test-screen-from-input 40 5
'("\e[1;5r"
"line1\r
line2\r
line3\r
line4\r
line5\r
line6"))))
;; reset should reset term-scroll-with-delete
(should (equal "line1
line2
line3
line4
line5
line6
line7"
(term-test-screen-from-input 40 5
'("\e[2;5r" ;set the region
"\ec" ;reset
"line1\r
line2\r
line3\r
line4\r
line5\r
line6\r
line7")))))
(ert-deftest term-set-directory () (ert-deftest term-set-directory ()
(let ((term-ansi-at-user (user-real-login-name))) (let ((term-ansi-at-user (user-real-login-name)))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment