i3 - improved tiling WM


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
possibly the position of the focused-inactive container within the
branch.

If the direction of movement is the same as the orientation of the
branch container, append or prepend the container to the branch in the
obvious way.  If the movement is to the right or downward, insert the
moving container in the first position (i.e., the leftmost or top
position resp.) If the movement is to the left or upward, insert the
moving container in the last position (i.e., the rightmost or bottom
position resp.)

If the direction of movement is different from the orientation of the
branch container, insert the container into the branch next to the
focused-inactive container. If the direction of movement is to the right
or downward, insert the moving container before the focused-inactive
container (i.e., insert above or left-of resp.) If the direction of
movement is to the left or upward, insert the moving container after the
focused-inactive container (i.e., insert below or right-of resp.)

fixes #1060

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

b/src/move.c

40
@@ -125,7 +125,9 @@ static void move_to_output_directed(Con *con, direction_t direction) {
41
  *
42
  */
43
 void tree_move(int direction) {
44
+    position_t position;
45
     DLOG("Moving in direction %d\n", direction);
46
+
47
     /* 1: get the first parent with the same orientation */
48
     Con *con = focused;
49
 
50
@@ -173,7 +175,10 @@ void tree_move(int direction) {
51
                           TAILQ_PREV(con, nodes_head, nodes) :
52
                           TAILQ_NEXT(con, nodes)))) {
53
                 if (!con_is_leaf(swap)) {
54
-                    insert_con_into(con, con_descend_focused(swap), AFTER);
55
+                    DLOG("Moving into our bordering branch\n");
56
+                    /* choose position based on *movement into* the branch */
57
+                    position = (direction == D_UP || direction == D_LEFT ? AFTER : BEFORE);
58
+                    insert_con_into(con, con_descend_direction(swap, direction), position);
59
                     goto end;
60
                 }
61
                 if (direction == D_LEFT || direction == D_UP)
62
@@ -214,22 +219,22 @@ void tree_move(int direction) {
63
     }
64
 
65
     DLOG("above = %p\n", above);
66
-    Con *next;
67
-    position_t position;
68
-    if (direction == D_UP || direction == D_LEFT) {
69
-        position = BEFORE;
70
-        next = TAILQ_PREV(above, nodes_head, nodes);
71
-    } else {
72
-        position = AFTER;
73
-        next = TAILQ_NEXT(above, nodes);
74
-    }
75
 
76
-    /* special case: there is a split container in the direction we are moving
77
-     * to, so descend and append */
78
-    if (next && !con_is_leaf(next))
79
-        insert_con_into(con, con_descend_focused(next), AFTER);
80
-    else
81
+    Con *next = (direction == D_UP || direction == D_LEFT ?
82
+            TAILQ_PREV(above, nodes_head, nodes) :
83
+            TAILQ_NEXT(above, nodes));
84
+
85
+    if (next && !con_is_leaf(next)) {
86
+        DLOG("Moving into the bordering branch of our adjacent container\n");
87
+        /* choose position based on *movement into* the branch */
88
+        position = (direction == D_UP || direction == D_LEFT ? AFTER : BEFORE);
89
+        insert_con_into(con, con_descend_direction(next, direction), position);
90
+    } else {
91
+        DLOG("Moving into container above\n");
92
+        /* choose position based on *movement next to* the leaf (or border) */
93
+        position = (direction == D_UP || direction == D_LEFT ? BEFORE : AFTER);
94
         insert_con_into(con, above, position);
95
+    }
96
 
97
 end:
98
     /* We need to call con_focus() to fix the focus stack "above" the container

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

104
@@ -0,0 +1,386 @@
105
+#!perl
106
+# vim:ts=4:sw=4:expandtab
107
+#
108
+# Please read the following documents before working on tests:
109
+# • http://build.i3wm.org/docs/testsuite.html
110
+#   (or docs/testsuite)
111
+#
112
+# • http://build.i3wm.org/docs/lib-i3test.html
113
+#   (alternatively: perldoc ./testcases/lib/i3test.pm)
114
+#
115
+# • http://build.i3wm.org/docs/ipc.html
116
+#   (or docs/ipc)
117
+#
118
+# • http://onyxneon.com/books/modern_perl/modern_perl_a4.pdf
119
+#   (unless you are already familiar with Perl)
120
+#
121
+# Test that movement of a con into a branch will place the moving con at the
122
+# correct position within the branch.
123
+#
124
+# If the direction of movement is the same as the orientation of the branch
125
+# container, append or prepend the container to the branch in the obvious way.
126
+# If the movement is to the right or downward, insert the moving container in
127
+# the first position (i.e., the leftmost or top position resp.) If the movement
128
+# is to the left or upward, insert the moving container in the last position
129
+# (i.e., the rightmost or bottom position resp.)
130
+#
131
+# If the direction of movement is different from the orientation of the branch
132
+# container, insert the container into the branch next to the focused-inactive
133
+# container. If the direction of movement is to the right or downward, insert
134
+# the moving container before the focused-inactive container (i.e., insert
135
+# above or left-of resp.) If the direction of movement is to the left or
136
+# upward, insert the moving container after the focused-inactive container
137
+# (i.e., insert below or right-of resp.)
138
+#
139
+# For testing purposes, we will demonstrate the behavior for tabbed containers
140
+# to represent the case of split-horizontal branches and stacked containers to
141
+# represent the case of split-vertical branches.
142
+#
143
+# Ticket: #1060
144
+# Bug still in: 4.6-109-g18cfc36
145
+
146
+use i3test;
147
+
148
+# Opens tabs on the presently focused branch and adds several additional
149
+# windows. Shifts focus to somewhere in the middle of the tabs so the most
150
+# general case can be assumed.
151
+sub open_tabs {
152
+    cmd 'layout tabbed';
153
+    open_window;
154
+    open_window;
155
+    open_window;
156
+    open_window;
157
+    cmd 'focus left; focus left'
158
+}
159
+
160
+# Likewise for a stack
161
+sub open_stack {
162
+    cmd 'layout stacking';
163
+    open_window;
164
+    open_window;
165
+    open_window;
166
+    open_window;
167
+    cmd 'focus up; focus up'
168
+}
169
+
170
+# Gets the position of the given leaf within the given branch. The first
171
+# position is one (1). Returns negative one (-1) if the leaf cannot be found
172
+# within the branch.
173
+sub get_leaf_position {
174
+    my ($branch, $leaf) = @_;
175
+    my $position = -1;
176
+    for my $i (0 .. @{$branch->{nodes}}) {
177
+        if ($branch->{nodes}[$i]->{id} == $leaf) {
178
+            $position = $i + 1;
179
+            last;
180
+        };
181
+    }
182
+    return $position;
183
+}
184
+
185
+# convenience function to focus a con by id to avoid having to type an ugly
186
+# command each time
187
+sub focus_con {
188
+    my $con_id = shift @_;
189
+    cmd "[con_id=\"$con_id\"] focus";
190
+}
191
+
192
+# Places a leaf into a branch and focuses the leaf. The newly created branch
193
+# will have orientation specified by the second parameter.
194
+sub branchify {
195
+    my ($con_id, $orientation) = @_;
196
+    focus_con($con_id);
197
+    $orientation eq 'horizontal' ? cmd 'splith' : cmd 'splitv';
198
+    open_window;
199
+    focus_con($con_id);
200
+}
201
+
202
+##############################################################################
203
+# When moving a con right into tabs, the moving con should be placed as the
204
+# first tab in the branch
205
+##############################################################################
206
+my $ws = fresh_workspace;
207
+
208
+# create the target leaf
209
+open_window;
210
+my $target_leaf = get_focused($ws);
211
+
212
+# create the tabbed branch container
213
+open_window;
214
+cmd 'splith';
215
+open_tabs;
216
+
217
+# move the target leaf into the tabbed branch
218
+focus_con($target_leaf);
219
+cmd 'move right';
220
+
221
+# the target leaf should be the first in the branch
222
+my $branch = shift @{get_ws_content($ws)};
223
+is($branch->{nodes}[0]->{id}, $target_leaf, 'moving con right into tabs placed it as the first tab in the branch');
224
+
225
+# repeat the test when the target is in a branch
226
+cmd 'move up; move left';
227
+branchify($target_leaf, 'vertical');
228
+cmd 'move right';
229
+
230
+# the target leaf should be the first in the branch
231
+$branch = pop @{get_ws_content($ws)};
232
+is($branch->{nodes}[0]->{id}, $target_leaf, 'moving con right into tabs from a branch placed it as the first tab in the branch');
233
+
234
+##############################################################################
235
+# When moving a con right into a stack, the moving con should be placed
236
+# above the focused-inactive leaf
237
+##############################################################################
238
+$ws = fresh_workspace;
239
+
240
+# create the target leaf
241
+open_window;
242
+$target_leaf = get_focused($ws);
243
+
244
+# create the stacked branch container and find the focused leaf
245
+open_window;
246
+cmd 'splith';
247
+open_stack;
248
+my $secondary_leaf = get_focused($ws);
249
+
250
+# move the target leaf into the stacked branch
251
+focus_con($target_leaf);
252
+cmd 'move right';
253
+
254
+# the secondary focus leaf should be below the target
255
+$branch = shift @{get_ws_content($ws)};
256
+my $target_leaf_position = get_leaf_position($branch, $target_leaf);
257
+my $secondary_leaf_position = get_leaf_position($branch, $secondary_leaf);
258
+
259
+is($target_leaf_position, $secondary_leaf_position - 1, 'moving con right into a stack placed it below the focused-inactive leaf');
260
+
261
+# repeat the test when the target is in a branch
262
+cmd 'move up; move left';
263
+branchify($target_leaf, 'vertical');
264
+cmd 'move right';
265
+
266
+# the secondary focus leaf should be below the target
267
+$branch = pop @{get_ws_content($ws)};
268
+$target_leaf_position = get_leaf_position($branch, $target_leaf);
269
+$secondary_leaf_position = get_leaf_position($branch, $secondary_leaf);
270
+
271
+is($target_leaf_position, $secondary_leaf_position - 1, 'moving con right into a stack from a branch placed it below the focused-inactive leaf');
272
+
273
+##############################################################################
274
+# When moving a con down into a stack, the moving con should be placed at the
275
+# top of the stack
276
+##############################################################################
277
+$ws = fresh_workspace;
278
+cmd 'layout splitv';
279
+
280
+# create the target leaf
281
+open_window;
282
+$target_leaf = get_focused($ws);
283
+
284
+# create the stacked branch container
285
+open_window;
286
+cmd 'splitv';
287
+open_stack;
288
+
289
+# move the target leaf into the stacked branch
290
+focus_con($target_leaf);
291
+cmd 'move down';
292
+
293
+# the target leaf should be on the top of the stack 
294
+$branch = shift @{get_ws_content($ws)};
295
+is($branch->{nodes}[0]->{id}, $target_leaf, 'moving con down into a stack placed it on the top of the stack');
296
+
297
+# repeat the test when the target is in a branch
298
+cmd 'move right; move up';
299
+branchify($target_leaf, 'horizontal');
300
+cmd 'move down';
301
+
302
+# the target leaf should be on the top of the stack 
303
+$branch = pop @{get_ws_content($ws)};
304
+is($branch->{nodes}[0]->{id}, $target_leaf, 'moving con down into a stack from a branch placed it on the top of the stack');
305
+
306
+##############################################################################
307
+# When moving a con down into tabs, the moving con should be placed before the
308
+# focused-inactive tab
309
+##############################################################################
310
+$ws = fresh_workspace;
311
+cmd 'layout splitv';
312
+
313
+# create the target leaf
314
+open_window;
315
+$target_leaf = get_focused($ws);
316
+
317
+# create the tabbed branch container and find the focused tab
318
+open_window;
319
+cmd 'splitv';
320
+open_tabs;
321
+$secondary_leaf = get_focused($ws);
322
+
323
+# move the target leaf into the tabbed branch
324
+focus_con($target_leaf);
325
+cmd 'move down';
326
+
327
+# the secondary focus tab should be to the right
328
+$branch = shift @{get_ws_content($ws)};
329
+$target_leaf_position = get_leaf_position($branch, $target_leaf);
330
+$secondary_leaf_position = get_leaf_position($branch, $secondary_leaf);
331
+
332
+is($target_leaf_position, $secondary_leaf_position - 1, 'moving con down into tabs placed it before the focused-inactive tab');
333
+
334
+# repeat the test when the target is in a branch
335
+cmd 'move right; move up';
336
+branchify($target_leaf, 'horizontal');
337
+cmd 'move down';
338
+
339
+$branch = pop @{get_ws_content($ws)};
340
+$target_leaf_position = get_leaf_position($branch, $target_leaf);
341
+$secondary_leaf_position = get_leaf_position($branch, $secondary_leaf);
342
+
343
+is($target_leaf_position, $secondary_leaf_position - 1, 'moving con down into tabs from a branch placed it before the focused-inactive tab');
344
+
345
+##############################################################################
346
+# When moving a con left into tabs, the moving con should be placed as the last
347
+# tab in the branch
348
+##############################################################################
349
+$ws = fresh_workspace;
350
+
351
+# create the tabbed branch container
352
+open_window;
353
+cmd 'splith';
354
+open_tabs;
355
+
356
+# create the target leaf
357
+cmd 'focus parent';
358
+open_window;
359
+$target_leaf = get_focused($ws);
360
+
361
+# move the target leaf into the tabbed branch
362
+cmd 'move left';
363
+
364
+# the target leaf should be last in the branch
365
+$branch = shift @{get_ws_content($ws)};
366
+
367
+is($branch->{nodes}->[-1]->{id}, $target_leaf, 'moving con left into tabs placed it as the last tab in the branch');
368
+
369
+# repeat the test when the target leaf is in a branch
370
+cmd 'move up; move right';
371
+branchify($target_leaf, 'vertical');
372
+cmd 'move left';
373
+
374
+$branch = shift @{get_ws_content($ws)};
375
+is($branch->{nodes}->[-1]->{id}, $target_leaf, 'moving con left into tabs from a branch placed it as the last tab in the branch');
376
+
377
+##############################################################################
378
+# When moving a con left into a stack, the moving con should be placed below
379
+# the focused-inactive leaf
380
+##############################################################################
381
+$ws = fresh_workspace;
382
+
383
+# create the stacked branch container and find the focused leaf
384
+open_window;
385
+open_stack;
386
+$secondary_leaf = get_focused($ws);
387
+
388
+# create the target leaf to the right
389
+cmd 'focus parent';
390
+open_window;
391
+$target_leaf = get_focused($ws);
392
+
393
+# move the target leaf into the stacked branch
394
+cmd 'move left';
395
+
396
+# the secondary focus leaf should be above
397
+$branch = shift @{get_ws_content($ws)};
398
+$target_leaf_position = get_leaf_position($branch, $target_leaf);
399
+$secondary_leaf_position = get_leaf_position($branch, $secondary_leaf);
400
+
401
+is($target_leaf_position, $secondary_leaf_position + 1, 'moving con left into a stack placed it above the focused-inactive leaf');
402
+
403
+# repeat the test when the target leaf is in a branch
404
+cmd 'move up; move right';
405
+branchify($target_leaf, 'vertical');
406
+cmd 'move left';
407
+
408
+# the secondary focus leaf should be above
409
+$branch = shift @{get_ws_content($ws)};
410
+$target_leaf_position = get_leaf_position($branch, $target_leaf);
411
+$secondary_leaf_position = get_leaf_position($branch, $secondary_leaf);
412
+
413
+is($target_leaf_position, $secondary_leaf_position + 1, 'moving con left into a stack from a branch placed it above the focused-inactive leaf');
414
+
415
+##############################################################################
416
+# When moving a con up into a stack, the moving con should be placed last in
417
+# the stack
418
+##############################################################################
419
+$ws = fresh_workspace;
420
+cmd 'layout splitv';
421
+
422
+# create the stacked branch container
423
+open_window;
424
+cmd 'splitv';
425
+open_stack;
426
+
427
+# create the target leaf
428
+cmd 'focus parent';
429
+open_window;
430
+$target_leaf = get_focused($ws);
431
+
432
+# move the target leaf into the stacked branch
433
+cmd 'move up';
434
+
435
+# the target leaf should be on the bottom of the stack
436
+$branch = shift @{get_ws_content($ws)};
437
+
438
+is($branch->{nodes}->[-1]->{id}, $target_leaf, 'moving con up into stack placed it on the bottom of the stack');
439
+
440
+# repeat the test when the target leaf is in a branch
441
+cmd 'move right; move down';
442
+branchify($target_leaf, 'horizontal');
443
+cmd 'move up';
444
+
445
+# the target leaf should be on the bottom of the stack
446
+$branch = shift @{get_ws_content($ws)};
447
+
448
+is($branch->{nodes}->[-1]->{id}, $target_leaf, 'moving con up into stack from a branch placed it on the bottom of the stack');
449
+
450
+##############################################################################
451
+# When moving a con up into tabs, the moving con should be placed after the
452
+# focused-inactive tab
453
+##############################################################################
454
+$ws = fresh_workspace;
455
+cmd 'layout splitv';
456
+
457
+# create the tabbed branch container and find the focused leaf
458
+open_window;
459
+cmd 'splitv';
460
+open_tabs;
461
+$secondary_leaf = get_focused($ws);
462
+
463
+# create the target leaf below
464
+cmd 'focus parent';
465
+open_window;
466
+$target_leaf = get_focused($ws);
467
+
468
+# move the target leaf into the tabbed branch
469
+cmd 'move up';
470
+
471
+# the secondary focus tab should be to the left
472
+$branch = shift @{get_ws_content($ws)};
473
+$target_leaf_position = get_leaf_position($branch, $target_leaf);
474
+$secondary_leaf_position = get_leaf_position($branch, $secondary_leaf);
475
+
476
+is($target_leaf_position, $secondary_leaf_position + 1, 'moving con up into tabs placed it after the focused-inactive tab');
477
+
478
+# repeat the test when the target leaf is in a branch
479
+cmd 'move right; move down';
480
+branchify($target_leaf, 'horizontal');
481
+cmd 'move up';
482
+
483
+# the secondary focus tab should be to the left
484
+$branch = shift @{get_ws_content($ws)};
485
+$target_leaf_position = get_leaf_position($branch, $target_leaf);
486
+$secondary_leaf_position = get_leaf_position($branch, $secondary_leaf);
487
+
488
+is($target_leaf_position, $secondary_leaf_position + 1, 'moving con up into tabs from a branch placed it after the focused-inactive tab');
489
+
490
+done_testing;