when switching terminals using alt+arrow, terminator sometimes choose the wrong terminal

[Lucas Nussbaum]

Hi,

When switching between terminals using shift+arrow, terminator sometimes switches to the wrong window : arrow+left will go to the top left window instead of the bottom left one, for example. it's not always reproducible (ie: the same construction of terminals won't always fail).

[Alan Jenkins (alan-james-jenkins)]

I can confirm this bug in terminator 0.12-1.

To replicate create a horizontal split, then split the top and bottom splits vertically to make a 2x2 grid of terminal windows.
Then move to the top right terminal and use alt and left to attempt to move to the top left terminal. This will instead result in you being switched to the bottom left terminal.

Same thing applies if you try to move from the top left terminal to top right (takes you to the bottom right terminal).

[jmonreal (jmonreal)]

Did you try the fix I made?

[Lucas Nussbaum (lucas)]

On 21/02/09 at 18:06 -0000, jmonreal wrote:
> Did you try the fix I made?

I just tried it. It seems to fix the case I ran into when reporting the
bug. However, it seems to break another case:

open terminator, split it with:
CTRL+SHIFT+e,CTRL+SHIFT+e,CTRL+SHIFT+e. You get three terminals, using
respectively 1/2, 1/4, 1/4 of the window. When going from left to right,
everything is fine. When going from right to left, the middle terminal
is skipped.

Without the patch, it works fine.
--
| Lucas Nussbaum
| <email address hidden> http://www.lucas-nussbaum.net/ |
| jabber: <email address hidden> GPG: 1024D/023B3F4F |

[jmonreal (jmonreal)]

I did the split the way you did and had no problem at all.

Im using kde 3.5.9.

[Lucas Nussbaum (lucas)]

On 25/02/09 at 03:04 -0000, jmonreal wrote:
> I did the split the way you did and had no problem at all.

With or without the patch? Without the patch, it works fine for me as
well.
--
| Lucas Nussbaum
| <email address hidden> http://www.lucas-nussbaum.net/ |
| jabber: <email address hidden> GPG: 1024D/023B3F4F |

[Chris Jones (cmsj)]

I've merged jmonreal's branch into trunk. I suspect we will always manage to produce slightly confusing results because of the way the widgets are nested, but if there is still a problem after the merge, please feel free to re-open this bug for further investigation.

[Tom Edwards (ubuntu-t0m)]

Sorry to be a pain, but I'm reopening this bug as it is affecting me still (in Karmic beta in the deb and the lp:terminator branch).

The behaviour is odd because of the use of "cursor_x" and cursor_y" which have odd behaviour when using scroll bars. These appear not describe to the absolute position of the cursor on the screen, but rather the position of the cursor inside the terminal window. This means that as you add lines to a terminal by pressing enter the value of of cursor_y will continue to increase even though the absolute position on screen has not changed as it scrolling down. This leads to the "odd" behaviour where the terminal moved to is unexpected.

I have modified terminator.py to make the decision about which terminal to choose based on the position of the centre of the current terminal on screen rather than the cursor position. This seems to produce consistent results with fairly complicated configurations and is a more intuitive than the apparent "random" behaviour that occurs with the previous version.

I hope this is welcome.
Kind regards,

Tom

[Chris Jones (cmsj)]

I'm not opposed to the change, but I'm also not hugely familiar with that code. I've subscribed Kees who wrote the directional navigation stuff initially for his thoughts (or lack thereof if you're busy Kees ;)

[Kees Cook (kees)]

I'll +1 this -- the follows-cursor thing never turned out as well as I was thinking it would. Using the center works for me. I haven't tested the code, but as long as it still takes into account the internal frame decoration sizes, go for it.

[Chris Jones (cmsj)]

I was just attempting to apply this patch to trunk and it seems to now have a problem - the while loops in on_term_resized() can become infinitely long, which obviously blocks the UI. I suspect this is because of some other patch work that has been going on in relation to this stuff :/

