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; |