[PATCH] trivial patch, jumping cursor in term

[PATCH] trivial patch, jumping cursor in term

[Ivan Kanis]

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

Hello Dan,

Here are the steps to reproduce the bug:

M-x term
C-x 2
Input a character

You'll see the cursor move to the top left when it shouldn't. I have
tracked the problem to the following call stack.

term-emulate-terminal
term-check-size
term-set-scroll-region
term-set-scroll-region

The cursor should move when receiving ESC [ R, so I've added a parameter
to term-set-scroll-region which will move the cursor if set to t.

Could you, please, apply the patch?

Kind regards,
-- 
Ivan
Kanis http://kanis.fr

Think like a man of action, act like a man of thought.
    -- Henry Bergson 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: term-cursor.patch --]
[-- Type: text/x-diff, Size: 1001 bytes --]

diff -r b9b27a5565ff lisp/misc/term.el
--- a/lisp/misc/term.el	Wed Sep 23 15:50:43 2009 +0200
+++ b/lisp/misc/term.el	Wed Sep 23 15:52:10 2009 +0200
@@ -3377,10 +3377,11 @@
    ((eq char ?r)
     (term-set-scroll-region
      (1- term-terminal-previous-parameter)
-     (1- term-terminal-parameter)))
+     (1- term-terminal-parameter)
+     t))
    (t)))
 
-(defun term-set-scroll-region (top bottom)
+(defun term-set-scroll-region (top bottom &optional esc-bracket-r)
   "Set scrolling region.
 TOP is the top-most line (inclusive) of the new scrolling region,
 while BOTTOM is the line following the new scrolling region (e.g. exclusive).
@@ -3398,7 +3399,8 @@
 	    (not (and (= term-scroll-start 0)
 		      (= term-scroll-end term-height)))))
   (term-move-columns (- (term-current-column)))
-  (term-goto 0 0))
+  (if esc-bracket-r
+      (term-goto 0 0)))
 
 ;; (defun term-switch-to-alternate-sub-buffer (set)
 ;;   ;; If asked to switch to (from) the alternate sub-buffer, and already (not)

Re: [PATCH] trivial patch, jumping cursor in term

[Dan Nicolaescu]

Ivan Kanis <expire-by-2009-09-28@kanis.fr> writes:

  > Hello Dan,
  > 
  > Here are the steps to reproduce the bug:
  > 
  > M-x term
  > C-x 2
  > Input a character

I can't reproduce this.
Can you please file a bug, including the patch, with a step description
on how to reproduce this starting from emacs -Q?  (making sure things
like this work correctly is non-trivial, and I might not get to it
soon).

If you can do the same in an "xterm" and point what M-x term does
different than "xterm", that would help a lot. 

  > You'll see the cursor move to the top left when it shouldn't. I have
  > tracked the problem to the following call stack.
  > 
  > term-emulate-terminal
  > term-check-size
  > term-set-scroll-region
  > term-set-scroll-region
  > 
  > The cursor should move when receiving ESC [ R, so I've added a parameter

Why?  

  > to term-set-scroll-region which will move the cursor if set to t.

  > Could you, please, apply the patch?
  > 
  > Kind regards,
  > -- 
  > Ivan
  > Kanis http://kanis.fr
  > 
  > Think like a man of action, act like a man of thought.
  >     -- Henry Bergson 

Please include a changeLog with the patch, to make it easier to
understand.


  > 
  > diff -r b9b27a5565ff lisp/misc/term.el
  > --- a/lisp/misc/term.el	Wed Sep 23 15:50:43 2009 +0200
  > +++ b/lisp/misc/term.el	Wed Sep 23 15:52:10 2009 +0200
  > @@ -3377,10 +3377,11 @@
  >     ((eq char ?r)
  >      (term-set-scroll-region
  >       (1- term-terminal-previous-parameter)
  > -     (1- term-terminal-parameter)))
  > +     (1- term-terminal-parameter)
  > +     t))
  >     (t)))
  >  
  > -(defun term-set-scroll-region (top bottom)
  > +(defun term-set-scroll-region (top bottom &optional esc-bracket-r)
                                                         ^^^^^^^^^^^^^
                                                This should be called
                                                move-point or similar.

  >    "Set scrolling region.
  >  TOP is the top-most line (inclusive) of the new scrolling region,
  >  while BOTTOM is the line following the new scrolling region (e.g. exclusive).
  > @@ -3398,7 +3399,8 @@
  >  	    (not (and (= term-scroll-start 0)
  >  		      (= term-scroll-end term-height)))))
  >    (term-move-columns (- (term-current-column)))
  > -  (term-goto 0 0))
  > +  (if esc-bracket-r
  > +      (term-goto 0 0)))
  >  
  >  ;; (defun term-switch-to-alternate-sub-buffer (set)
  >  ;;   ;; If asked to switch to (from) the alternate sub-buffer, and already (not)

Re: [PATCH] trivial patch, jumping cursor in term

[Stefan Monnier]

>>>>> "Ivan" == Ivan Kanis <expire-by-2009-09-28@kanis.fr> writes:

> Hello Dan,

> Here are the steps to reproduce the bug:

> M-x term
> C-x 2
> Input a character

The C-x 2 doesn't do anything here, so I don't know how to reproduce
your problem.

> You'll see the cursor move to the top left when it shouldn't. I have
> tracked the problem to the following call stack.

> term-emulate-terminal
> term-check-size
> term-set-scroll-region
> term-set-scroll-region

> The cursor should move when receiving ESC [ R, so I've added a parameter
> to term-set-scroll-region which will move the cursor if set to t.

> Could you, please, apply the patch?

I've installed the patch below instead.  Can you confirm it works as well?


        Stefan


--- term.el.~1.120.~	2009-09-23 22:02:59.000000000 -0400
+++ term.el	2009-09-23 22:09:37.000000000 -0400
@@ -3369,7 +3369,8 @@
    ((eq char ?r)
     (term-set-scroll-region
      (1- term-terminal-previous-parameter)
-     (1- term-terminal-parameter)))
+     (1- term-terminal-parameter))
+    (term-goto 0 0))
    (t)))
 
 (defun term-set-scroll-region (top bottom)
@@ -3389,8 +3390,7 @@
 	(or (term-using-alternate-sub-buffer)
 	    (not (and (= term-scroll-start 0)
 		      (= term-scroll-end term-height)))))