[Chris Jones (cmsj)]

trunk has now been significantly refactored and I rewrote the directional navigation code in the space of one evening, so I'm sure it will be even less "correct" than previously so, but I would appreciate any feedback people have, or patches!

[Raybuntu (raybuntu)]

Hi
I'm having the same issue with 0.14. I even tried 0.9 trunk and still no luck :(.
Moving up and down seems ok but right and left is broken.

best regards
Ray

[Chris Jones (cmsj)]

I agree, there are still some quirks in current trunk. Milestoning so they get looked at before 1.0.

[jmonreal (jmonreal)]

On Lun 15 Mar 2010 07:18:43 Chris Jones escribió:
> I agree, there are still some quirks in current trunk. Milestoning so
> they get looked at before 1.0.
>
> ** Changed in: terminator
> Status: Incomplete => Confirmed
>
> ** Changed in: terminator
> Importance: Undecided => Medium
>
> ** Changed in: terminator
> Milestone: None => 1.0-beta
>
> ** Changed in: terminator
> Assignee: jmonreal (jmonreal) => Chris Jones (cmsj)
>
Ill give it a look today, Im from Chile, and this is still moving, so I havent
been able to look at it.
But today Ill try.

[Adam Ehlers Nyholm Thomsen (adament)]

I think parts of this is caused because somehow the cursor_x, cursor_y calculations in window.py line 629 are wrong. My investigations show that it's the terminal.get_cursor_position() which is wrong, my investigations seem to show that self.vte.get_cursor_position returns strange row, col numbers. The bug is reproducable using a 2x2 grid.

[Tom Edwards (ubuntu-t0m)]

Hi Adam,

I found that the position returned was the position in the buffer, not the position on screen. The value of the y coordinate would keep advancing and advancing, so the code would work in the first instance, but as soon as the buffer became larger than could be displayed on screen the calculation would be "wrong". When I get a bit more time, I meant to come back to this bug and take another look.

Regards,

Tom

[Chris Jones (cmsj)]

Hmm, yeah using get_cursor_position() was probably a mistake. It's a real shame, because the cursor position seemed like a nice way to tie-break the equal matches, but I guess I'll rework it to just tiebreak based on the exact centre of the terminal.

[Juan Manuel Santos (juan-manuel-santos)]

Hello everybody. I've been using Terminator at work for a while now, and this bug is particularly annoying for me.

I have been hacking around at Terminator today trying to fix it. First I tried to attack this bug from the layout flank, trying to position the current terminal in the layout, and guessing from there and the requested direction, which one should be focused. It was actually really hard for me to do, so I decided to fix the current way of doing things (that would be more Pythonic now wouldn't it? ;))

Basically what I did in this patch, was to throw out the old x and y positions based on cursor positions, and get (as Chris said) the center of the terminal, and use those coordinates as calculations. I'm testing it and it seems to work OK, though I haven't tested it in really twisted layouts.

Two things to note:
* There is a "get_focussed_terminal". Just for spelling sake, I believe it should be "get_focused_terminal". I only mark this out because it took me a while to find it, cause I didn't notice that it had two ss' :P.
*That same method in terminator.py seems not to return the current terminal (always return None for me). I printed the flags for all terminals and were all the same. As a workaround I set a local variable in the focus_changed method, to record the widget that is passed as parameter as the current terminal (and modified get_focussed.. so it returned that variable). Just a hint.

Hope the patch is useful, please test.

Cheers
Juan Manuel

[Chris Jones (cmsj)]

Juan: Thanks for the patch, that does do what I was suggesting, although I note that even with the patch we still sometimes select a surprising terminal! I'll pull your patch into trunk :)

[Juan Manuel Santos (juan-manuel-santos)]

Thanks, glad to see it helped a little at least :)

