i3 - improved tiling WM

Reviews
Bugs

Movement into a branch considers movement direction

Patch status: needinfo

Patch by Tony Crisci

Long description:

Changes the behavior of movement into a branch with respect to the
position the moving con will be placed within the branch when the
movement is complete.

The correct position is determined by the direction of movement and the
position of the focused-inactive container within the branch.

When the movement is down or to the right, the moving con should be
placed before the focused-inactive container within the branch.

When the movement is up or to the left, the moving con should be placed
after the focused-inactive container within the branch.

fixes #1060

To apply this patch, use:
curl http://cr.i3wm.org/patch/337/raw.patch | git am

b/src/move.c

@@ -125,7 +125,9 @@ static void move_to_output_directed(Con *con, direction_t direction) {
  *
  */
 void tree_move(int direction) {
+    position_t position;
     DLOG("Moving in direction %d\n", direction);
+
     /* 1: get the first parent with the same orientation */
     Con *con = focused;
 
@@ -173,7 +175,9 @@ void tree_move(int direction) {
                           TAILQ_PREV(con, nodes_head, nodes) :
                           TAILQ_NEXT(con, nodes)))) {
                 if (!con_is_leaf(swap)) {
-                    insert_con_into(con, con_descend_focused(swap), AFTER);
+                    DLOG("Moving into a branch\n");
+                    position = (direction == D_UP || direction == D_LEFT ? AFTER : BEFORE);
+                    insert_con_into(con, con_descend_focused(swap), position);
                     goto end;
                 }
                 if (direction == D_LEFT || direction == D_UP)
@@ -215,7 +219,6 @@ void tree_move(int direction) {
 
     DLOG("above = %p\n", above);
     Con *next;
-    position_t position;
     if (direction == D_UP || direction == D_LEFT) {
         position = BEFORE;
         next = TAILQ_PREV(above, nodes_head, nodes);

b/testcases/t/213-move-branch-position.t

@@ -0,0 +1,151 @@
+#!perl
+# vim:ts=4:sw=4:expandtab
+#
+# Please read the following documents before working on tests:
+# • http://build.i3wm.org/docs/testsuite.html
+#   (or docs/testsuite)
+#
+# • http://build.i3wm.org/docs/lib-i3test.html
+#   (alternatively: perldoc ./testcases/lib/i3test.pm)
+#
+# • http://build.i3wm.org/docs/ipc.html
+#   (or docs/ipc)
+#
+# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf
+#   (unless you are already familiar with Perl)
+#
+# Test that movement of a con into a branch will place the moving con at the
+# correct position within the branch.
+#
+# The correct position is determined by the direction of movement and the
+# position of the focused-inactive container within the branch.
+#
+# Ticket: #1060
+# Bug still in: 4.6-109-g18cfc36
+
+use i3test;
+
+# Opens tabs on the presently focused branch and adds several additional
+# windows. Shifts focus to somewhere in the middle of the tabs so the most
+# general case can be assumed.
+sub open_tabs {
+    cmd 'layout tabbed';
+    open_window;
+    open_window;
+    open_window;
+    open_window;
+    cmd 'focus left; focus left'
+}
+
+############################################################################
+# When the movement is down or to the right, the moving con should be placed
+# before the focused-inactive container within the branch.
+############################################################################
+
+################
+## Move Right ##
+################
+my $ws = fresh_workspace;
+
+# create the target leaf
+my $leaf = open_window;
+
+# create the tabbed container and find the focused tab
+open_window;
+cmd 'splith';
+open_tabs;
+my $secondary_focus_tab = get_focused($ws);
+
+# focus the target leaf to the left
+cmd 'focus parent; focus left';
+# move the target leaf into the branch
+cmd 'move right';
+
+# the secondary focus tab should be to the right
+cmd 'focus right';
+is($secondary_focus_tab, get_focused($ws), 'moving con *right* into tabs placed it *before* the focused-inactive tab container');
+
+$leaf->unmap;
+wait_for_unmap;
+
+###############
+## Move down ##
+###############
+$ws = fresh_workspace;
+cmd 'layout splitv';
+
+# create the target leaf
+$leaf = open_window;
+
+# create the tabbed container and find the focused tab
+open_window;
+cmd 'splitv';
+open_tabs;
+$secondary_focus_tab = get_focused($ws);
+
+# focus the target leaf above
+cmd 'focus parent; focus up';
+
+# move the target leaf into the branch
+cmd 'move down';
+
+# the secondary focus tab should be to the right
+cmd 'focus right';
+is($secondary_focus_tab, get_focused($ws), 'moving con *down* into tabs placed it *before* the focused-inactive tab container');
+
+$leaf->unmap;
+wait_for_unmap;
+
+############################################################################
+# When the movement is up or to the left, the moving con should be placed
+# after the focused-inactive container within the branch.
+############################################################################
+
+###############
+## Move Left ##
+###############
+$ws = fresh_workspace;
+
+# create the tabbed container and find the focused tab
+open_window;
+open_tabs;
+$secondary_focus_tab = get_focused($ws);
+
+# create the target leaf to the right
+cmd 'focus parent';
+$leaf = open_window;
+
+# move the target leaf into the branch
+cmd 'move left';
+
+# the secondary focus tab should be to the left
+cmd 'focus left';
+is($secondary_focus_tab, get_focused($ws), 'moving con *left* into tabs placed it *after* the focused-inactive tab container');
+
+$leaf->unmap;
+wait_for_unmap;
+
+#############
+## Move Up ##
+#############
+$ws = fresh_workspace;
+cmd 'layout splitv';
+
+# create the tabbed container and find the focused tab
+open_window;
+cmd 'splitv';
+open_tabs;
+$secondary_focus_tab = get_focused($ws);
+
+# create the target leaf below
+cmd 'focus parent';
+$leaf = open_window;
+
+# move the target leaf into the branch
+cmd 'move up';
+
+# the secondary focus tab should be to the left
+cmd 'focus left';
+is($secondary_focus_tab, get_focused($ws), 'moving con *up* into tabs placed it *after* the focused-inactive tab container');
+
+done_testing;

63	@@ -0,0 +1,151 @@
64	+#!perl
65	+# vim:ts=4:sw=4:expandtab
66	+#
67	+# Please read the following documents before working on tests:
68	+# • http://build.i3wm.org/docs/testsuite.html
69	+# (or docs/testsuite)
70	+#
71	+# • http://build.i3wm.org/docs/lib-i3test.html
72	+# (alternatively: perldoc ./testcases/lib/i3test.pm)
73	+#
74	+# • http://build.i3wm.org/docs/ipc.html
75	+# (or docs/ipc)
76	+#
77	+# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf
78	+# (unless you are already familiar with Perl)
79	+#
80	+# Test that movement of a con into a branch will place the moving con at the
81	+# correct position within the branch.
82	+#
83	+# The correct position is determined by the direction of movement and the
84	+# position of the focused-inactive container within the branch.
85	+#
86	+# Ticket: #1060
87	+# Bug still in: 4.6-109-g18cfc36
88	+
89	+use i3test;
90	+
91	+# Opens tabs on the presently focused branch and adds several additional
92	+# windows. Shifts focus to somewhere in the middle of the tabs so the most
93	+# general case can be assumed.
94	+sub open_tabs {
95	+ cmd 'layout tabbed';
96	+ open_window;
97	+ open_window;
98	+ open_window;
99	+ open_window;
100	+ cmd 'focus left; focus left'
101	+}
102	+
103	+############################################################################
104	+# When the movement is down or to the right, the moving con should be placed
105	+# before the focused-inactive container within the branch.
106	+############################################################################
107	+
108	+################
109	+## Move Right ##
110	+################
111	+my $ws = fresh_workspace;
112	+
113	+# create the target leaf
114	+my $leaf = open_window;
115	+
116	+# create the tabbed container and find the focused tab
117	+open_window;
118	+cmd 'splith';
119	+open_tabs;
120	+my $secondary_focus_tab = get_focused($ws);
121	+
122	+# focus the target leaf to the left
123	+cmd 'focus parent; focus left';
124	+# move the target leaf into the branch
125	+cmd 'move right';
126	+
127	+# the secondary focus tab should be to the right
128	+cmd 'focus right';
129	+is($secondary_focus_tab, get_focused($ws), 'moving con right into tabs placed it before the focused-inactive tab container');
130	+
131	+$leaf->unmap;
132	+wait_for_unmap;
133	+
134	+###############
135	+## Move down ##
136	+###############
137	+$ws = fresh_workspace;
138	+cmd 'layout splitv';
139	+
140	+# create the target leaf
141	+$leaf = open_window;
142	+
143	+# create the tabbed container and find the focused tab
144	+open_window;
145	+cmd 'splitv';
146	+open_tabs;
147	+$secondary_focus_tab = get_focused($ws);
148	+
149	+# focus the target leaf above
150	+cmd 'focus parent; focus up';
151	+
152	+# move the target leaf into the branch
153	+cmd 'move down';
154	+
155	+# the secondary focus tab should be to the right
156	+cmd 'focus right';
157	+is($secondary_focus_tab, get_focused($ws), 'moving con down into tabs placed it before the focused-inactive tab container');
158	+
159	+$leaf->unmap;
160	+wait_for_unmap;
161	+
162	+############################################################################
163	+# When the movement is up or to the left, the moving con should be placed
164	+# after the focused-inactive container within the branch.
165	+############################################################################
166	+
167	+###############
168	+## Move Left ##
169	+###############
170	+$ws = fresh_workspace;
171	+
172	+# create the tabbed container and find the focused tab
173	+open_window;
174	+open_tabs;
175	+$secondary_focus_tab = get_focused($ws);
176	+
177	+# create the target leaf to the right
178	+cmd 'focus parent';
179	+$leaf = open_window;
180	+
181	+# move the target leaf into the branch
182	+cmd 'move left';
183	+
184	+# the secondary focus tab should be to the left
185	+cmd 'focus left';
186	+is($secondary_focus_tab, get_focused($ws), 'moving con left into tabs placed it after the focused-inactive tab container');
187	+
188	+$leaf->unmap;
189	+wait_for_unmap;
190	+
191	+#############
192	+## Move Up ##
193	+#############
194	+$ws = fresh_workspace;
195	+cmd 'layout splitv';
196	+
197	+# create the tabbed container and find the focused tab
198	+open_window;
199	+cmd 'splitv';
200	+open_tabs;
201	+$secondary_focus_tab = get_focused($ws);
202	+
203	+# create the target leaf below
204	+cmd 'focus parent';
205	+$leaf = open_window;
206	+
207	+# move the target leaf into the branch
208	+cmd 'move up';
209	+
210	+# the secondary focus tab should be to the left
211	+cmd 'focus left';
212	+is($secondary_focus_tab, get_focused($ws), 'moving con up into tabs placed it after the focused-inactive tab container');
213	+
214	+done_testing;

29	@@ -125,7 +125,9 @@ static void move_to_output_directed(Con *con, direction_t direction) {
30	*
31	*/
32	void tree_move(int direction) {
33	+ position_t position;
34	DLOG("Moving in direction %d\n", direction);
35	+
36	/* 1: get the first parent with the same orientation */
37	Con *con = focused;
38
39	@@ -173,7 +175,9 @@ void tree_move(int direction) {
40	TAILQ_PREV(con, nodes_head, nodes) :
41	TAILQ_NEXT(con, nodes)))) {
42	if (!con_is_leaf(swap)) {
43	- insert_con_into(con, con_descend_focused(swap), AFTER);
44	+ DLOG("Moving into a branch\n");
45	+ position = (direction == D_UP \|\| direction == D_LEFT ? AFTER : BEFORE);
46	+ insert_con_into(con, con_descend_focused(swap), position);
47	goto end;
48	}
49	if (direction == D_LEFT \|\| direction == D_UP)
50	@@ -215,7 +219,6 @@ void tree_move(int direction) {
51
52	DLOG("above = %p\n", above);
53	Con *next;
54	- position_t position;
55	if (direction == D_UP \|\| direction == D_LEFT) {
56	position = BEFORE;
57	next = TAILQ_PREV(above, nodes_head, nodes);