-  (term-move-columns (- (term-current-column)))
-  (term-goto 0 0))
+  (term-move-columns (- (term-current-column))))
 
 ;; (defun term-switch-to-alternate-sub-buffer (set)
 ;;   ;; If asked to switch to (from) the alternate sub-buffer, and already (not)

Re: [PATCH] trivial patch, jumping cursor in term

[Ivan Kanis]

Hello Stefan,

Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> The C-x 2 doesn't do anything here, so I don't know how to reproduce
> your problem.

Here I hope are better steps:

emacs -Q
M-x term
RET          (it default to /bin/bash here)
C-c C-j      (switch to line mode)
C-x 2
RET          (the cursor jumps to the top right)

I am running Ubuntu 8.4, emacs 23.1 and TERM is set to eterm-color. I
hope you can reproduce it.

> I've installed the patch below instead.  Can you confirm it works as well?

I have applied it and I am afraid it doesn't work. You should probably
back it out.
-- 
Ivan
Kanis http://kanis.fr

Personally I'm always ready to learn, although I do not always like
being taught.
    -- Winston Churchill 

Re: [PATCH] trivial patch, jumping cursor in term

[Stefan Monnier]

>> The C-x 2 doesn't do anything here, so I don't know how to reproduce
>> your problem.
> Here I hope are better steps:

> emacs -Q
> M-x term
> RET          (it default to /bin/bash here)
> C-c C-j      (switch to line mode)
> C-x 2
> RET          (the cursor jumps to the top right)

I see it now, thank you.

>> I've installed the patch below instead.  Can you confirm it works as well?
> I have applied it and I am afraid it doesn't work. You should probably
> back it out.

Hmm... it should have *exactly* the same effect as your patch.


        Stefan

Re: [PATCH] trivial patch, jumping cursor in term

[Dan Nicolaescu]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

  > >> The C-x 2 doesn't do anything here, so I don't know how to reproduce
  > >> your problem.
  > > Here I hope are better steps:
  > 
  > > emacs -Q
  > > M-x term
  > > RET          (it default to /bin/bash here)
  > > C-c C-j      (switch to line mode)
  > > C-x 2
  > > RET          (the cursor jumps to the top right)
  > 
  > I see it now, thank you.
  > 
  > >> I've installed the patch below instead.  Can you confirm it works as well?
  > > I have applied it and I am afraid it doesn't work. You should probably
  > > back it out.
  > 
  > Hmm... it should have *exactly* the same effect as your patch.

After your changes yesterday an error is thrown when using the above scenario:
error in process filter: Wrong type argument: number-or-marker-p, nil

Version 1.118 (i.e. just before your changes) does not do this.

Re: [PATCH] trivial patch, jumping cursor in term

[Ivan Kanis]

Hi Dan, Stefan,


Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> Hmm... it should have *exactly* the same effect as your patch.

Does it work for you? It didn't seem to work for me...

Dan Nicolaescu <dann@ics.uci.edu> wrote:

> After your changes yesterday an error is thrown when using the above scenario:
> error in process filter: Wrong type argument: number-or-marker-p, nil

Yeah I've seen it too, the following trivial patch fixes it.

--- a/lisp/misc/term.el Thu Sep 24 12:01:01 2009 +0200
+++ b/lisp/misc/term.el Fri Sep 25 10:53:21 2009 +0200
@@ -3730,7 +3730,7 @@
   (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 (= 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)))))