Can you reproduce the bug with the patch then? Under which circumstances? Maybe then we can take a more in-depth look into this. I'd be glad to help if I can.

Cheers
Juan Manuel

[Chris Jones (cmsj)]

Juan: I'll try to come up with a reproducible layout, but I just opened a large window and split it a bunch of random ways and then started navigating around.

[Juan Manuel Santos (juan-manuel-santos)]

Chris: I know it´s been a while, but the other day I accidentally triggered this behaviour. I wanted to have a (lines*columns) 3*2 layout, which I managed by resizing some terminals. On this layout it is quite easy to trigger the described behaviour when trying to move up from some terminals (e.g. I can trigger it right now on a freshly opened layout like this when moving up from the bottom-right terminal).

When I submitted the patch, I was half way through a much more difficult approach to map the layout in memory, but that was when I gave up and came up with the patch I sent. Maybe I could dive into the code again and try to finish my former approach.

[Kejope (kejope)]

I can consistently reproduce this bug with vertical or horizontal splits.

I have never worked with vte, sorry. Is it really possible that there is no implementation to query the current physical x,y position? _Something_ has to keep track of that information! Only using the center x, y methodology allows that some layout patterns REQUIRE you to follow a specific path to get to a particular terminal.

Whereas, using the current physical x, y of the current terminal, I could "know" if I were on the last line of the left terminal, so that going right would take me directly to the finish terminal. Of course, I would have to query all terminals immediately to the right whose window-based physical coordinates are within the the bounds of this one, then pay attention to ratios, etc. Maybe. That is just a quick brainstorm. I'm sure you have put much more thought into it. :)

It seems possible to create a maze game out of a layout. Here is a simple one, starting from a new window (precede all lone [ o e w ] with ctrl+shift+ and all lone [ u d l r ] with alt+):
[ (type "start" in this terminal) e o u o d o (type "finish") l ] This gives one long left side and four rows on the right side, which doesn't seem so unreasonable. Now, using alt-directions, navigate to the finish terminal. The ONLY way to get there is by doing [ r d d ]. Why not [ r d d d ]? Because the center x, y of the left terminal is lined up with the second row on the right side, rather than with the top row. (Maybe. Testing from further down indicates that the physical x, y may not be so reliable.) (With my included tt.sh, you can automate this test with ./tt.sh maze1 )

Here is a layout that consistently produces a bizarre choice for alt-up: [ o e u e e l o ] From either bottom terminal, alt-up takes you to same destination terminal. Seeing it happen, you will recognize that alt-up from the bottom-left terminal has zero business going to that destination. If you split that destination terminal vertically, the behaviour seems a tiny bit more logical for this case, but still smacks of an inconsistent experience. (With my included tt.sh, you can automate this test with ./tt.sh dirs1 )

I thought to rotate that visual shape, to make it buggy for alt-left, but instead found an even more extreme alt-up: [ o e u e l o e ] If you add more vertical splits at the end of those instructions, each one seems more inconsistent. (With my included tt.sh, you can automate this test with ./tt.sh dirs2 )

