diff options
author | Ryan Dwyer <ryandwyer1@gmail.com> | 2018-08-15 15:14:35 +1000 |
---|---|---|
committer | Ryan Dwyer <ryandwyer1@gmail.com> | 2018-08-15 15:14:35 +1000 |
commit | 701fcafc70f18c6f1982a742019e875beeb258d7 (patch) | |
tree | e40d92b53c4fefde865b1d23d3c1bcae8ea54c37 | |
parent | Merge pull request #2445 from RyanDwyer/resize-tiling-via-cursor (diff) | |
download | sway-701fcafc70f18c6f1982a742019e875beeb258d7.tar.gz sway-701fcafc70f18c6f1982a742019e875beeb258d7.tar.zst sway-701fcafc70f18c6f1982a742019e875beeb258d7.zip |
Use list_find in more places and refactor/fix workspace prev_next functions
The original purpose of this commit is to replace some for loops with
list_find. But while doing this I found the workspace_prev_next_impl
functions to be difficult to read and also contained a bug, so I
refactored them and fixed the bug.
To reproduce the bug:
* Have two outputs, where the left output has workspaces 1, 2, 3 and the
right output has workspaces 4, 5, 6. Make workspace 2 focused_inactive
and workspace 4 focused.
* Run `workspace prev`.
* Previously it would visit the left output, then apply `workspace prev`
to workspace 2, which focuses workspace 1.
* Now it will focus the rightmost workspace on the left output
(workspace 3).
The refactoring I made to the workspace functions are:
* Added the static keyword.
* They now accept an int dir rather than bool, to avoid an unnecessary
conversion.
* Rather than preparing start and end variables for the purpose of
iterating, just iterate everything.
* Replace for loops with list_find.
* Don't call workspace_output_prev_next_impl (this fixes the bug).
-rw-r--r-- | common/list.c | 2 | ||||
-rw-r--r-- | include/list.h | 2 | ||||
-rw-r--r-- | sway/tree/layout.c | 17 | ||||
-rw-r--r-- | sway/tree/root.c | 8 | ||||
-rw-r--r-- | sway/tree/workspace.c | 76 |
5 files changed, 36 insertions, 69 deletions
diff --git a/common/list.c b/common/list.c index a3a22d8f..ee268c9a 100644 --- a/common/list.c +++ b/common/list.c | |||
@@ -77,7 +77,7 @@ int list_seq_find(list_t *list, int compare(const void *item, const void *data), | |||
77 | return -1; | 77 | return -1; |
78 | } | 78 | } |
79 | 79 | ||
80 | int list_find(list_t *list, void *item) { | 80 | int list_find(list_t *list, const void *item) { |
81 | for (int i = 0; i < list->length; i++) { | 81 | for (int i = 0; i < list->length; i++) { |
82 | if (list->items[i] == item) { | 82 | if (list->items[i] == item) { |
83 | return i; | 83 | return i; |
diff --git a/include/list.h b/include/list.h index 7c0e4bd2..03851a82 100644 --- a/include/list.h +++ b/include/list.h | |||
@@ -20,7 +20,7 @@ void list_qsort(list_t *list, int compare(const void *left, const void *right)); | |||
20 | // Return index for first item in list that returns 0 for given compare | 20 | // Return index for first item in list that returns 0 for given compare |
21 | // function or -1 if none matches. | 21 | // function or -1 if none matches. |
22 | int list_seq_find(list_t *list, int compare(const void *item, const void *cmp_to), const void *cmp_to); | 22 | int list_seq_find(list_t *list, int compare(const void *item, const void *cmp_to), const void *cmp_to); |
23 | int list_find(list_t *list, void *item); | 23 | int list_find(list_t *list, const void *item); |
24 | // stable sort since qsort is not guaranteed to be stable | 24 | // stable sort since qsort is not guaranteed to be stable |
25 | void list_stable_sort(list_t *list, int compare(const void *a, const void *b)); | 25 | void list_stable_sort(list_t *list, int compare(const void *a, const void *b)); |
26 | // swap two elements in a list | 26 | // swap two elements in a list |
diff --git a/sway/tree/layout.c b/sway/tree/layout.c index 38e14d00..2b710403 100644 --- a/sway/tree/layout.c +++ b/sway/tree/layout.c | |||
@@ -20,14 +20,7 @@ | |||
20 | #include "log.h" | 20 | #include "log.h" |
21 | 21 | ||
22 | static int index_child(const struct sway_container *child) { | 22 | static int index_child(const struct sway_container *child) { |
23 | struct sway_container *parent = child->parent; | 23 | return list_find(child->parent->children, child); |
24 | for (int i = 0; i < parent->children->length; ++i) { | ||
25 | if (parent->children->items[i] == child) { | ||
26 | return i; | ||
27 | } | ||
28 | } | ||
29 | // This happens if the child is a floating container | ||
30 | return -1; | ||
31 | } | 24 | } |
32 | 25 | ||
33 | static void container_handle_fullscreen_reparent(struct sway_container *con, | 26 | static void container_handle_fullscreen_reparent(struct sway_container *con, |
@@ -125,11 +118,9 @@ struct sway_container *container_remove_child(struct sway_container *child) { | |||
125 | } | 118 | } |
126 | 119 | ||
127 | struct sway_container *parent = child->parent; | 120 | struct sway_container *parent = child->parent; |
128 | for (int i = 0; i < parent->children->length; ++i) { | 121 | int index = index_child(child); |
129 | if (parent->children->items[i] == child) { | 122 | if (index != -1) { |
130 | list_del(parent->children, i); | 123 | list_del(parent->children, index); |
131 | break; | ||
132 | } | ||
133 | } | 124 | } |
134 | child->parent = NULL; | 125 | child->parent = NULL; |
135 | container_notify_subtree_changed(parent); | 126 | container_notify_subtree_changed(parent); |
diff --git a/sway/tree/root.c b/sway/tree/root.c index 79f2194e..a974a461 100644 --- a/sway/tree/root.c +++ b/sway/tree/root.c | |||
@@ -84,11 +84,9 @@ void root_scratchpad_remove_container(struct sway_container *con) { | |||
84 | return; | 84 | return; |
85 | } | 85 | } |
86 | con->scratchpad = false; | 86 | con->scratchpad = false; |
87 | for (int i = 0; i < root_container.sway_root->scratchpad->length; ++i) { | 87 | int index = list_find(root_container.sway_root->scratchpad, con); |
88 | if (root_container.sway_root->scratchpad->items[i] == con) { | 88 | if (index != -1) { |
89 | list_del(root_container.sway_root->scratchpad, i); | 89 | list_del(root_container.sway_root->scratchpad, index); |
90 | break; | ||
91 | } | ||
92 | } | 90 | } |
93 | } | 91 | } |
94 | 92 | ||
diff --git a/sway/tree/workspace.c b/sway/tree/workspace.c index 0177068b..cd2a7a04 100644 --- a/sway/tree/workspace.c +++ b/sway/tree/workspace.c | |||
@@ -286,8 +286,8 @@ struct sway_container *workspace_by_name(const char *name) { | |||
286 | * the end and beginning. If next is false, the previous workspace is returned, | 286 | * the end and beginning. If next is false, the previous workspace is returned, |
287 | * otherwise the next one is returned. | 287 | * otherwise the next one is returned. |
288 | */ | 288 | */ |
289 | struct sway_container *workspace_output_prev_next_impl( | 289 | static struct sway_container *workspace_output_prev_next_impl( |
290 | struct sway_container *output, bool next) { | 290 | struct sway_container *output, int dir) { |
291 | if (!output) { | 291 | if (!output) { |
292 | return NULL; | 292 | return NULL; |
293 | } | 293 | } |
@@ -302,27 +302,17 @@ struct sway_container *workspace_output_prev_next_impl( | |||
302 | focus : | 302 | focus : |
303 | container_parent(focus, C_WORKSPACE)); | 303 | container_parent(focus, C_WORKSPACE)); |
304 | 304 | ||
305 | int i; | 305 | int index = list_find(output->children, workspace); |
306 | for (i = 0; i < output->children->length; i++) { | 306 | size_t new_index = wrap(index + dir, output->children->length); |
307 | if (output->children->items[i] == workspace) { | 307 | return output->children->items[new_index]; |
308 | return output->children->items[ | ||
309 | wrap(i + (next ? 1 : -1), output->children->length)]; | ||
310 | } | ||
311 | } | ||
312 | |||
313 | // Doesn't happen, at worst the for loop returns the previously active | ||
314 | // workspace | ||
315 | return NULL; | ||
316 | } | 308 | } |
317 | 309 | ||
318 | /** | 310 | /** |
319 | * Get the previous or next workspace. If the first/last workspace on an output | 311 | * Get the previous or next workspace. If the first/last workspace on an output |
320 | * is active, proceed to the previous/next output's previous/next workspace. If | 312 | * is active, proceed to the previous/next output's previous/next workspace. |
321 | * next is false, the previous workspace is returned, otherwise the next one is | ||
322 | * returned. | ||
323 | */ | 313 | */ |
324 | struct sway_container *workspace_prev_next_impl( | 314 | static struct sway_container *workspace_prev_next_impl( |
325 | struct sway_container *workspace, bool next) { | 315 | struct sway_container *workspace, int dir) { |
326 | if (!workspace) { | 316 | if (!workspace) { |
327 | return NULL; | 317 | return NULL; |
328 | } | 318 | } |
@@ -331,52 +321,40 @@ struct sway_container *workspace_prev_next_impl( | |||
331 | return NULL; | 321 | return NULL; |
332 | } | 322 | } |
333 | 323 | ||
334 | struct sway_container *current_output = workspace->parent; | 324 | struct sway_container *output = workspace->parent; |
335 | int offset = next ? 1 : -1; | 325 | int index = list_find(output->children, workspace); |
336 | int start = next ? 0 : 1; | 326 | int new_index = index + dir; |
337 | int end; | ||
338 | if (next) { | ||
339 | end = current_output->children->length - 1; | ||
340 | } else { | ||
341 | end = current_output->children->length; | ||
342 | } | ||
343 | int i; | ||
344 | for (i = start; i < end; i++) { | ||
345 | if (current_output->children->items[i] == workspace) { | ||
346 | return current_output->children->items[i + offset]; | ||
347 | } | ||
348 | } | ||
349 | 327 | ||
350 | // Given workspace is the first/last on the output, jump to the | 328 | if (new_index >= 0 && new_index < output->children->length) { |
351 | // previous/next output | 329 | return output->children->items[index + dir]; |
352 | int num_outputs = root_container.children->length; | ||
353 | for (i = 0; i < num_outputs; i++) { | ||
354 | if (root_container.children->items[i] == current_output) { | ||
355 | struct sway_container *next_output = root_container.children->items[ | ||
356 | wrap(i + offset, num_outputs)]; | ||
357 | return workspace_output_prev_next_impl(next_output, next); | ||
358 | } | ||
359 | } | 330 | } |
360 | 331 | ||
361 | // Doesn't happen, at worst the for loop returns the previously active | 332 | // Look on a different output |
362 | // workspace on the active output | 333 | int output_index = list_find(root_container.children, output); |
363 | return NULL; | 334 | new_index = wrap(output_index + dir, root_container.children->length); |
335 | output = root_container.children->items[new_index]; | ||
336 | |||
337 | if (dir == 1) { | ||
338 | return output->children->items[0]; | ||
339 | } else { | ||
340 | return output->children->items[output->children->length - 1]; | ||
341 | } | ||
364 | } | 342 | } |
365 | 343 | ||
366 | struct sway_container *workspace_output_next(struct sway_container *current) { | 344 | struct sway_container *workspace_output_next(struct sway_container *current) { |
367 | return workspace_output_prev_next_impl(current, true); | 345 | return workspace_output_prev_next_impl(current, 1); |
368 | } | 346 | } |
369 | 347 | ||
370 | struct sway_container *workspace_next(struct sway_container *current) { | 348 | struct sway_container *workspace_next(struct sway_container *current) { |
371 | return workspace_prev_next_impl(current, true); | 349 | return workspace_prev_next_impl(current, 1); |
372 | } | 350 | } |
373 | 351 | ||
374 | struct sway_container *workspace_output_prev(struct sway_container *current) { | 352 | struct sway_container *workspace_output_prev(struct sway_container *current) { |
375 | return workspace_output_prev_next_impl(current, false); | 353 | return workspace_output_prev_next_impl(current, -1); |
376 | } | 354 | } |
377 | 355 | ||
378 | struct sway_container *workspace_prev(struct sway_container *current) { | 356 | struct sway_container *workspace_prev(struct sway_container *current) { |
379 | return workspace_prev_next_impl(current, false); | 357 | return workspace_prev_next_impl(current, -1); |
380 | } | 358 | } |
381 | 359 | ||
382 | bool workspace_switch(struct sway_container *workspace, | 360 | bool workspace_switch(struct sway_container *workspace, |