Take care,
-- 
Ivan
Kanis http://kanis.fr

The cause of these complaints lies in WinZip, which turns an all
upper-case directory into an all lower case one in a fit of
helpfulness
    -- Ant Documentation 

Re: [PATCH] trivial patch, jumping cursor in term

[Stefan Monnier]

>> Hmm... it should have *exactly* the same effect as your patch.
> Does it work for you? It didn't seem to work for me...

It worked as well (or as poorly, depending on the test) as yours (if
you look at the code, you'll see my patch really does exactly the same
thing as yours, tho slightly differently).

I tested the patch below, and it doesn't seem to fix the bug at all
in my case (using: M-x term RET RET  C-c C-j  C-x 2  RET).   :-(


        Stefan


--- term.el.~1.122.~	2009-09-24 22:47:06.000000000 -0400
+++ term.el	2009-09-25 10:25:33.000000000 -0400
@@ -3369,10 +3369,11 @@
    ((eq char ?r)
     (term-set-scroll-region
      (1- term-terminal-previous-parameter)
-     (1- term-terminal-parameter)))
+     (1- term-terminal-parameter)
+     t))
    (t)))
 
-(defun term-set-scroll-region (top bottom)
+(defun term-set-scroll-region (top bottom &optional esc-bracket-r)
   "Set scrolling region.
 TOP is the top-most line (inclusive) of the new scrolling region,
 while BOTTOM is the line following the new scrolling region (e.g. exclusive).
@@ -3390,7 +3391,8 @@
 	    (not (and (= term-scroll-start 0)
 		      (= term-scroll-end term-height)))))
   (term-move-columns (- (term-current-column)))
-  (term-goto 0 0))
+  (if esc-bracket-r
+      (term-goto 0 0)))
 
 ;; (defun term-switch-to-alternate-sub-buffer (set)
 ;;   ;; If asked to switch to (from) the alternate sub-buffer, and already (not)
@@ -3721,7 +3723,7 @@
   (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 (= 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)))))

Re: [PATCH] trivial patch, jumping cursor in term

[Ivan Kanis]

Stefan Monnier <monnier@iro.umontreal.ca> wrote:

> I tested the patch below, and it doesn't seem to fix the bug at all
> in my case (using: M-x term RET RET  C-c C-j  C-x 2  RET).   :-(

It's another bug. Notice the cursor is not going on the top left. I have
noticed it too but it's not as distracting because the cursor is not far
from the prompt. I tried to track where this new bug comes from. However
the function term-emulate-terminal is just too hairy.
-- 
Ivan
Kanis http://kanis.fr

The average Ph.D. thesis is nothing but the transference of bones
from one graveyard to another.
    -- Frank J. Dobie