[ e o l o u o e ] gives us the same visual layout, but totally different alt-up behaviour.
[[[This paragraph kept for posterity, because I didn't feel like deleting it! :) -> OK, I spent some time with that last pattern, then ctrl-shift-w for the last o and everything after it, then rebuilding last e's again, each time trying to find "something" to influence the final alt-up behaviour. It still feels flaky. (sorry) I didn't expect to focus so much on that part, or I would have tried to automate it with xdotool. :> ]]]
I automated it with xdotool! :)

Test Machine: AMD Athlon(tm) 64 X2 Dual Core Processor 5600+ with 4G RAM. Ubuntu Karmic. python-vte 1:0.22.2-0ubuntu2.1. Ter...

[Kejope (kejope)]

Code follows: (previous post was too large for Launchpad)
#!/bin/bash
CREATED=2010-09-30
MODIFIED=2010-10-01
VERSION=01.001.000
LICENSE="Public Domain"
##########################################
## Use this code at your own risk! ##
## I am not responspible for any ##
## results that come from your using ##
## it! ##
##########################################

# You should definitely make a backup of ~/.config/terminator/config
# This program will erase that file.

# Ignore this terminator PID when killing later. If you want/need this functionality,
# you will have to get the appropriate pid and put it here:
IGNOREPID=

# Set this to "true" if you want to run terminator with -d
DEBUG=false

# This may need to be a higher integer delay for the sleep command, for older machines
DELAY=1

ME="$0"

emptyconfig() {
 echo "" > ~/.config/terminator/config
 return 0
}

pretest() {
 local what="$1"

 echo "test=$what"
 case "$what" in
  vanilla)
   # Setup config
   # We have Titlebars.
   emptyconfig
   PARAM=
   ;;

  notitlebars)
   # Setup config
   # We disable Titlebars.
   emptyconfig
   echo "[profiles]" > ~/.config/terminator/config
   echo "[[default]]" >> ~/.config/terminator/config
   echo "show_titlebar = False" >> ~/.config/terminator/config
   PARAM=
   ;;

  dirs[123]|freeze[123])
   # Setup config
   emptyconfig
   PARAM=--geometry=800x600
   ;;

  maze1)
   # Setup config
   emptyconfig
   PARAM=--geometry=800x600
   ;;

  *)
   ERR="ERR:pretest: Invalid TEST: $what"
   return 1
   ;;
 esac
 return 0
}

xdodo() {
# do a sequence of xdotool key injections, then sleep for $DELAY seconds
# currently recognize:
# [ e o w ] as ctrl+shift+ combos
# [ u d l r ] as alt+ combos

 local k="$1"

 # TODO Implement window safety here, using xdotool to detect correct window
 # Probably prefer: xdodo [type "TEXT"]... | KEY...
 # TODO Change interface to accept unlimited parameters for sequences
 case "$k" in
  [EeOoWw])
   xdotool key ctrl+shift+$k
   ;;

  U|u)
   xdotool key alt+Up
   ;;

  D|d)
   xdotool key alt+Down
   ;;

  L|l)
   xdotool key alt+Left
   ;;

  R|r)
   xdotool key alt+Right
   ;;

  *)
   ERR="ERR:xdodo: Invalid key command: $k"
   return 1
   ;;
 esac
 sleep $(( DELAY * 1 ))
 return 0
}

xdotest() {
 local what="$1"
 local count=$2
 local i
 local k
 local abc="ABCDEFGHIJKLMNOPQRSTUVWXYZ" # if loop goes higher than 26 items, need to get a different plan
 case "$what" in
  vanilla|notitlebars)
   # open some terminals ...
   i=0
   xdotool type "${abc:i:1} "
   for i in $( seq 1 $count ); do
    k="o"; if ! xdodo "$k"; then return 1; fi
    #k="u"; if ! xdodo "$k"; then return 1; fi
    xdotool type "${abc:i:1} "
   done
   # ... and close them
   for i in $( seq 1 $count ); do
    k="w"; if ! xdodo "$k"; then return 1; fi
   done
   ;;

  dirs1)
   # create a layout, echoing a step number after each step ...
   i=0
   for k in o e u e e l o; do
    xdotool type "${abc:i:1} "
    if ! xdodo "$k"; then return 1; fi
    ((i++))
   done
   xdotool type "${abc:i:1} "
   sleep $(( DELAY * 1 ))
   # ... and move around between the terminals
   xdotool type " [ d u d l u ] "...

[Kejope (kejope)]

Sorry, forgot to mention that that last layout pattern [ e o l o u o e ] can be automated with: ./tt.sh dirs3

[Kejope (kejope)]

Tiny typo prevented vanilla and notitlebars tests from running. Fixed.

