bug#31193: 26.1; error in `term-down' after window configuration change

[Noam Postavsky]

tags 31193 + patch
quit

Phil Sainty <psainty@orcon.net.nz> writes:

> On 2018-04-18 01:23, Noam Postavsky wrote:
>> Hmm, I'm not able to reproduce this, even though it makes sense to me
>> that my fix for Bug#30544 [1: a64c11a3aa] would cause this.  Perhaps it
>> depends on your shell prompt as well?
>
> I do have a non-trivial prompt; but after setting PS1="$ " I still see
> the same issue, so I'm unsure whether that's a factor.  My shell is
> bash.

Hmm, maybe it's a difference in bash versions?  I tried adding a newline
to my prompt with PS1=$'one\ntwo$ ', but bash doesn't send the newline
after the terminal resizes.  Seems it's clever enough just to redraw the
last line.

GNU bash, version 4.4.12(1)-release (x86_64-pc-linux-gnu)

And since terminal-emulate-terminal doesn't receive a \n character, it
doesn't call term-down before calling (term-current-row) near the end of
that function:

        (when (>= (term-current-row) term-height)
          (term-handle-deferred-scroll))

So I definitely can't see how you trigger this with PS1="$ ", but
perhaps your version of bash sends a newline for some other reason?  Can
you show the backtrace with PS1="$ "?

>> I think changing them to function calls, i.e., (term-current-row)
>> instead of term-current-row would be the right thing; all the other
>> term
>> functions use the function rather than accessing the variable directly.
>
> That sounds sane, and I can confirm that it fixes the problem.

Okay, here's the patch with message.  Eli, should I apply this to
emacs-26?  I believe it's safe because the term-current-row function
simply returns the value of the term-current-row variable if that is
non-nil.  Hence it should only affect cases where we were going to
signal an error due to type-mismatch anyway.

>From c1faca2b07ac86f84450bbb95ecb6d8635410281 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Tue, 17 Apr 2018 19:17:18 -0400
Subject: [PATCH] Don't assume term-current-row cache is valid (Bug#31193)

* lisp/term.el (term-down): Call `term-current-row' instead of
directly accessing the variable `term-current-row.  Following a resize
of the terminal's window, `term-current-row' is reset to nil, so it is
not safe to assume it is a number.
---
 lisp/term.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/term.el b/lisp/term.el
index 0a5efa4abc..6860ea6934 100644
--- a/lisp/term.el
+++ b/lisp/term.el
@@ -3764,7 +3764,7 @@ term-down
   (let ((start-column (term-horizontal-column)))
     (when (and check-for-scroll (or term-scroll-with-delete term-pager-count))
       (setq down (term-handle-scroll down)))
-    (unless (and (= term-current-row 0) (< down 0))
+    (unless (and (= (term-current-row) 0) (< down 0))
       (term-adjust-current-row-cache down)
       (when (or (/= (point) (point-max)) (< down 0))
        (setq down (- down (term-vertical-motion down)))))
@@ -3774,7 +3774,7 @@ term-down
           (setq term-current-column 0)
           (setq term-start-line-column 0))
          (t
-          (when (= term-current-row 0)
+          (when (= (term-current-row) 0)
             ;; Insert lines if at the beginning.
             (save-excursion (term-insert-char ?\n (- down)))
             (save-excursion
-- 
2.11.0