From: John Shahid Date: Mon, 29 Apr 2019 17:53:38 +0000 (-0400) Subject: Fix setting and resetting of scroll-with-delete X-Git-Tag: emacs-27.0.90~2948 X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=e44b56d16ad0749e599700a73509c16391ed973e;p=emacs.git 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). --- diff --git a/lisp/term.el b/lisp/term.el index 283e5684b72..553c3a1af4f 100644 --- a/lisp/term.el +++ b/lisp/term.el @@ -390,8 +390,16 @@ This emulates (more or less) the behavior of xterm.") "A queue of strings whose echo we want suppressed.") (defvar term-terminal-undecoded-bytes nil) (defvar term-current-face 'term) -(defvar term-scroll-start 0 "Top-most line (inclusive) of scrolling region.") -(defvar term-scroll-end) ; Number of line (zero-based) after scrolling region. +(defvar-local term-scroll-start 0 + "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 "Number of lines before we need to page; if nil, paging is disabled.") (defvar term-saved-cursor nil) @@ -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-row) (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-old-local-map) (make-local-variable 'term-old-mode-map) @@ -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) + (term--reset-scroll-region) + (easy-menu-add term-terminal-menu) (easy-menu-add term-signals-menu) (or term-input-ring @@ -1133,6 +1140,9 @@ Entry to this mode runs the hooks on `term-mode-hook'." (let ((inhibit-read-only t)) (delete-char 1))))) +(defun term--last-line () + (1- term-height)) + (defun term--filter-buffer-substring (content) (with-temp-buffer (insert content) @@ -1174,7 +1184,7 @@ Entry to this mode runs the hooks on `term-mode-hook'." (setq term-start-line-column nil) (setq term-current-row 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 ;; clear them again since we're changing point (Bug#30544). (setq term-start-line-column nil) @@ -3205,7 +3215,7 @@ option is enabled. See `term-set-goto-process-mark'." (goto-char term-home-marker) (term-vertical-motion (1+ count)) (set-marker term-home-marker (point)) - (setq term-current-row (1- term-height)))))) + (setq term-current-row (term--last-line)))))) (defun term-reset-terminal () "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'." (term-ansi-reset) (setq term-current-row 0) (setq term-current-column 1) - (setq term-scroll-start 0) - (setq term-scroll-end term-height) + (term--reset-scroll-region) (setq term-insert-mode nil) ;; FIXME: No idea why this is here, it looks wrong. --Stef (setq term-ansi-face-already-done nil)) @@ -3423,6 +3432,10 @@ option is enabled. See `term-set-goto-process-mark'." (1- (or (nth 1 params) 0)))) (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) "Set 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." 0 top)) (setq term-scroll-end - (if (or (<= bottom term-scroll-start) (> bottom term-height)) - term-height + (if (or (<= bottom term-scroll-start) (> bottom (term--last-line))) + (term--last-line) bottom)) (setq term-scroll-with-delete (or (term-using-alternate-sub-buffer) (not (and (= term-scroll-start 0) - (= term-scroll-end term-height))))) + (= term-scroll-end (term--last-line)))))) (term-move-columns (- (term-current-column))) (term-goto 0 0)) @@ -3568,7 +3581,7 @@ The top-most line is line 0." (when (> moved lines) (backward-char)) (cond ((<= deficit 0) ;; OK, had enough in the buffer for request. - (recenter (1- term-height))) + (recenter (term--last-line))) ((term-pager-continue deficit))))) (defun term-pager-page (arg) @@ -3582,7 +3595,7 @@ The top-most line is line 0." (goto-char (point-min)) (when (= (vertical-motion term-height) term-height) (backward-char)) - (recenter (1- term-height))) + (recenter (term--last-line))) ;; Pager mode command to go to end of buffer. (defun term-pager-eob () @@ -3600,7 +3613,7 @@ The top-most line is line 0." ;; Move cursor to end of window. (vertical-motion term-height) (backward-char)) - (recenter (1- term-height))) + (recenter (term--last-line))) (defun term-pager-back-page (arg) (interactive "p") diff --git a/test/lisp/term-tests.el b/test/lisp/term-tests.el index 9f5dcd559eb..6923096d224 100644 --- a/test/lisp/term-tests.el +++ b/test/lisp/term-tests.el @@ -119,7 +119,141 @@ line3\r line4\r line5\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 () (let ((term-ansi-at-user (user-real-login-name)))