Inside VirtualBox, clean Ubuntu Lucid.
works: ./tt.sh vanilla 5
freezes: ./tt.sh vanilla 6
works: ./tt.sh notitlebars 3
freezes: ./tt.sh notitlebars 4
buggy alt-direction behaviour displayed: each of dirs1, dirs2, and dirs3 tests
freezes: each of freeze1, freeze2, and freeze3 tests

The automated tool includes a 'sleep 1' in between key presses. You can easily change the delay amount.
Also, if the geometry of the window is wider, then freeze1, freeze2, and freeze3 tests will not actually freeze, unless you do ctrl+shift+e more times.

[Kejope (kejope)]

... and the forgotten attachment.

[mattismyname (mattismyname)]

Just curious, does anybody work on terminator anymore or is there a better alternative that most people use now? Just curious because this bug is still pretty painful for anybody using terminator. Today I actually got into a situation where it was literally impossible to navigate to one of the terminals via arrow keys.

Thanks for any info.

[Maxime Jayat (maxime-jayat)]

Hey, since it has bothered me for a long time, I've worked on that bug.

There are two problems that I have fixed in two different commits pushed in my branch:

Problem 1:
Steps to reproduce:
-New window
-Split vertically
-Split both terminals horizontally
-Drag the left horizontal split down (by a large amount, or you may fall into problem 2).
-alt-up from the bottom-left terminal goes to the top-right terminal (all other terminal switches are correct)

I've tracked it down to a sign error in the offset calculation between terminals (get_nav_offset), it is fixed in the first commit.

Problem 2:
Steps to reproduce:
-New window
-Make sure the "terminal separator size" in the preferences is higher than 1. The higher, the easier to reproduce.
-Split horizontally
-Split both terminals vertically
-Drag the top vertical split one or two pixels to the left (less than the separator size).
-alt-right from to bottom-left goes to top-right, and alt-left from top-right goes to bottom-left.

The problem can also be reproduced when splitting vertically first, but then problem 1 also occurs and interacts with it, so the behaviour is different.

The bug here is more of an algorithm problem (see Window.navigate_terminal).
To do alt-right (all other directions work the same):
1-we build a list of all terminals which left edge is on the right of the right edge of the current terminal (get_nav_possible)
2-for each of those, we measure the horizontal distance between the current right edge and the left edge of the candidate terminal.
3-we sort the list of the measures and we switch to the terminal with the lowest distance. In case of tie, we use the position of the cursor for the tie break.

When reproducing, when you drag the top split one or two pixels to the left, you reduce the horizontal distance between bottom-left and top-right without invalidating the get_nav_possible test. So the chosen terminal is probably not the one expected by the user.

In my second commit, I have changed the get_nav_possible test so that only terminals which are on the same line or column as the current terminal are candidates for selection.

Please consider merging these fixes, I've been using them for some time without any issue.

[Bryce Harrington (bryce)]

I've committed a fix in rev 1554 for bug #1433810, which seems to significantly improve the behavior when doing alt+<arrow> navigation. I have a feeling that will serve to address some or maybe all the issues in this bug as well, but I'm not certain enough to close it.

Please give current trunk a try, and reply here whether it now works as you expect or, if not, propose a patch that fixes it more properly.

[Juan Manuel Santos (juan-manuel-santos)]

I'm the guy who submitted the patch to Terminator previously in c#18. I can confirm that, while Terminator was in a much better shape in trunk as of a couple of months ago, than when this bug was reported, the current trunk finished improving it and at least for my setup I no longer hit any issues.

My default Terminator layout is a 2x2 array. Sometimes when accidentally closing a pane and opening a new one, the new one would have its height a bit off with respect to its parallel pane (on the opposite column). This would sometimes (but not always) trigger the bug, in a random direction. Sometimes the specific reopened pane was not affected, but the one diagonally across.

Now with current trunk this problem is fixed, at least for me. Thanks!